Skip to content

Commit 1faa2ac

Browse files
bugfix(CUI-22): separate active row from selectedRowIds in MultiSelectableContent
Clicking a row body to trigger `onSingleRowSelected` (e.g. opening a detail panel) used to call `row.toggleRowSelected(true)`, polluting react-table's `selectedRowIds`. A subsequent checkbox click on a different row would then read the stale entry and emit it through `onMultiSelectionChanged`, dragging the previously-viewed row into bulk operations the user never explicitly selected. Track the active/focused row in a separate `activeRowId` state that drives visual highlight via `TableRowMultiSelectable.isSelected`, while leaving `selectedRowIds` untouched on row-body click. The public API stays identical. Adds a regression test covering the ARTESCA-8467 reproduction. Issue: CUI-22
1 parent 13bff0f commit 1faa2ac

2 files changed

Lines changed: 131 additions & 3 deletions

File tree

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
import { Table, TableProps } from './Tablev2.component';
2+
import { fireEvent, render, screen, waitFor, within } from '@testing-library/react';
3+
import { ThemeProvider } from 'styled-components';
4+
import { coreUIAvailableThemes } from '../../style/theme';
5+
6+
jest.mock('./TableUtils', () => ({
7+
...jest.requireActual('./TableUtils'),
8+
// since convertRemToPixels rely on getComputedStyle(document.documentElement) which is not available in jest
9+
// we mock it
10+
convertRemToPixels: () => 12,
11+
}));
12+
13+
jest.mock('react-virtualized-auto-sizer', () => ({ children }) => {
14+
return children({
15+
height: 600,
16+
width: 600,
17+
});
18+
});
19+
20+
const data = [
21+
{ firstName: 'Sotiria', lastName: 'Agathangelou', age: 90 },
22+
{ firstName: 'Stefania', lastName: 'Evgenios', age: 27 },
23+
{ firstName: 'Yohann', lastName: 'Rodolph', age: 27 },
24+
{ firstName: 'Ninette', lastName: 'Caroline', age: 31 },
25+
];
26+
27+
const columns: TableProps['columns'] = [
28+
{ Header: 'First Name', accessor: 'firstName' },
29+
{ Header: 'Last Name', accessor: 'lastName' },
30+
{ Header: 'Age', accessor: 'age' },
31+
];
32+
33+
const renderMultiSelectTable = (
34+
props: {
35+
onMultiSelectionChanged?: jest.Mock;
36+
onSingleRowSelected?: jest.Mock;
37+
} = {},
38+
) =>
39+
render(
40+
<ThemeProvider theme={coreUIAvailableThemes.artescaLight}>
41+
<Table columns={columns} data={data} defaultSortingKey="firstName">
42+
<Table.MultiSelectableContent
43+
rowHeight="h40"
44+
separationLineVariant="backgroundLevel3"
45+
{...props}
46+
/>
47+
</Table>
48+
</ThemeProvider>,
49+
);
50+
51+
describe('MultiSelectableContent', () => {
52+
it('reports only checkbox-selected rows in onMultiSelectionChanged', async () => {
53+
const onMultiSelectionChanged = jest.fn();
54+
renderMultiSelectTable({ onMultiSelectionChanged });
55+
56+
await waitFor(() => screen.queryAllByRole('img', { hidden: true }));
57+
58+
const rows = screen.getAllByRole('row');
59+
const targetRow = rows[1];
60+
const checkbox = within(targetRow).getByRole('checkbox');
61+
62+
fireEvent.click(checkbox);
63+
64+
expect(onMultiSelectionChanged).toHaveBeenCalled();
65+
const lastCallRows = onMultiSelectionChanged.mock.calls.at(-1)![0];
66+
expect(lastCallRows).toHaveLength(1);
67+
expect(lastCallRows[0].original).toEqual(data[3]); // Ninette (firstName-sorted index 0)
68+
});
69+
70+
it('does not include a previously single-clicked row in subsequent multi-selection (ARTESCA-8467)', async () => {
71+
const onSingleRowSelected = jest.fn();
72+
const onMultiSelectionChanged = jest.fn();
73+
renderMultiSelectTable({ onSingleRowSelected, onMultiSelectionChanged });
74+
75+
await waitFor(() => screen.queryAllByRole('img', { hidden: true }));
76+
77+
// Skip the header (rows[0]) — data rows are rows[1..4] sorted by firstName:
78+
// Ninette, Sotiria, Stefania, Yohann.
79+
80+
// Simulate clicking the row body (not the checkbox) to trigger the
81+
// "view details" path. Click a data cell within the row.
82+
fireEvent.click(screen.getByText('Ninette'));
83+
84+
expect(onSingleRowSelected).toHaveBeenCalledTimes(1);
85+
expect(onSingleRowSelected.mock.calls[0][0].original).toEqual(data[3]);
86+
87+
// Re-fetch the rows because RenderRow is memoized inside the parent and
88+
// remounts on every parent render. The viewed row's checkbox must stay
89+
// unchecked — this is the contract that prevents stale rows from leaking
90+
// into multi-selection.
91+
const viewedRow = screen.getAllByRole('row')[1];
92+
expect(within(viewedRow).getByRole('checkbox')).not.toBeChecked();
93+
94+
// Now check a different row's checkbox.
95+
const checkboxRow = screen.getAllByRole('row')[3]; // Stefania
96+
fireEvent.click(within(checkboxRow).getByRole('checkbox'));
97+
98+
expect(onMultiSelectionChanged).toHaveBeenCalled();
99+
const lastCallRows = onMultiSelectionChanged.mock.calls.at(-1)![0];
100+
expect(lastCallRows).toHaveLength(1);
101+
expect(lastCallRows[0].original).toEqual(data[1]); // Stefania
102+
});
103+
104+
it('keeps the active row checkbox unchecked across subsequent checkbox clicks', async () => {
105+
const onSingleRowSelected = jest.fn();
106+
const onMultiSelectionChanged = jest.fn();
107+
renderMultiSelectTable({ onSingleRowSelected, onMultiSelectionChanged });
108+
109+
await waitFor(() => screen.queryAllByRole('img', { hidden: true }));
110+
111+
fireEvent.click(screen.getByText('Ninette'));
112+
113+
const otherCheckboxRow = screen.getAllByRole('row')[3];
114+
fireEvent.click(within(otherCheckboxRow).getByRole('checkbox'));
115+
116+
// The active row's checkbox stays unchecked even after another row's
117+
// checkbox click — visual highlight on the active row is driven by the
118+
// `isSelected` prop on the styled-component (covered in storybook).
119+
const activeRow = screen.getAllByRole('row')[1];
120+
expect(within(activeRow).getByRole('checkbox')).not.toBeChecked();
121+
expect(within(screen.getAllByRole('row')[3]).getByRole('checkbox')).toBeChecked();
122+
});
123+
});

src/lib/components/tablev2/MultiSelectableContent.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useEffect, memo, CSSProperties } from 'react';
1+
import { useEffect, useState, memo, CSSProperties } from 'react';
22
import { Row } from 'react-table';
33
import { areEqual } from 'react-window';
44
import { useTableContext } from './Tablev2.component';
@@ -86,6 +86,11 @@ export const MultiSelectableContent = <
8686
});
8787
}, [setHiddenColumns]);
8888

89+
// Tracks the row most recently activated via `onSingleRowSelected` (e.g. for
90+
// a detail panel). Kept separate from react-table's `selectedRowIds` so that
91+
// viewing a row does not mark it as checkbox-selected for bulk operations.
92+
const [activeRowId, setActiveRowId] = useState<string | null>(null);
93+
8994
const handleMultipleSelectedRows = (
9095
selectedRowIds,
9196
rows,
@@ -136,15 +141,15 @@ export const MultiSelectableContent = <
136141
? () => {
137142
onSingleRowSelected(row);
138143
toggleAllRowsSelected(false);
139-
row.toggleRowSelected(true);
144+
setActiveRowId(row.id);
140145
}
141146
: () => handleMultipleSelectedRows(selectedRowIds, rows, row, index),
142147
};
143148

144149
return (
145150
<TableRowMultiSelectable
146151
{...rowProps}
147-
isSelected={row.isSelected}
152+
isSelected={row.isSelected || activeRowId === row.id}
148153
separationLineVariant={separationLineVariant}
149154
className="tr"
150155
>

0 commit comments

Comments
 (0)