Skip to content

Commit f6cc295

Browse files
bert-ebert-e
committed
Merge branch 'improvement/fix-tablev2-row-remount-on-rerender' into q/1.0
2 parents e270b79 + fa5bfc8 commit f6cc295

4 files changed

Lines changed: 209 additions & 140 deletions

File tree

‎src/lib/components/tablev2/MultiSelectableContent.tsx‎

Lines changed: 107 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import { useEffect, useState, memo, CSSProperties } from 'react';
1+
import { useEffect, useState, memo, useMemo, useRef } from 'react';
22
import { Row } from 'react-table';
3-
import { areEqual } from 'react-window';
3+
import { areEqual, ListChildComponentProps } from 'react-window';
44
import { useTableContext } from './Tablev2.component';
55
import {
66
HeadRow,
@@ -14,7 +14,7 @@ import {
1414
TableLocalType,
1515
TableVariantType,
1616
} from './TableUtils';
17-
import { RenderRowType, TableRows, useTableScrollbar } from './TableCommon';
17+
import { TableRows, useTableScrollbar } from './TableCommon';
1818
import useSyncedScroll from './useSyncedScroll';
1919
import { Box } from '../box/Box';
2020
import { Loader } from '../loader/Loader.component';
@@ -124,79 +124,114 @@ export const MultiSelectableContent = <
124124

125125
const { headerRef } = useSyncedScroll<DATA_ROW>();
126126

127-
const RenderRow = memo(({ index, style }: RenderRowType) => {
128-
const row = rows[index];
129-
prepareRow(row);
127+
/**
128+
* These values change identity on (almost) every render. We read them through refs so the row
129+
* renderer below can keep a stable identity (see RenderRow).
130+
*/
131+
const prepareRowRef = useRef(prepareRow);
132+
prepareRowRef.current = prepareRow;
133+
const selectedRowIdsRef = useRef(selectedRowIds);
134+
selectedRowIdsRef.current = selectedRowIds;
135+
const onSingleRowSelectedRef = useRef(onSingleRowSelected);
136+
onSingleRowSelectedRef.current = onSingleRowSelected;
137+
const toggleAllRowsSelectedRef = useRef(toggleAllRowsSelected);
138+
toggleAllRowsSelectedRef.current = toggleAllRowsSelected;
139+
const handleMultipleSelectedRowsRef = useRef(handleMultipleSelectedRows);
140+
handleMultipleSelectedRowsRef.current = handleMultipleSelectedRows;
130141

131-
const rowProps = {
132-
...row.getRowProps({
133-
/**
134-
* Note:We need to pass the style property to the row component.
135-
* Otherwise when we scroll down, the next rows are flashing
136-
* because they are re-rendered in loop.
137-
*/
138-
style: { ...style },
139-
}),
140-
onClick: onSingleRowSelected
141-
? () => {
142-
onSingleRowSelected(row);
143-
toggleAllRowsSelected(false);
144-
setActiveRowId(row.id);
145-
}
146-
: () => handleMultipleSelectedRows(selectedRowIds, rows, row, index),
147-
};
142+
/**
143+
* RenderRow MUST keep a stable identity across re-renders. It used to be redefined inline on
144+
* every render, so react-window saw a new component type each time and remounted (not just
145+
* re-rendered) every row — and therefore every cell — whenever the table re-rendered for any
146+
* reason. That made async cell content reload and flash. We now read the row from react-window's
147+
* `data` (itemData) prop and the volatile callbacks/state from refs, so the component is only
148+
* recreated when something that affects the rendered output (activeRowId / separationLineVariant)
149+
* actually changes. Checkbox selection still updates because react-table rebuilds `rows` (and
150+
* therefore `data`) when `selectedRowIds` changes.
151+
*/
152+
const RenderRow = useMemo(
153+
() =>
154+
memo(({ index, style, data }: ListChildComponentProps<Row<DATA_ROW>[]>) => {
155+
const rows = data;
156+
const row = data[index];
157+
prepareRowRef.current(row);
148158

149-
return (
150-
<TableRowMultiSelectable
151-
{...rowProps}
152-
isSelected={row.isSelected || activeRowId === row.id}
153-
separationLineVariant={separationLineVariant}
154-
className="tr"
155-
>
156-
{row.cells.map((cell) => {
157-
const cellProps = cell.getCellProps({
158-
style: {
159-
...cell.column.cellStyle,
160-
// Vertically center the text in cells.
161-
display: 'flex',
162-
flexDirection: 'column',
163-
justifyContent: 'center',
164-
},
165-
role: 'gridcell',
166-
});
159+
const rowProps = {
160+
...row.getRowProps({
161+
/**
162+
* Note:We need to pass the style property to the row component.
163+
* Otherwise when we scroll down, the next rows are flashing
164+
* because they are re-rendered in loop.
165+
*/
166+
style: { ...style },
167+
}),
168+
onClick: () => {
169+
const onSingleRowSelected = onSingleRowSelectedRef.current;
170+
if (onSingleRowSelected) {
171+
onSingleRowSelected(row);
172+
toggleAllRowsSelectedRef.current(false);
173+
setActiveRowId(row.id);
174+
} else {
175+
handleMultipleSelectedRowsRef.current(
176+
selectedRowIdsRef.current,
177+
rows,
178+
row,
179+
index,
180+
);
181+
}
182+
},
183+
};
167184

168-
if (cell.column.id === 'selection') {
169-
return (
170-
<div
171-
{...cellProps}
172-
onClick={
173-
onSingleRowSelected
174-
? (event) => {
175-
event.stopPropagation();
176-
handleMultipleSelectedRows(
177-
selectedRowIds,
178-
rows,
179-
row,
180-
index,
181-
);
182-
}
183-
: undefined
184-
}
185-
>
186-
{cell.render('Cell')}
187-
</div>
188-
);
189-
}
185+
return (
186+
<TableRowMultiSelectable
187+
{...rowProps}
188+
isSelected={row.isSelected || activeRowId === row.id}
189+
separationLineVariant={separationLineVariant}
190+
className="tr"
191+
>
192+
{row.cells.map((cell) => {
193+
const cellProps = cell.getCellProps({
194+
style: {
195+
...cell.column.cellStyle,
196+
// Vertically center the text in cells.
197+
display: 'flex',
198+
flexDirection: 'column',
199+
justifyContent: 'center',
200+
},
201+
role: 'gridcell',
202+
});
203+
204+
if (cell.column.id === 'selection') {
205+
return (
206+
<div
207+
{...cellProps}
208+
onClick={(event) => {
209+
if (!onSingleRowSelectedRef.current) return;
210+
event.stopPropagation();
211+
handleMultipleSelectedRowsRef.current(
212+
selectedRowIdsRef.current,
213+
rows,
214+
row,
215+
index,
216+
);
217+
}}
218+
>
219+
{cell.render('Cell')}
220+
</div>
221+
);
222+
}
190223

191-
return (
192-
<div {...cellProps} className="td">
193-
{cell.render('Cell')}
194-
</div>
195-
);
196-
})}
197-
</TableRowMultiSelectable>
198-
);
199-
}, areEqual);
224+
return (
225+
<div {...cellProps} className="td">
226+
{cell.render('Cell')}
227+
</div>
228+
);
229+
})}
230+
</TableRowMultiSelectable>
231+
);
232+
}, areEqual),
233+
[activeRowId, separationLineVariant],
234+
);
200235

201236
return (
202237
<>

‎src/lib/components/tablev2/SingleSelectableContent.tsx‎

Lines changed: 89 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { memo, useEffect, useRef } from 'react';
2-
import { areEqual, FixedSizeList } from 'react-window';
1+
import { memo, useEffect, useMemo, useRef } from 'react';
2+
import { areEqual, FixedSizeList, ListChildComponentProps } from 'react-window';
33
import { Row } from 'react-table';
44
import { useTableContext } from './Tablev2.component';
55
import {
@@ -14,7 +14,7 @@ import {
1414
TableLocalType,
1515
TableVariantType,
1616
} from './TableUtils';
17-
import { RenderRowType, TableRows, useTableScrollbar } from './TableCommon';
17+
import { TableRows, useTableScrollbar } from './TableCommon';
1818
import useSyncedScroll from './useSyncedScroll';
1919
import { Loader } from '../loader/Loader.component';
2020
import { Box } from '../box/Box';
@@ -77,69 +77,93 @@ export function SingleSelectableContent<
7777
return () => clearTimeout(timer);
7878
}, [autoScrollToSelected, selectedId, rows]);
7979

80-
const RenderRow = memo(({ index, style }: RenderRowType) => {
81-
const row = rows[index];
82-
prepareRow(row);
83-
let rowProps = row.getRowProps({
84-
/**
85-
* Note: We need to pass the style property to the row component.
86-
* Otherwise when we scroll down, the next rows are flashing
87-
* because they are re-rendered in loop.
88-
*/
89-
style: { ...style },
90-
});
91-
92-
rowProps = {
93-
...rowProps,
94-
...{
95-
onClick: () => {
96-
if (onRowSelected) return onRowSelected(row);
97-
},
98-
tabIndex: onRowSelected ? 0 : undefined,
99-
onKeyDown: (event) => {
100-
if (
101-
onRowSelected &&
102-
(event.key === ' ' ||
103-
event.key === 'Enter' ||
104-
event.key === 'Spacebar')
105-
) {
106-
event.preventDefault();
107-
onRowSelected(row);
108-
}
109-
},
110-
},
111-
};
112-
113-
return (
114-
<TableRow
115-
{...rowProps}
116-
isSelected={selectedId === row.id}
117-
aria-selected={selectedId === row.id ? 'true' : 'false'}
118-
separationLineVariant={separationLineVariant}
119-
selectedId={selectedId}
120-
className="tr"
121-
>
122-
{row.cells.map((cell) => {
123-
let cellProps = cell.getCellProps({
124-
style: {
125-
...cell.column.cellStyle,
126-
// Vertically center the text in cells.
127-
display: 'flex',
128-
flexDirection: 'column',
129-
justifyContent: 'center',
80+
/**
81+
* `prepareRow` and `onRowSelected` change identity on every render. We read them through refs
82+
* so the row renderer below can keep a stable identity (see RenderRow).
83+
*/
84+
const prepareRowRef = useRef(prepareRow);
85+
prepareRowRef.current = prepareRow;
86+
const onRowSelectedRef = useRef(onRowSelected);
87+
onRowSelectedRef.current = onRowSelected;
88+
89+
/**
90+
* RenderRow MUST keep a stable identity across re-renders. It used to be redefined inline on
91+
* every render, so react-window saw a new component type each time and remounted (not just
92+
* re-rendered) every row — and therefore every cell — whenever the table re-rendered for any
93+
* reason. That made async cell content reload and flash. We now read the row from react-window's
94+
* `data` (itemData) prop and the volatile callbacks from refs, so the component only needs to be
95+
* recreated when something that affects the rendered output (selectedId / separationLineVariant)
96+
* actually changes.
97+
*/
98+
const RenderRow = useMemo(
99+
() =>
100+
memo(({ index, style, data }: ListChildComponentProps<Row<DATA_ROW>[]>) => {
101+
const row = data[index];
102+
prepareRowRef.current(row);
103+
let rowProps = row.getRowProps({
104+
/**
105+
* Note: We need to pass the style property to the row component.
106+
* Otherwise when we scroll down, the next rows are flashing
107+
* because they are re-rendered in loop.
108+
*/
109+
style: { ...style },
110+
});
111+
112+
rowProps = {
113+
...rowProps,
114+
...{
115+
onClick: () => {
116+
const onRowSelected = onRowSelectedRef.current;
117+
if (onRowSelected) return onRowSelected(row);
130118
},
131-
role: 'gridcell',
132-
});
133-
134-
return (
135-
<div {...cellProps} className="td">
136-
{cell.render('Cell')}
137-
</div>
138-
);
139-
})}
140-
</TableRow>
141-
);
142-
}, areEqual);
119+
tabIndex: onRowSelectedRef.current ? 0 : undefined,
120+
onKeyDown: (event) => {
121+
const onRowSelected = onRowSelectedRef.current;
122+
if (
123+
onRowSelected &&
124+
(event.key === ' ' ||
125+
event.key === 'Enter' ||
126+
event.key === 'Spacebar')
127+
) {
128+
event.preventDefault();
129+
onRowSelected(row);
130+
}
131+
},
132+
},
133+
};
134+
135+
return (
136+
<TableRow
137+
{...rowProps}
138+
isSelected={selectedId === row.id}
139+
aria-selected={selectedId === row.id ? 'true' : 'false'}
140+
separationLineVariant={separationLineVariant}
141+
selectedId={selectedId}
142+
className="tr"
143+
>
144+
{row.cells.map((cell) => {
145+
let cellProps = cell.getCellProps({
146+
style: {
147+
...cell.column.cellStyle,
148+
// Vertically center the text in cells.
149+
display: 'flex',
150+
flexDirection: 'column',
151+
justifyContent: 'center',
152+
},
153+
role: 'gridcell',
154+
});
155+
156+
return (
157+
<div {...cellProps} className="td">
158+
{cell.render('Cell')}
159+
</div>
160+
);
161+
})}
162+
</TableRow>
163+
);
164+
}, areEqual),
165+
[selectedId, separationLineVariant],
166+
);
143167

144168
const { hasScrollbar, scrollBarWidth, handleScrollbarWidth } =
145169
useTableScrollbar();

‎src/lib/components/tablev2/TableCommon.tsx‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,7 @@ type TableRowsProps<
128128
locale?: TableLocalType;
129129
children?: (children: JSX.Element) => JSX.Element;
130130
customItemKey?: (index: number, data: DATA_ROW) => string;
131-
RenderRow: React.MemoExoticComponent<
132-
({ index, style }: RenderRowType) => JSX.Element
133-
>;
131+
RenderRow: ComponentType<ListChildComponentProps<Row<DATA_ROW>[]>>;
134132
listRef?: Ref<FixedSizeList<Row<DATA_ROW>[]>>;
135133
};
136134
export function TableRows<

0 commit comments

Comments
 (0)