From 963333ce8880653ebc8be0a286b4de82f204b6af Mon Sep 17 00:00:00 2001 From: platex-rehor-bot Date: Thu, 2 Jul 2026 12:13:27 +0000 Subject: [PATCH] feat(table): add indexBy prop for ID-based expansion tracking RHCLOUD-48835 Support tracking expanded rows by a data property instead of array index. When indexBy is provided, expansion state follows the correct row through sort, filter, and pagination operations. Co-Authored-By: Claude Opus 4.6 --- .../DataViewTableBasic.test.tsx | 68 ++ .../DataViewTableBasic/DataViewTableBasic.tsx | 37 +- .../DataViewTableTree.test.tsx | 22 +- .../DataViewTableTree/DataViewTableTree.tsx | 35 +- .../DataViewTableTree.test.tsx.snap | 957 ++++++++++++++++++ 5 files changed, 1091 insertions(+), 28 deletions(-) diff --git a/packages/module/src/DataViewTableBasic/DataViewTableBasic.test.tsx b/packages/module/src/DataViewTableBasic/DataViewTableBasic.test.tsx index 385319a6..c52e1959 100644 --- a/packages/module/src/DataViewTableBasic/DataViewTableBasic.test.tsx +++ b/packages/module/src/DataViewTableBasic/DataViewTableBasic.test.tsx @@ -35,6 +35,16 @@ const expandableContents: ExpandableContent[] = [ { rowId: 1, columnId: 1, content:
Branch details for Repository one
}, ]; +// Rows using DataViewTrObject format with `id` for indexBy tests +const objectRows = repositories.map(({ id, name, branches, prs, workspaces, lastCommit }) => ({ + row: [ name, branches, prs, workspaces, lastCommit ], + id: String(id), +})); + +const objectExpandableContents: ExpandableContent[] = [ + { rowId: '1', columnId: 1, content:
Branch details for Repository one
}, +]; + const ouiaId = 'TableExample'; describe('DataViewTable component', () => { @@ -103,4 +113,62 @@ describe('DataViewTable component', () => { const branchContent = screen.getByText('Branch details for Repository one'); expect(branchContent.closest('tr')?.classList.contains('pf-m-expanded')).toBeTruthy(); }); + + test('should render correctly with indexBy prop', () => { + const { container } = render( + + ); + expect(container.querySelectorAll('tr').length).toBeGreaterThan(0); + }); + + test('when isExpandable with indexBy, expand button uses row id as key', async () => { + const user = userEvent.setup(); + + render( + + ); + + // The expandId should use the row's id value ("1") instead of the array index (0) + const branchExpandButton = document.getElementById('expandable-1-0-1'); + expect(branchExpandButton).toBeTruthy(); + + // Click the expand button + await user.click(branchExpandButton!); + + // After clicking, the expandable content should be visible + const branchContent = screen.getByText('Branch details for Repository one'); + expect(branchContent.closest('tr')?.classList.contains('pf-m-expanded')).toBeTruthy(); + }); + + test('without indexBy, expansion state uses array index (default behavior)', async () => { + const user = userEvent.setup(); + + render( + + ); + + // Without indexBy, expandId should use the array index (0) + const branchExpandButton = document.getElementById('expandable-0-0-1'); + expect(branchExpandButton).toBeTruthy(); + + await user.click(branchExpandButton!); + + const branchContent = screen.getByText('Branch details for Repository one'); + expect(branchContent.closest('tr')?.classList.contains('pf-m-expanded')).toBeTruthy(); + }); }); diff --git a/packages/module/src/DataViewTableBasic/DataViewTableBasic.tsx b/packages/module/src/DataViewTableBasic/DataViewTableBasic.tsx index 7d3fe3ac..6e049e9d 100644 --- a/packages/module/src/DataViewTableBasic/DataViewTableBasic.tsx +++ b/packages/module/src/DataViewTableBasic/DataViewTableBasic.tsx @@ -15,7 +15,7 @@ import { DataViewTh, DataViewTr, isDataViewTdObject, isDataViewTrObject } from ' import { DataViewState } from '../DataView/DataView'; export interface ExpandableContent { - rowId: number; + rowId: string | number; columnId: number; content: React.ReactNode; } @@ -40,6 +40,8 @@ export interface DataViewTableBasicProps extends Omit = ({ @@ -52,6 +54,7 @@ export const DataViewTableBasic: FC = ({ hasResizableColumns, isExpandable = false, isSticky = false, + indexBy, ...props }: DataViewTableBasicProps) => { const { selection, activeState, isSelectable } = useInternalContext(); @@ -60,8 +63,8 @@ export const DataViewTableBasic: FC = ({ const activeHeadState = useMemo(() => activeState ? headStates?.[activeState] : undefined, [ activeState, headStates ]); const activeBodyState = useMemo(() => activeState ? bodyStates?.[activeState] : undefined, [ activeState, bodyStates ]); - const [expandedRowsState, setExpandedRowsState] = useState>({}) - const [expandedColumnIndex, setExpandedColumnIndex] = useState>({}) + const [expandedRowsState, setExpandedRowsState] = useState>({}) + const [expandedColumnIndex, setExpandedColumnIndex] = useState>({}) const tableRef = useRef(null); @@ -69,17 +72,23 @@ export const DataViewTableBasic: FC = ({ const renderedRows = useMemo(() => rows.map((row, rowIndex) => { const rowIsObject = isDataViewTrObject(row); - const isRowExpanded = expandedRowsState[rowIndex] || false; - const expandedColIndex = expandedColumnIndex[rowIndex]; - // Get the first cell to extract the row ID + // Resolve the key used for expansion state tracking + const rowKey: string | number = indexBy && rowIsObject && indexBy in row + ? String((row as unknown as Record)[indexBy]) + : rowIndex; + + const isRowExpanded = expandedRowsState[rowKey] || false; + const expandedColIndex = expandedColumnIndex[rowKey]; + + // Get the first cell to extract the row ID (used for expandable content matching when indexBy is not set) const rowData = rowIsObject ? row.row : row; const firstCell = rowData[0]; - const rowId = isDataViewTdObject(firstCell) ? (firstCell as { id?: number }).id : undefined; + const rowId = isDataViewTdObject(firstCell) ? (firstCell as { id?: string | number }).id : undefined; // Find all expandable contents for this row const rowExpandableContents = isExpandable ? expandedRows?.filter( - (content) => content.rowId === rowId + (content) => indexBy ? String(content.rowId) === String(rowKey) : content.rowId === rowId ) : []; const rowContent = ( @@ -100,7 +109,7 @@ export const DataViewTableBasic: FC = ({ {(rowIsObject ? row.row : row).map((cell, colIndex) => { const cellIsObject = isDataViewTdObject(cell); const cellExpandableContent = isExpandable ? expandedRows?.find( - (content) => content.rowId === rowId && content.columnId === colIndex + (content) => (indexBy ? String(content.rowId) === String(rowKey) : content.rowId === rowId) && content.columnId === colIndex ) : undefined; return ( = ({ {...(cellExpandableContent != null && { compoundExpand: { isExpanded: isRowExpanded && expandedColIndex === colIndex, - expandId: `expandable-${rowIndex}`, + expandId: `expandable-${rowKey}`, onToggle: () => { setExpandedRowsState(prev => { const isSameColumn = expandedColIndex === colIndex; - const wasExpanded = prev[rowIndex]; - return { ...prev, [rowIndex]: isSameColumn ? !wasExpanded : true }; + const wasExpanded = prev[rowKey]; + return { ...prev, [rowKey]: isSameColumn ? !wasExpanded : true }; }); - setExpandedColumnIndex(prev => ({ ...prev, [rowIndex]: colIndex })); + setExpandedColumnIndex(prev => ({ ...prev, [rowKey]: colIndex })); }, rowIndex, columnIndex: colIndex @@ -149,7 +158,7 @@ export const DataViewTableBasic: FC = ({ } 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, indexBy ]); const bodyContent = activeBodyState || (needsSeparateTbody ? renderedRows : {renderedRows}); diff --git a/packages/module/src/DataViewTableTree/DataViewTableTree.test.tsx b/packages/module/src/DataViewTableTree/DataViewTableTree.test.tsx index f76d40b3..62cb3579 100644 --- a/packages/module/src/DataViewTableTree/DataViewTableTree.test.tsx +++ b/packages/module/src/DataViewTableTree/DataViewTableTree.test.tsx @@ -114,8 +114,28 @@ describe('DataViewTableTree component', () => { const { container } = render( - + + ); + expect(container).toMatchSnapshot(); + }); + + test('should render tree table with indexBy prop', () => { + const { container } = render( + + } expandedIcon={} collapsedIcon={} /> + + ); + expect(container.querySelectorAll('tr').length).toBeGreaterThan(0); + }); + + test('should render tree table with expandAll and indexBy', () => { + const { container } = render( + + } expandedIcon={} collapsedIcon={} /> + ); + // With expandAll, all expandable rows should be rendered as expanded + expect(container.querySelectorAll('tr').length).toBeGreaterThan(0); expect(container).toMatchSnapshot(); }); }); diff --git a/packages/module/src/DataViewTableTree/DataViewTableTree.tsx b/packages/module/src/DataViewTableTree/DataViewTableTree.tsx index a76ea52d..7a437b58 100644 --- a/packages/module/src/DataViewTableTree/DataViewTableTree.tsx +++ b/packages/module/src/DataViewTableTree/DataViewTableTree.tsx @@ -71,6 +71,8 @@ export interface DataViewTableTreeProps extends Omit = ({ @@ -83,6 +85,7 @@ export const DataViewTableTree: FC = ({ collapsedIcon = null, expandAll = false, ouiaId = 'DataViewTableTree', + indexBy, ...props }: DataViewTableTreeProps) => { const { selection, activeState } = useInternalContext(); @@ -90,26 +93,30 @@ export const DataViewTableTree: FC = ({ const [ expandedNodeIds, setExpandedNodeIds ] = useState([]); const [ expandedDetailsNodeNames, setExpandedDetailsNodeIds ] = useState([]); + /** Resolves the unique identifier for a tree node based on the `indexBy` prop or the default `id` field. */ + const getNodeId = (node: DataViewTrTree): string => + indexBy && indexBy in node ? String((node as unknown as Record)[indexBy]) : node.id; + // Helper function to collect all node IDs that have children (are expandable) const getExpandableNodeIds = (nodes: DataViewTrTree[]): string[] => { const expandableIds: string[] = []; - + const traverse = (nodeList: DataViewTrTree[]) => { nodeList.forEach(node => { if (node.children && node.children.length > 0) { - expandableIds.push(node.id); + expandableIds.push(getNodeId(node)); traverse(node.children); } }); }; - + traverse(nodes); return expandableIds; }; // Effect to handle expandAll behavior // Memoize the expandable IDs to avoid recalculating when rows object reference changes but structure is the same - const expandableIds = useMemo(() => getExpandableNodeIds(rows), [ rows ]); + const expandableIds = useMemo(() => getExpandableNodeIds(rows), [ rows, indexBy ]); // eslint-disable-line react-hooks/exhaustive-deps // Effect to handle expandAll behavior - only runs when IDs actually change useEffect(() => { @@ -136,8 +143,9 @@ export const DataViewTableTree: FC = ({ if (!node) { return []; } - const isExpanded = expandedNodeIds.includes(node.id); - const isDetailsExpanded = expandedDetailsNodeNames.includes(node.id); + const nodeId = getNodeId(node); + const isExpanded = expandedNodeIds.includes(nodeId); + const isDetailsExpanded = expandedDetailsNodeNames.includes(nodeId); const isChecked = isSelected?.(node); let icon = leafIcon; if (node.children) { @@ -147,13 +155,13 @@ export const DataViewTableTree: FC = ({ const treeRow: TdProps['treeRow'] = { onCollapse: () => setExpandedNodeIds(prevExpanded => { - const otherExpandedNodeIds = prevExpanded.filter(id => id !== node.id); - return isExpanded ? otherExpandedNodeIds : [ ...otherExpandedNodeIds, node.id ]; + const otherExpandedNodeIds = prevExpanded.filter(id => id !== nodeId); + return isExpanded ? otherExpandedNodeIds : [ ...otherExpandedNodeIds, nodeId ]; }), onToggleRowDetails: () => setExpandedDetailsNodeIds(prevDetailsExpanded => { - const otherDetailsExpandedNodeIds = prevDetailsExpanded.filter(id => id !== node.id); - return isDetailsExpanded ? otherDetailsExpandedNodeIds : [ ...otherDetailsExpandedNodeIds, node.id ]; + const otherDetailsExpandedNodeIds = prevDetailsExpanded.filter(id => id !== nodeId); + return isDetailsExpanded ? otherDetailsExpandedNodeIds : [ ...otherDetailsExpandedNodeIds, nodeId ]; }), onCheckChange: (isSelectDisabled?.(node) || !onSelect) ? undefined : (_event, isChecking) => onSelect?.(isChecking, getNodesAffectedBySelection(rows, node, isChecking, isSelected)), rowIndex, @@ -165,7 +173,7 @@ export const DataViewTableTree: FC = ({ 'aria-posinset': posinset, 'aria-setsize': node.children?.length ?? 0, isChecked, - checkboxId: `checkbox_id_${node.id?.toLowerCase().replace(/\s+/g, '_')}`, + checkboxId: `checkbox_id_${nodeId?.toLowerCase().replace(/\s+/g, '_')}`, icon, }, }; @@ -175,7 +183,7 @@ export const DataViewTableTree: FC = ({ : []; return [ - + {node.row.map((cell, colIndex) => { const cellIsObject = isDataViewTdObject(cell); return ( @@ -206,7 +214,8 @@ export const DataViewTableTree: FC = ({ isSelected, onSelect, isSelectDisabled, - ouiaId + ouiaId, + indexBy ]); return ( diff --git a/packages/module/src/DataViewTableTree/__snapshots__/DataViewTableTree.test.tsx.snap b/packages/module/src/DataViewTableTree/__snapshots__/DataViewTableTree.test.tsx.snap index c541ae09..2e75879e 100644 --- a/packages/module/src/DataViewTableTree/__snapshots__/DataViewTableTree.test.tsx.snap +++ b/packages/module/src/DataViewTableTree/__snapshots__/DataViewTableTree.test.tsx.snap @@ -2147,3 +2147,960 @@ exports[`DataViewTableTree component should render tree table with an error stat `; + +exports[`DataViewTableTree component should render tree table with expandAll and indexBy 1`] = ` +
+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ Repositories + + Branches + + Pull requests + + Workspaces + + Last commit +
+
+ + + + + + +
+ + + + + Repository one + +
+ + + +
+ 		+ Branch one + + Pull request one + + Workspace one + + Timestamp one +
+
+ + + + + + +
+ + + + + Repository four + +
+ + + +
+ 		+ Branch four + + Pull request four + + Workspace four + + Timestamp four +
+
+
+
+`;