From 6c435f2e8d8f60a963f6cd8f5e82f0ea312de55d Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Fri, 1 May 2026 16:33:24 -0400 Subject: [PATCH] feat(DataViewTableBasic): Make select column sticky When attempting to make the first column in a table row sticky, it doesn't make the selection column sticky and the selection column will scroll with the rest of the table. This PR makes the selection column sticky along with the "first" column. --- README.md | 4 +- cypress/component/DataViewTableBasic.cy.tsx | 43 +++++++ .../data-view/examples/Table/Table.md | 5 +- packages/module/src/DataViewTable/index.ts | 1 + .../stickySelectionColumn.test.ts | 112 ++++++++++++++++++ .../DataViewTable/stickySelectionColumn.ts | 56 +++++++++ .../DataViewTableBasic.test.tsx | 43 +++++++ .../DataViewTableBasic/DataViewTableBasic.tsx | 35 +++++- .../DataViewTableHead.test.tsx | 27 ++++- .../DataViewTableHead/DataViewTableHead.tsx | 28 ++++- 10 files changed, 342 insertions(+), 12 deletions(-) create mode 100644 packages/module/src/DataViewTable/stickySelectionColumn.test.ts create mode 100644 packages/module/src/DataViewTable/stickySelectionColumn.ts diff --git a/README.md b/README.md index 5f218c2c..8a3a392a 100644 --- a/README.md +++ b/README.md @@ -152,8 +152,8 @@ When adding/making changes to a component, always make sure your code is tested: ## Testing and Linting - run `npm run test` to run the unit tests -- run `cypress:run:ci:cp` to run component tests -- run `cypress:run:ci:e2e` to run E2E tests +- run `npm run cypress:run:ci:cp` to run component tests +- run `npm run cypress:run:ci:e2e` to run E2E tests - run `npm run lint` to run the linter ## A11y testing diff --git a/cypress/component/DataViewTableBasic.cy.tsx b/cypress/component/DataViewTableBasic.cy.tsx index 694e62f3..7cd8bed5 100644 --- a/cypress/component/DataViewTableBasic.cy.tsx +++ b/cypress/component/DataViewTableBasic.cy.tsx @@ -23,6 +23,30 @@ const rows = repositories.map(item => Object.values(item)); const columns = [ 'Repositories', 'Branches', 'Pull requests', 'Workspaces', 'Last commit' ]; +const stickyColumns = [ + { cell: 'Repositories', props: { isStickyColumn: true, hasRightBorder: true } }, + 'Branches', + 'Pull requests', + 'Workspaces', + 'Last commit', +]; + +const stickyRows = [ + { name: 'Repository one', branches: 'Branch one', prs: 'Pull request one', workspaces: 'Workspace one', lastCommit: 'Timestamp one' }, +].map(item => [ + { cell: item.name, props: { isStickyColumn: true, hasRightBorder: true } }, + item.branches, + item.prs, + item.workspaces, + item.lastCommit, +]); + +const selection = { + onSelect: () => undefined, + isSelected: () => false, + isSelectDisabled: () => false, +}; + describe('DataViewTableBasic', () => { it('renders a basic data view table', () => { @@ -102,4 +126,23 @@ describe('DataViewTableBasic', () => { cy.get('[data-ouia-component-id="data-tr-loading"]').contains('Data is loading'); }); + it('applies sticky column styling to the selection and first data column when isSticky and the first column is sticky', () => { + const ouiaId = 'data-sticky-select'; + + cy.mount( + + + + ); + + cy.get('thead tr th.pf-v6-c-table__sticky-cell').should('have.length', 2); + cy.get('tbody tr').first().find('td.pf-v6-c-table__sticky-cell').should('have.length', 2); + }); + }); \ No newline at end of file diff --git a/packages/module/patternfly-docs/content/extensions/data-view/examples/Table/Table.md b/packages/module/patternfly-docs/content/extensions/data-view/examples/Table/Table.md index 2aa382ad..23dbe8c3 100644 --- a/packages/module/patternfly-docs/content/extensions/data-view/examples/Table/Table.md +++ b/packages/module/patternfly-docs/content/extensions/data-view/examples/Table/Table.md @@ -19,7 +19,8 @@ propComponents: 'DataViewTrTree', 'DataViewTrObject', 'DataViewTh', - 'DataViewThResizableProps' + 'DataViewThResizableProps', + 'DataViewTableHead' ] sourceLink: https://github.com/patternfly/react-data-view/blob/main/packages/module/patternfly-docs/content/extensions/data-view/examples/Table/Table.md --- @@ -105,6 +106,8 @@ When sticky headers and columns are enabled: - The table is wrapped in `OuterScrollContainer` and `InnerScrollContainer` components to enable sticky behavior - Sticky columns can have additional styling like borders using `hasRightBorder` or `hasLeftBorder` props +When **row selection** is enabled (via the `DataView` `selection` prop) and the first column in the `columns` array is a sticky column (`isStickyColumn: true` in that column’s `props`), the row-selection checkbox column is included in the same sticky group as that first data column. The first data column’s `stickyLeftOffset` is aligned so it sits to the right of the selection column. This requires the first **defined** column in `columns` to be the sticky one; a leading `null` placeholder in `columns` is not treated as a sticky first column for this behavior. + ### Sticky header and columns example ```js file="./DataViewTableStickyExample.tsx" diff --git a/packages/module/src/DataViewTable/index.ts b/packages/module/src/DataViewTable/index.ts index 35373805..c1eeddae 100644 --- a/packages/module/src/DataViewTable/index.ts +++ b/packages/module/src/DataViewTable/index.ts @@ -1,2 +1,3 @@ export { default } from './DataViewTable'; export * from './DataViewTable'; +export * from './stickySelectionColumn'; diff --git a/packages/module/src/DataViewTable/stickySelectionColumn.test.ts b/packages/module/src/DataViewTable/stickySelectionColumn.test.ts new file mode 100644 index 00000000..f0ecffb6 --- /dev/null +++ b/packages/module/src/DataViewTable/stickySelectionColumn.test.ts @@ -0,0 +1,112 @@ +import { + mergeFirstStickyDataColumnProps, + shouldIncludeStickySelectionColumn, + STICKY_SELECTION_COLUMN_WIDTH, + stickySelectionCellProps, +} from './stickySelectionColumn'; + +describe('stickySelectionColumn', () => { + describe('stickySelectionCellProps', () => { + it('matches row-selection sticky grouping props', () => { + expect(stickySelectionCellProps).toEqual({ + isStickyColumn: true, + hasRightBorder: false, + stickyMinWidth: STICKY_SELECTION_COLUMN_WIDTH, + }); + }); + }); + + describe('shouldIncludeStickySelectionColumn', () => { + it('is true when table is sticky, selectable, and first column is sticky', () => { + expect( + shouldIncludeStickySelectionColumn( + [ { cell: 'Name', props: { isStickyColumn: true } } ], + true, + true + ) + ).toBe(true); + }); + + it('is false when table is not sticky', () => { + expect( + shouldIncludeStickySelectionColumn( + [ { cell: 'Name', props: { isStickyColumn: true } } ], + true, + false + ) + ).toBe(false); + }); + + it('is false when not selectable', () => { + expect( + shouldIncludeStickySelectionColumn( + [ { cell: 'Name', props: { isStickyColumn: true } } ], + false, + true + ) + ).toBe(false); + }); + + it('is false when first column is not sticky', () => { + expect( + shouldIncludeStickySelectionColumn( + [ { cell: 'Name', props: { isStickyColumn: false } } ], + true, + true + ) + ).toBe(false); + }); + + it('is false when columns is empty', () => { + expect(shouldIncludeStickySelectionColumn([], true, true)).toBe(false); + }); + + it('is false when first column entry is null', () => { + expect( + shouldIncludeStickySelectionColumn([ null, { cell: 'Name', props: { isStickyColumn: true } } ], true, true) + ).toBe(false); + }); + }); + + describe('mergeFirstStickyDataColumnProps', () => { + it('adds stickyLeftOffset when including selection sticky', () => { + expect( + mergeFirstStickyDataColumnProps( + { isStickyColumn: true, hasRightBorder: true }, + true + ) + ).toEqual({ + isStickyColumn: true, + hasRightBorder: true, + stickyLeftOffset: STICKY_SELECTION_COLUMN_WIDTH, + }); + }); + + it('preserves existing stickyLeftOffset', () => { + expect( + mergeFirstStickyDataColumnProps( + { isStickyColumn: true, stickyLeftOffset: '80px' }, + true + ) + ).toEqual({ + isStickyColumn: true, + stickyLeftOffset: '80px', + }); + }); + + it('does not merge when first column is not sticky', () => { + expect( + mergeFirstStickyDataColumnProps({ isStickyColumn: false }, true) + ).toEqual({ isStickyColumn: false }); + }); + + it('returns column props unchanged when not including sticky selection', () => { + const props = { isStickyColumn: true, hasRightBorder: true }; + expect(mergeFirstStickyDataColumnProps(props, false)).toBe(props); + }); + + it('returns undefined when column props are undefined', () => { + expect(mergeFirstStickyDataColumnProps(undefined, true)).toBeUndefined(); + }); + }); +}); diff --git a/packages/module/src/DataViewTable/stickySelectionColumn.ts b/packages/module/src/DataViewTable/stickySelectionColumn.ts new file mode 100644 index 00000000..53dc5ef8 --- /dev/null +++ b/packages/module/src/DataViewTable/stickySelectionColumn.ts @@ -0,0 +1,56 @@ +import { TdProps, ThProps } from '@patternfly/react-table'; + +/** + * Min width / left offset for the row-selection column when it is grouped with a sticky first data column. + * Matches PatternFly’s typical checkbox column width so the Name column’s sticky inset aligns. + */ +export const STICKY_SELECTION_COLUMN_WIDTH = '4rem'; + +/** Props applied to the injected checkbox Th/Td when they participate in a sticky first-column group */ +export const stickySelectionCellProps: Pick< + ThProps, + 'isStickyColumn' | 'hasRightBorder' | 'stickyMinWidth' +> = { + isStickyColumn: true, + hasRightBorder: false, + stickyMinWidth: STICKY_SELECTION_COLUMN_WIDTH, +}; + +/** `columns[0]` object shape from {@link DataViewTh} (avoids importing DataViewTable and a circular dependency). */ +function isStickyFirstColumnDefinition(column: unknown): boolean { + return ( + column != null && + typeof column === 'object' && + 'props' in column && + (column as { props?: { isStickyColumn?: boolean } }).props?.isStickyColumn === true + ); +} + +export function shouldIncludeStickySelectionColumn( + columns: unknown[], + isSelectable: boolean, + isStickyTable: boolean +): boolean { + if (!isStickyTable || !isSelectable || columns.length === 0) { + return false; + } + const first = columns[0]; + if (first == null) { + return false; + } + return isStickyFirstColumnDefinition(first); +} + +/** Adds horizontal inset so the first sticky data column sits after the sticky selection column */ +export function mergeFirstStickyDataColumnProps

( + columnProps: P | undefined, + includeStickySelection: boolean +): P | undefined { + if (!columnProps || !includeStickySelection || !columnProps.isStickyColumn) { + return columnProps; + } + return { + ...columnProps, + stickyLeftOffset: columnProps.stickyLeftOffset ?? STICKY_SELECTION_COLUMN_WIDTH, + }; +} diff --git a/packages/module/src/DataViewTableBasic/DataViewTableBasic.test.tsx b/packages/module/src/DataViewTableBasic/DataViewTableBasic.test.tsx index 385319a6..571814ba 100644 --- a/packages/module/src/DataViewTableBasic/DataViewTableBasic.test.tsx +++ b/packages/module/src/DataViewTableBasic/DataViewTableBasic.test.tsx @@ -1,7 +1,9 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { DataView } from '../DataView'; +import { DataViewSelection } from '../InternalContext'; import { DataViewTableBasic, ExpandableContent } from './DataViewTableBasic'; +import { DataViewTh } from '../DataViewTable'; interface Repository { id: number; @@ -31,6 +33,20 @@ const rows = repositories.map(({ id, name, branches, prs, workspaces, lastCommit const columns = [ 'Repositories', 'Branches', 'Pull requests', 'Workspaces', 'Last commit' ]; +const stickyColumns: DataViewTh[] = [ + { cell: 'Repositories', props: { isStickyColumn: true, hasRightBorder: true } }, + 'Branches', + 'Pull requests', + 'Workspaces', + 'Last commit', +]; + +const mockSelection: DataViewSelection = { + onSelect: jest.fn(), + isSelected: jest.fn(() => false), + isSelectDisabled: jest.fn(() => false), +}; + const expandableContents: ExpandableContent[] = [ { rowId: 1, columnId: 1, content:

Branch details for Repository one
}, ]; @@ -72,6 +88,33 @@ describe('DataViewTable component', () => { expect(container).toMatchSnapshot(); }); + test('applies sticky classes to selection and first data cells when isSticky and first column is sticky', () => { + const stickyRows = repositories.map(({ id, name, branches, prs, workspaces, lastCommit }) => [ + { id, cell: name, props: { isStickyColumn: true, hasRightBorder: true } }, + branches, + prs, + workspaces, + lastCommit, + ]); + + const { container } = render( + + + + ); + + const firstBodyRow = container.querySelector('tbody tr'); + const bodyCells = firstBodyRow?.querySelectorAll('td'); + expect(bodyCells?.[0]?.classList.contains('pf-v6-c-table__sticky-cell')).toBe(true); + expect(bodyCells?.[1]?.classList.contains('pf-v6-c-table__sticky-cell')).toBe(true); + }); + test('when isExpandable cell should be clickable and expandable', async () => { const user = userEvent.setup(); diff --git a/packages/module/src/DataViewTableBasic/DataViewTableBasic.tsx b/packages/module/src/DataViewTableBasic/DataViewTableBasic.tsx index 7d3fe3ac..bd21cc3b 100644 --- a/packages/module/src/DataViewTableBasic/DataViewTableBasic.tsx +++ b/packages/module/src/DataViewTableBasic/DataViewTableBasic.tsx @@ -12,6 +12,11 @@ import { import { useInternalContext } from '../InternalContext'; import { DataViewTableHead } from '../DataViewTableHead'; import { DataViewTh, DataViewTr, isDataViewTdObject, isDataViewTrObject } from '../DataViewTable'; +import { + mergeFirstStickyDataColumnProps, + shouldIncludeStickySelectionColumn, + stickySelectionCellProps, +} from '../DataViewTable/stickySelectionColumn'; import { DataViewState } from '../DataView/DataView'; export interface ExpandableContent { @@ -57,6 +62,11 @@ export const DataViewTableBasic: FC = ({ const { selection, activeState, isSelectable } = useInternalContext(); const { onSelect, isSelected, isSelectDisabled } = selection ?? {}; + const includeStickySelection = useMemo( + () => shouldIncludeStickySelectionColumn(columns, isSelectable, isSticky), + [ columns, isSelectable, isSticky ] + ); + const activeHeadState = useMemo(() => activeState ? headStates?.[activeState] : undefined, [ activeState, headStates ]); const activeBodyState = useMemo(() => activeState ? bodyStates?.[activeState] : undefined, [ activeState, bodyStates ]); @@ -87,6 +97,7 @@ export const DataViewTableBasic: FC = ({ {isSelectable && ( { @@ -102,10 +113,13 @@ export const DataViewTableBasic: FC = ({ const cellExpandableContent = isExpandable ? expandedRows?.find( (content) => content.rowId === rowId && content.columnId === colIndex ) : undefined; + const baseTdProps = cellIsObject ? (cell?.props ?? {}) : {}; + const tdProps = + colIndex === 0 ? mergeFirstStickyDataColumnProps(baseTdProps, includeStickySelection) : baseTdProps; return ( = ({ } else { return rowContent; } - }), [ rows, isSelectable, isSelected, isSelectDisabled, onSelect, ouiaId, expandedRowsState, expandedColumnIndex, expandedRows, isExpandable, needsSeparateTbody ]); + }), [ + rows, + isSelectable, + isSelected, + isSelectDisabled, + onSelect, + ouiaId, + expandedRowsState, + expandedColumnIndex, + expandedRows, + isExpandable, + needsSeparateTbody, + includeStickySelection, + ]); const bodyContent = activeBodyState || (needsSeparateTbody ? renderedRows : {renderedRows}); @@ -158,7 +185,7 @@ export const DataViewTableBasic: FC = ({ - { activeHeadState || } + { activeHeadState || } { bodyContent }
@@ -167,7 +194,7 @@ export const DataViewTableBasic: FC = ({ } else { return ( - { activeHeadState || } + { activeHeadState || } { bodyContent }
); diff --git a/packages/module/src/DataViewTableHead/DataViewTableHead.test.tsx b/packages/module/src/DataViewTableHead/DataViewTableHead.test.tsx index 47f774d4..d4b34726 100644 --- a/packages/module/src/DataViewTableHead/DataViewTableHead.test.tsx +++ b/packages/module/src/DataViewTableHead/DataViewTableHead.test.tsx @@ -1,11 +1,20 @@ -import { render } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; import { Table } from '@patternfly/react-table'; import { DataViewTableHead } from './DataViewTableHead'; import { DataViewSelection } from '../InternalContext'; import { DataView } from '../DataView'; +import { DataViewTh } from '../DataViewTable'; const columns = [ 'Repositories', 'Branches', 'Pull requests', 'Workspaces', 'Last commit' ]; +const stickyFirstColumn: DataViewTh[] = [ + { cell: 'Repositories', props: { isStickyColumn: true, hasRightBorder: true } }, + 'Branches', + 'Pull requests', + 'Workspaces', + 'Last commit', +]; + const ouiaId = 'HeaderExample'; describe('DataViewTableHead component', () => { @@ -45,5 +54,21 @@ describe('DataViewTableHead component', () => { ); expect(container).toMatchSnapshot(); }); + + test('applies sticky classes to selection and first column when isSticky and first column is sticky', () => { + render( + + + +
+ + ); + + const selectionTh = screen.getByText('Data selection table head cell').closest('th'); + expect(selectionTh?.classList.contains('pf-v6-c-table__sticky-cell')).toBe(true); + + const repositoriesTh = screen.getByRole('columnheader', { name: 'Repositories' }); + expect(repositoriesTh.classList.contains('pf-v6-c-table__sticky-cell')).toBe(true); + }); }); diff --git a/packages/module/src/DataViewTableHead/DataViewTableHead.tsx b/packages/module/src/DataViewTableHead/DataViewTableHead.tsx index 81fc1e19..b54df3f1 100644 --- a/packages/module/src/DataViewTableHead/DataViewTableHead.tsx +++ b/packages/module/src/DataViewTableHead/DataViewTableHead.tsx @@ -2,6 +2,11 @@ import { FC, useMemo } from 'react'; import { Th, Thead, TheadProps, Tr } from '@patternfly/react-table'; import { useInternalContext } from '../InternalContext'; import { DataViewTh, isDataViewThObject } from '../DataViewTable'; +import { + mergeFirstStickyDataColumnProps, + shouldIncludeStickySelectionColumn, + stickySelectionCellProps, +} from '../DataViewTable/stickySelectionColumn'; import { DataViewTh as DataViewThElement } from '../DataViewTh/DataViewTh'; /** extends TheadProps */ @@ -14,6 +19,8 @@ export interface DataViewTableHeadProps extends TheadProps { ouiaId?: string; /** @hide Indicates whether table is resizable */ hasResizableColumns?: boolean; + /** When true with a sticky first data column and row selection, the selection column participates in the sticky group */ + isSticky?: boolean; } export const DataViewTableHead: FC = ({ @@ -21,15 +28,25 @@ export const DataViewTableHead: FC = ({ columns, ouiaId = 'DataViewTableHead', hasResizableColumns, + isSticky = false, ...props }: DataViewTableHeadProps) => { - const { selection } = useInternalContext(); + const { selection, isSelectable } = useInternalContext(); const { onSelect, isSelected } = selection ?? {}; + const includeStickySelection = useMemo( + () => shouldIncludeStickySelectionColumn(columns, isSelectable, isSticky), + [ columns, isSelectable, isSticky ] + ); + const cells = useMemo( () => [ onSelect && isSelected && !isTreeTable ? ( - + ) : null, ...columns.map((column, index) => ( = ({ content={isDataViewThObject(column) ? column.cell : column} resizableProps={isDataViewThObject(column) ? column.resizableProps : undefined} data-ouia-component-id={`${ouiaId}-th-${index}`} - thProps={isDataViewThObject(column) ? (column?.props ?? {}) : {}} + thProps={mergeFirstStickyDataColumnProps( + isDataViewThObject(column) ? (column?.props ?? {}) : {}, + index === 0 && includeStickySelection + )} hasResizableColumns={hasResizableColumns} /> )) ], - [ columns, ouiaId, onSelect, isSelected, isTreeTable, hasResizableColumns ] + [ columns, ouiaId, onSelect, isSelected, isTreeTable, hasResizableColumns, includeStickySelection ] ); return (