Skip to content

feat(table): add indexBy prop for ID-based expansion tracking #678

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
platex-rehor-bot wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(table): add indexBy prop for ID-based expansion tracking #678

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions packages/module/src/DataViewTableBasic/DataViewTableBasic.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,16 @@ const expandableContents: ExpandableContent[] = [
{ rowId: 1, columnId: 1, content: <div>Branch details for Repository one</div> },
];

// 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: <div>Branch details for Repository one</div> },
];

const ouiaId = 'TableExample';

describe('DataViewTable component', () => {
Expand Down Expand Up @@ -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(
<DataViewTableBasic aria-label='Repositories table' ouiaId={ouiaId} columns={columns} rows={objectRows} indexBy="id" />
);
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(
<DataViewTableBasic
aria-label='Repositories table'
ouiaId={ouiaId}
columns={columns}
rows={objectRows}
isExpandable={true}
expandedRows={objectExpandableContents}
indexBy="id"
/>
);

// 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(
<DataViewTableBasic
aria-label='Repositories table'
ouiaId={ouiaId}
columns={columns}
rows={rows}
isExpandable={true}
expandedRows={expandableContents}
/>
);

// 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();
});
});
37 changes: 23 additions & 14 deletions packages/module/src/DataViewTableBasic/DataViewTableBasic.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -40,6 +40,8 @@ export interface DataViewTableBasicProps extends Omit<TableProps, 'onSelect' | '
isExpandable?: boolean;
/** Toggles sticky columns and header */
isSticky?: boolean;
/** 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`). */
indexBy?: string;
}

export const DataViewTableBasic: FC<DataViewTableBasicProps> = ({
Expand All @@ -52,6 +54,7 @@ export const DataViewTableBasic: FC<DataViewTableBasicProps> = ({
hasResizableColumns,
isExpandable = false,
isSticky = false,
indexBy,
...props
}: DataViewTableBasicProps) => {
const { selection, activeState, isSelectable } = useInternalContext();
Expand All @@ -60,26 +63,32 @@ export const DataViewTableBasic: FC<DataViewTableBasicProps> = ({
const activeHeadState = useMemo(() => activeState ? headStates?.[activeState] : undefined, [ activeState, headStates ]);
const activeBodyState = useMemo(() => activeState ? bodyStates?.[activeState] : undefined, [ activeState, bodyStates ]);

const [expandedRowsState, setExpandedRowsState] = useState<Record<number, boolean>>({})
const [expandedColumnIndex, setExpandedColumnIndex] = useState<Record<number, number>>({})
const [expandedRowsState, setExpandedRowsState] = useState<Record<string | number, boolean>>({})
const [expandedColumnIndex, setExpandedColumnIndex] = useState<Record<string | number, number>>({})

const tableRef = useRef<HTMLTableElement>(null);

const needsSeparateTbody = isExpandable;

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<string, unknown>)[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 = (
Expand All @@ -100,7 +109,7 @@ export const DataViewTableBasic: FC<DataViewTableBasicProps> = ({
{(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 (
<Td
Expand All @@ -109,14 +118,14 @@ export const DataViewTableBasic: FC<DataViewTableBasicProps> = ({
{...(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
Expand Down Expand Up @@ -149,7 +158,7 @@ export const DataViewTableBasic: FC<DataViewTableBasicProps> = ({
} 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 : <Tbody>{renderedRows}</Tbody>);

Expand Down
22 changes: 21 additions & 1 deletion packages/module/src/DataViewTableTree/DataViewTableTree.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,28 @@ describe('DataViewTableTree component', () => {
const { container } = render(
<DataView activeState="loading">
<DataViewTable isTreeTable aria-label='Repositories table' ouiaId={ouiaId} columns={columns} bodyStates={{ loading: "Data is loading" }} rows={[]} />
</DataView>
</DataView>
);
expect(container).toMatchSnapshot();
});

test('should render tree table with indexBy prop', () => {
const { container } = render(
<DataView selection={mockSelection}>
<DataViewTable isTreeTable aria-label='Repositories table' ouiaId={ouiaId} columns={columns} rows={rows} indexBy="id" leafIcon={<LeafIcon/>} expandedIcon={<FolderOpenIcon aria-hidden />} collapsedIcon={<FolderIcon aria-hidden />} />
</DataView>
);
expect(container.querySelectorAll('tr').length).toBeGreaterThan(0);
});

test('should render tree table with expandAll and indexBy', () => {
const { container } = render(
<DataView selection={mockSelection}>
<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 />} />
</DataView>
);
// With expandAll, all expandable rows should be rendered as expanded
expect(container.querySelectorAll('tr').length).toBeGreaterThan(0);
expect(container).toMatchSnapshot();
});
});
35 changes: 22 additions & 13 deletions packages/module/src/DataViewTableTree/DataViewTableTree.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ export interface DataViewTableTreeProps extends Omit<TableProps, 'onSelect' | 'r
expandAll?: boolean;
/** Custom OUIA ID */
ouiaId?: string;
/** Property name used as unique node identifier for expansion tracking. When provided, this property is used instead of `id` to key the expanded state. */
indexBy?: string;
}

export const DataViewTableTree: FC<DataViewTableTreeProps> = ({
Expand All @@ -83,33 +85,38 @@ export const DataViewTableTree: FC<DataViewTableTreeProps> = ({
collapsedIcon = null,
expandAll = false,
ouiaId = 'DataViewTableTree',
indexBy,
...props
}: DataViewTableTreeProps) => {
const { selection, activeState } = useInternalContext();
const { onSelect, isSelected, isSelectDisabled } = selection ?? {};
const [ expandedNodeIds, setExpandedNodeIds ] = useState<string[]>([]);
const [ expandedDetailsNodeNames, setExpandedDetailsNodeIds ] = useState<string[]>([]);

/** 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<string, unknown>)[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(() => {
Expand All @@ -136,8 +143,9 @@ export const DataViewTableTree: FC<DataViewTableTreeProps> = ({
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) {
Expand All @@ -147,13 +155,13 @@ export const DataViewTableTree: FC<DataViewTableTreeProps> = ({
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,
Expand All @@ -165,7 +173,7 @@ export const DataViewTableTree: FC<DataViewTableTreeProps> = ({
'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,
},
};
Expand All @@ -175,7 +183,7 @@ export const DataViewTableTree: FC<DataViewTableTreeProps> = ({
: [];

return [
<TreeRowWrapper key={node.id} row={{ props: treeRow.props }} ouiaId={`${ouiaId}-tr-${rowIndex}`}>
<TreeRowWrapper key={nodeId} row={{ props: treeRow.props }} ouiaId={`${ouiaId}-tr-${rowIndex}`}>
{node.row.map((cell, colIndex) => {
const cellIsObject = isDataViewTdObject(cell);
return (
Expand Down Expand Up @@ -206,7 +214,8 @@ export const DataViewTableTree: FC<DataViewTableTreeProps> = ({
isSelected,
onSelect,
isSelectDisabled,
ouiaId
ouiaId,
indexBy
]);

return (
Expand Down
Loading