Skip to content

Commit 93fc8b0

Browse files
platex-rehor-botclaude
platex-rehor-bot
and
claude
authored
feat(table): add indexBy prop for ID-based expansion tracking (#678)
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 <noreply@anthropic.com>
1 parent cf95c31 commit 93fc8b0

5 files changed

Lines changed: 1091 additions & 28 deletions

File tree

‎packages/module/src/DataViewTableBasic/DataViewTableBasic.test.tsx‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,16 @@ const expandableContents: ExpandableContent[] = [
3535
{ rowId: 1, columnId: 1, content: <div>Branch details for Repository one</div> },
3636
];
3737

38+
// Rows using DataViewTrObject format with `id` for indexBy tests
39+
const objectRows = repositories.map(({ id, name, branches, prs, workspaces, lastCommit }) => ({
40+
row: [ name, branches, prs, workspaces, lastCommit ],
41+
id: String(id),
42+
}));
43+
44+
const objectExpandableContents: ExpandableContent[] = [
45+
{ rowId: '1', columnId: 1, content: <div>Branch details for Repository one</div> },
46+
];
47+
3848
const ouiaId = 'TableExample';
3949

4050
describe('DataViewTable component', () => {
@@ -103,4 +113,62 @@ describe('DataViewTable component', () => {
103113
const branchContent = screen.getByText('Branch details for Repository one');
104114
expect(branchContent.closest('tr')?.classList.contains('pf-m-expanded')).toBeTruthy();
105115
});
116+
117+
test('should render correctly with indexBy prop', () => {
118+
const { container } = render(
119+
<DataViewTableBasic aria-label='Repositories table' ouiaId={ouiaId} columns={columns} rows={objectRows} indexBy="id" />
120+
);
121+
expect(container.querySelectorAll('tr').length).toBeGreaterThan(0);
122+
});
123+
124+
test('when isExpandable with indexBy, expand button uses row id as key', async () => {
125+
const user = userEvent.setup();
126+
127+
render(
128+
<DataViewTableBasic
129+
aria-label='Repositories table'
130+
ouiaId={ouiaId}
131+
columns={columns}
132+
rows={objectRows}
133+
isExpandable={true}
134+
expandedRows={objectExpandableContents}
135+
indexBy="id"
136+
/>
137+
);
138+
139+
// The expandId should use the row's id value ("1") instead of the array index (0)
140+
const branchExpandButton = document.getElementById('expandable-1-0-1');
141+
expect(branchExpandButton).toBeTruthy();
142+
143+
// Click the expand button
144+
await user.click(branchExpandButton!);
145+
146+
// After clicking, the expandable content should be visible
147+
const branchContent = screen.getByText('Branch details for Repository one');
148+
expect(branchContent.closest('tr')?.classList.contains('pf-m-expanded')).toBeTruthy();
149+
});
150+
151+
test('without indexBy, expansion state uses array index (default behavior)', async () => {
152+
const user = userEvent.setup();
153+
154+
render(
155+
<DataViewTableBasic
156+
aria-label='Repositories table'
157+
ouiaId={ouiaId}
158+
columns={columns}
159+
rows={rows}
160+
isExpandable={true}
161+
expandedRows={expandableContents}
162+
/>
163+
);
164+
165+
// Without indexBy, expandId should use the array index (0)
166+
const branchExpandButton = document.getElementById('expandable-0-0-1');
167+
expect(branchExpandButton).toBeTruthy();
168+
169+
await user.click(branchExpandButton!);
170+
171+
const branchContent = screen.getByText('Branch details for Repository one');
172+
expect(branchContent.closest('tr')?.classList.contains('pf-m-expanded')).toBeTruthy();
173+
});
106174
});

‎packages/module/src/DataViewTableBasic/DataViewTableBasic.tsx‎

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { DataViewTh, DataViewTr, isDataViewTdObject, isDataViewTrObject } from '
1515
import { DataViewState } from '../DataView/DataView';
1616

1717
export interface ExpandableContent {
18-
rowId: number;
18+
rowId: string | number;
1919
columnId: number;
2020
content: React.ReactNode;
2121
}
@@ -40,6 +40,8 @@ export interface DataViewTableBasicProps extends Omit<TableProps, 'onSelect' | '
4040
isExpandable?: boolean;
4141
/** Toggles sticky columns and header */
4242
isSticky?: boolean;
43+
/** Property name used as unique row identifier for expansion tracking. When provided, expanded state follows the data through sort/filter/pagination instead of using the array index. Applicable when rows are objects with an identifying property (e.g., `id`). */
44+
indexBy?: string;
4345
}
4446

4547
export const DataViewTableBasic: FC<DataViewTableBasicProps> = ({
@@ -52,6 +54,7 @@ export const DataViewTableBasic: FC<DataViewTableBasicProps> = ({
5254
hasResizableColumns,
5355
isExpandable = false,
5456
isSticky = false,
57+
indexBy,
5558
...props
5659
}: DataViewTableBasicProps) => {
5760
const { selection, activeState, isSelectable } = useInternalContext();
@@ -60,26 +63,32 @@ export const DataViewTableBasic: FC<DataViewTableBasicProps> = ({
6063
const activeHeadState = useMemo(() => activeState ? headStates?.[activeState] : undefined, [ activeState, headStates ]);
6164
const activeBodyState = useMemo(() => activeState ? bodyStates?.[activeState] : undefined, [ activeState, bodyStates ]);
6265

63-
const [expandedRowsState, setExpandedRowsState] = useState<Record<number, boolean>>({})
64-
const [expandedColumnIndex, setExpandedColumnIndex] = useState<Record<number, number>>({})
66+
const [expandedRowsState, setExpandedRowsState] = useState<Record<string | number, boolean>>({})
67+
const [expandedColumnIndex, setExpandedColumnIndex] = useState<Record<string | number, number>>({})
6568

6669
const tableRef = useRef<HTMLTableElement>(null);
6770

6871
const needsSeparateTbody = isExpandable;
6972

7073
const renderedRows = useMemo(() => rows.map((row, rowIndex) => {
7174
const rowIsObject = isDataViewTrObject(row);
72-
const isRowExpanded = expandedRowsState[rowIndex] || false;
73-
const expandedColIndex = expandedColumnIndex[rowIndex];
7475

75-
// Get the first cell to extract the row ID
76+
// Resolve the key used for expansion state tracking
77+
const rowKey: string | number = indexBy && rowIsObject && indexBy in row
78+
? String((row as unknown as Record<string, unknown>)[indexBy])
79+
: rowIndex;
80+
81+
const isRowExpanded = expandedRowsState[rowKey] || false;
82+
const expandedColIndex = expandedColumnIndex[rowKey];
83+
84+
// Get the first cell to extract the row ID (used for expandable content matching when indexBy is not set)
7685
const rowData = rowIsObject ? row.row : row;
7786
const firstCell = rowData[0];
78-
const rowId = isDataViewTdObject(firstCell) ? (firstCell as { id?: number }).id : undefined;
87+
const rowId = isDataViewTdObject(firstCell) ? (firstCell as { id?: string | number }).id : undefined;
7988

8089
// Find all expandable contents for this row
8190
const rowExpandableContents = isExpandable ? expandedRows?.filter(
82-
(content) => content.rowId === rowId
91+
(content) => indexBy ? String(content.rowId) === String(rowKey) : content.rowId === rowId
8392
) : [];
8493

8594
const rowContent = (
@@ -100,7 +109,7 @@ export const DataViewTableBasic: FC<DataViewTableBasicProps> = ({
100109
{(rowIsObject ? row.row : row).map((cell, colIndex) => {
101110
const cellIsObject = isDataViewTdObject(cell);
102111
const cellExpandableContent = isExpandable ? expandedRows?.find(
103-
(content) => content.rowId === rowId && content.columnId === colIndex
112+
(content) => (indexBy ? String(content.rowId) === String(rowKey) : content.rowId === rowId) && content.columnId === colIndex
104113
) : undefined;
105114
return (
106115
<Td
@@ -109,14 +118,14 @@ export const DataViewTableBasic: FC<DataViewTableBasicProps> = ({
109118
{...(cellExpandableContent != null && {
110119
compoundExpand: {
111120
isExpanded: isRowExpanded && expandedColIndex === colIndex,
112-
expandId: `expandable-${rowIndex}`,
121+
expandId: `expandable-${rowKey}`,
113122
onToggle: () => {
114123
setExpandedRowsState(prev => {
115124
const isSameColumn = expandedColIndex === colIndex;
116-
const wasExpanded = prev[rowIndex];
117-
return { ...prev, [rowIndex]: isSameColumn ? !wasExpanded : true };
125+
const wasExpanded = prev[rowKey];
126+
return { ...prev, [rowKey]: isSameColumn ? !wasExpanded : true };
118127
});
119-
setExpandedColumnIndex(prev => ({ ...prev, [rowIndex]: colIndex }));
128+
setExpandedColumnIndex(prev => ({ ...prev, [rowKey]: colIndex }));
120129
},
121130
rowIndex,
122131
columnIndex: colIndex
@@ -149,7 +158,7 @@ export const DataViewTableBasic: FC<DataViewTableBasicProps> = ({
149158
} else {
150159
return rowContent;
151160
}
152-
}), [ rows, isSelectable, isSelected, isSelectDisabled, onSelect, ouiaId, expandedRowsState, expandedColumnIndex, expandedRows, isExpandable, needsSeparateTbody ]);
161+
}), [ rows, isSelectable, isSelected, isSelectDisabled, onSelect, ouiaId, expandedRowsState, expandedColumnIndex, expandedRows, isExpandable, needsSeparateTbody, indexBy ]);
153162

154163
const bodyContent = activeBodyState || (needsSeparateTbody ? renderedRows : <Tbody>{renderedRows}</Tbody>);
155164

‎packages/module/src/DataViewTableTree/DataViewTableTree.test.tsx‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,28 @@ describe('DataViewTableTree component', () => {
114114
const { container } = render(
115115
<DataView activeState="loading">
116116
<DataViewTable isTreeTable aria-label='Repositories table' ouiaId={ouiaId} columns={columns} bodyStates={{ loading: "Data is loading" }} rows={[]} />
117-
</DataView>
117+
</DataView>
118+
);
119+
expect(container).toMatchSnapshot();
120+
});
121+
122+
test('should render tree table with indexBy prop', () => {
123+
const { container } = render(
124+
<DataView selection={mockSelection}>
125+
<DataViewTable isTreeTable aria-label='Repositories table' ouiaId={ouiaId} columns={columns} rows={rows} indexBy="id" leafIcon={<LeafIcon/>} expandedIcon={<FolderOpenIcon aria-hidden />} collapsedIcon={<FolderIcon aria-hidden />} />
126+
</DataView>
127+
);
128+
expect(container.querySelectorAll('tr').length).toBeGreaterThan(0);
129+
});
130+
131+
test('should render tree table with expandAll and indexBy', () => {
132+
const { container } = render(
133+
<DataView selection={mockSelection}>
134+
<DataViewTable isTreeTable aria-label='Repositories table' ouiaId={ouiaId} columns={columns} expandAll rows={rows} indexBy="id" leafIcon={<LeafIcon/>} expandedIcon={<FolderOpenIcon aria-hidden />} collapsedIcon={<FolderIcon aria-hidden />} />
135+
</DataView>
118136
);
137+
// With expandAll, all expandable rows should be rendered as expanded
138+
expect(container.querySelectorAll('tr').length).toBeGreaterThan(0);
119139
expect(container).toMatchSnapshot();
120140
});
121141
});

‎packages/module/src/DataViewTableTree/DataViewTableTree.tsx‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ export interface DataViewTableTreeProps extends Omit<TableProps, 'onSelect' | 'r
7171
expandAll?: boolean;
7272
/** Custom OUIA ID */
7373
ouiaId?: string;
74+
/** Property name used as unique node identifier for expansion tracking. When provided, this property is used instead of `id` to key the expanded state. */
75+
indexBy?: string;
7476
}
7577

7678
export const DataViewTableTree: FC<DataViewTableTreeProps> = ({
@@ -83,33 +85,38 @@ export const DataViewTableTree: FC<DataViewTableTreeProps> = ({
8385
collapsedIcon = null,
8486
expandAll = false,
8587
ouiaId = 'DataViewTableTree',
88+
indexBy,
8689
...props
8790
}: DataViewTableTreeProps) => {
8891
const { selection, activeState } = useInternalContext();
8992
const { onSelect, isSelected, isSelectDisabled } = selection ?? {};
9093
const [ expandedNodeIds, setExpandedNodeIds ] = useState<string[]>([]);
9194
const [ expandedDetailsNodeNames, setExpandedDetailsNodeIds ] = useState<string[]>([]);
9295

96+
/** Resolves the unique identifier for a tree node based on the `indexBy` prop or the default `id` field. */
97+
const getNodeId = (node: DataViewTrTree): string =>
98+
indexBy && indexBy in node ? String((node as unknown as Record<string, unknown>)[indexBy]) : node.id;
99+
93100
// Helper function to collect all node IDs that have children (are expandable)
94101
const getExpandableNodeIds = (nodes: DataViewTrTree[]): string[] => {
95102
const expandableIds: string[] = [];
96-
103+
97104
const traverse = (nodeList: DataViewTrTree[]) => {
98105
nodeList.forEach(node => {
99106
if (node.children && node.children.length > 0) {
100-
expandableIds.push(node.id);
107+
expandableIds.push(getNodeId(node));
101108
traverse(node.children);
102109
}
103110
});
104111
};
105-
112+
106113
traverse(nodes);
107114
return expandableIds;
108115
};
109116

110117
// Effect to handle expandAll behavior
111118
// Memoize the expandable IDs to avoid recalculating when rows object reference changes but structure is the same
112-
const expandableIds = useMemo(() => getExpandableNodeIds(rows), [ rows ]);
119+
const expandableIds = useMemo(() => getExpandableNodeIds(rows), [ rows, indexBy ]); // eslint-disable-line react-hooks/exhaustive-deps
113120

114121
// Effect to handle expandAll behavior - only runs when IDs actually change
115122
useEffect(() => {
@@ -136,8 +143,9 @@ export const DataViewTableTree: FC<DataViewTableTreeProps> = ({
136143
if (!node) {
137144
return [];
138145
}
139-
const isExpanded = expandedNodeIds.includes(node.id);
140-
const isDetailsExpanded = expandedDetailsNodeNames.includes(node.id);
146+
const nodeId = getNodeId(node);
147+
const isExpanded = expandedNodeIds.includes(nodeId);
148+
const isDetailsExpanded = expandedDetailsNodeNames.includes(nodeId);
141149
const isChecked = isSelected?.(node);
142150
let icon = leafIcon;
143151
if (node.children) {
@@ -147,13 +155,13 @@ export const DataViewTableTree: FC<DataViewTableTreeProps> = ({
147155
const treeRow: TdProps['treeRow'] = {
148156
onCollapse: () =>
149157
setExpandedNodeIds(prevExpanded => {
150-
const otherExpandedNodeIds = prevExpanded.filter(id => id !== node.id);
151-
return isExpanded ? otherExpandedNodeIds : [ ...otherExpandedNodeIds, node.id ];
158+
const otherExpandedNodeIds = prevExpanded.filter(id => id !== nodeId);
159+
return isExpanded ? otherExpandedNodeIds : [ ...otherExpandedNodeIds, nodeId ];
152160
}),
153161
onToggleRowDetails: () =>
154162
setExpandedDetailsNodeIds(prevDetailsExpanded => {
155-
const otherDetailsExpandedNodeIds = prevDetailsExpanded.filter(id => id !== node.id);
156-
return isDetailsExpanded ? otherDetailsExpandedNodeIds : [ ...otherDetailsExpandedNodeIds, node.id ];
163+
const otherDetailsExpandedNodeIds = prevDetailsExpanded.filter(id => id !== nodeId);
164+
return isDetailsExpanded ? otherDetailsExpandedNodeIds : [ ...otherDetailsExpandedNodeIds, nodeId ];
157165
}),
158166
onCheckChange: (isSelectDisabled?.(node) || !onSelect) ? undefined : (_event, isChecking) => onSelect?.(isChecking, getNodesAffectedBySelection(rows, node, isChecking, isSelected)),
159167
rowIndex,
@@ -165,7 +173,7 @@ export const DataViewTableTree: FC<DataViewTableTreeProps> = ({
165173
'aria-posinset': posinset,
166174
'aria-setsize': node.children?.length ?? 0,
167175
isChecked,
168-
checkboxId: `checkbox_id_${node.id?.toLowerCase().replace(/\s+/g, '_')}`,
176+
checkboxId: `checkbox_id_${nodeId?.toLowerCase().replace(/\s+/g, '_')}`,
169177
icon,
170178
},
171179
};
@@ -175,7 +183,7 @@ export const DataViewTableTree: FC<DataViewTableTreeProps> = ({
175183
: [];
176184

177185
return [
178-
<TreeRowWrapper key={node.id} row={{ props: treeRow.props }} ouiaId={`${ouiaId}-tr-${rowIndex}`}>
186+
<TreeRowWrapper key={nodeId} row={{ props: treeRow.props }} ouiaId={`${ouiaId}-tr-${rowIndex}`}>
179187
{node.row.map((cell, colIndex) => {
180188
const cellIsObject = isDataViewTdObject(cell);
181189
return (
@@ -206,7 +214,8 @@ export const DataViewTableTree: FC<DataViewTableTreeProps> = ({
206214
isSelected,
207215
onSelect,
208216
isSelectDisabled,
209-
ouiaId
217+
ouiaId,
218+
indexBy
210219
]);
211220

212221
return (

0 commit comments

Comments
 (0)