Skip to content

Commit 5223a2d

Browse files
bvaughnbvaughn
committed
Fix potential scroll jumping when using dynamic row heights
1 parent f3a5aa7 commit 5223a2d

File tree

4 files changed

+139
-130
lines changed

4 files changed

+139
-130
lines changed

‎lib/components/list/useDynamicRowHeight.test.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,14 @@ describe("useDynamicRowHeight", () => {
6363
});
6464

6565
describe("getRowHeight", () => {
66-
test("returns undefined for a row that has not yet been measured", () => {
66+
test("returns estimated height for a row that has not yet been measured", () => {
6767
const { result } = renderHook(() =>
6868
useDynamicRowHeight({
6969
defaultRowHeight: 100
7070
})
7171
);
7272

73-
expect(result.current.getRowHeight(0)).toBeUndefined();
73+
expect(result.current.getRowHeight(0)).toBe(100);
7474
});
7575

7676
test("returns the most recently measured size", () => {
@@ -87,7 +87,7 @@ describe("useDynamicRowHeight", () => {
8787
});
8888
expect(result.current.getRowHeight(0)).toBe(15);
8989
expect(result.current.getRowHeight(1)).toBe(20);
90-
expect(result.current.getRowHeight(2)).toBeUndefined();
90+
expect(result.current.getRowHeight(2)).toBe(100);
9191
expect(result.current.getRowHeight(3)).toBe(25);
9292

9393
act(() => {
@@ -113,7 +113,7 @@ describe("useDynamicRowHeight", () => {
113113
expect(result.current.getRowHeight(0)).toBe(10);
114114

115115
rerender("b");
116-
expect(result.current.getRowHeight(0)).toBeUndefined();
116+
expect(result.current.getRowHeight(0)).toBe(100);
117117
});
118118
});
119119

@@ -137,8 +137,8 @@ describe("useDynamicRowHeight", () => {
137137
act(() => {
138138
result.current.observeRowElements([elementA, elementB]);
139139
});
140-
expect(result.current.getRowHeight(0)).toBeUndefined();
141-
expect(result.current.getRowHeight(1)).toBeUndefined();
140+
expect(result.current.getRowHeight(0)).toBe(100);
141+
expect(result.current.getRowHeight(1)).toBe(100);
142142

143143
act(() => {
144144
setElementSize({
@@ -147,7 +147,7 @@ describe("useDynamicRowHeight", () => {
147147
height: 20
148148
});
149149
});
150-
expect(result.current.getRowHeight(0)).toBeUndefined();
150+
expect(result.current.getRowHeight(0)).toBe(100);
151151
expect(result.current.getRowHeight(1)).toBe(20);
152152

153153
act(() => {

‎lib/components/list/useDynamicRowHeight.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,19 @@ export function useDynamicRowHeight({
4444

4545
const getRowHeight = useCallback(
4646
(index: number) => {
47-
return map.get(index);
47+
const measuredHeight = map.get(index);
48+
if (measuredHeight !== undefined) {
49+
return measuredHeight;
50+
}
51+
52+
// Temporarily store default height in the cache map to avoid scroll jumps if rowProps change
53+
// Else rowProps changes can impact the average height, and cause rows to shift up or down within the list
54+
// see github.com/bvaughn/react-window/issues/863
55+
map.set(index, defaultRowHeight);
56+
57+
return defaultRowHeight;
4858
},
49-
[map]
59+
[defaultRowHeight, map]
5060
);
5161

5262
const setRowHeight = useCallback((index: number, size: number) => {

‎lib/hooks/useStableCallback.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ export function useStableCallback<Args, Return>(
66
fn: (args: Args) => Return
77
): (args: Args) => Return {
88
const ref = useRef<typeof fn>(() => {
9-
throw new Error("Cannot call an event handler while rendering.");
9+
throw new Error("Cannot call during render.");
1010
});
1111

1212
useIsomorphicLayoutEffect(() => {

‎src/routes/ScratchpadRoute.tsx‎

Lines changed: 119 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -1,140 +1,139 @@
1-
import { useState } from "react";
1+
import { useCallback, useState, type ButtonHTMLAttributes } from "react";
22
import {
3-
Grid,
4-
useGridCallbackRef,
5-
type Align,
6-
type CellComponentProps
3+
List,
4+
useDynamicRowHeight,
5+
useListCallbackRef,
6+
type RowComponentProps
77
} from "react-window";
8-
import { Block } from "../components/Block";
9-
import { Box } from "../components/Box";
10-
import { Checkbox } from "../components/Checkbox";
11-
import { Input } from "../components/Input";
12-
import { Select, type Option } from "../components/Select";
13-
import { cn } from "../utils/cn";
148

15-
const ALIGNMENTS: Option<Align>[] = (
16-
["auto", "center", "end", "smart", "start"] satisfies Align[]
17-
).map((value) => ({
18-
label: `align: ${value}`,
19-
value
20-
}));
9+
type Item = {
10+
children: Array<{ index: number }>;
11+
id: number;
12+
minHeight: number;
13+
name: string;
14+
};
2115

2216
export default function ScratchpadRoute() {
23-
const [rtl, setRtl] = useState(false);
24-
const [columnIndex, setColumnIndex] = useState<number | undefined>();
25-
const [rowIndex, setRowIndex] = useState<number | undefined>();
26-
const [gridRef, setGridRef] = useGridCallbackRef(null);
27-
const [align, setAlign] = useState(ALIGNMENTS[0]);
17+
const [key, setKey] = useState(0);
18+
const [items, setItems] = useState(createItems);
19+
const [expandedSet, setExpandedSet] = useState<Set<number>>(new Set());
20+
21+
const [list, setList] = useListCallbackRef();
22+
const rowHeight = useDynamicRowHeight({
23+
defaultRowHeight: 24
24+
});
25+
26+
const toggleExpand = useCallback(
27+
(id: number) => {
28+
const newSet = new Set(expandedSet);
29+
if (newSet.has(id)) {
30+
newSet.delete(id);
31+
} else {
32+
newSet.add(id);
33+
}
34+
setExpandedSet(newSet);
35+
},
36+
[expandedSet]
37+
);
2838

2939
return (
30-
<Box direction="column" gap={4}>
31-
<Box
32-
align="center"
33-
direction="row"
34-
gap={4}
35-
onKeyDown={(event) => {
36-
switch (event.key) {
37-
case "Enter": {
38-
if (columnIndex !== undefined && rowIndex !== undefined) {
39-
gridRef?.scrollToCell({
40-
columnAlign: align.value,
41-
columnIndex,
42-
rowAlign: align.value,
43-
rowIndex
44-
});
45-
} else if (columnIndex !== undefined) {
46-
gridRef?.scrollToColumn({
47-
align: align.value,
48-
index: columnIndex
49-
});
50-
} else if (rowIndex !== undefined) {
51-
gridRef?.scrollToRow({
52-
align: align.value,
53-
index: rowIndex
54-
});
55-
}
56-
break;
57-
}
58-
}
59-
}}
60-
>
61-
<Select
62-
className="flex-1"
63-
onChange={setAlign}
64-
options={ALIGNMENTS}
65-
placeholder="Align"
66-
value={align}
67-
/>
68-
<Checkbox checked={rtl} onChange={setRtl} />
69-
<Input
70-
autoFocus
71-
className="grow"
72-
min={0}
73-
max={99}
74-
onChange={(value) => {
75-
const parsed = parseInt(value);
76-
setColumnIndex(isNaN(parsed) ? undefined : parsed);
77-
}}
78-
placeholder="Column"
79-
step={1}
80-
type="number"
81-
value={columnIndex === undefined ? "" : "" + columnIndex}
82-
/>
83-
<Input
84-
autoFocus
85-
className="grow"
86-
min={0}
87-
max={99}
88-
onChange={(value) => {
89-
const parsed = parseInt(value);
90-
setRowIndex(isNaN(parsed) ? undefined : parsed);
40+
<div className="flex flex-col gap-2">
41+
<div className="flex items-center gap-2">
42+
<Button
43+
onClick={() => {
44+
setItems(createItems());
45+
setExpandedSet(new Set());
46+
setKey(key + 1);
9147
}}
92-
placeholder="Row"
93-
step={1}
94-
type="number"
95-
value={rowIndex === undefined ? "" : "" + rowIndex}
96-
/>
97-
</Box>
98-
<Block className="w-full h-100" data-focus-within="bold">
99-
<Grid
100-
cellComponent={CellComponent}
101-
cellProps={{
102-
focusedColumnIndex: columnIndex,
103-
focusedRowIndex: rowIndex
48+
>
49+
Reset
50+
</Button>
51+
<Button
52+
onClick={() => {
53+
list?.scrollToRow({
54+
align: "end",
55+
index: items.length - 1
56+
});
10457
}}
105-
columnCount={100}
106-
columnWidth={75}
107-
dir={rtl ? "rtl" : undefined}
108-
key={rtl ? "rtl" : "ltr"}
109-
gridRef={setGridRef}
110-
rowCount={100}
111-
rowHeight={35}
112-
/>
113-
</Block>
114-
</Box>
58+
>
59+
Scroll to Bottom
60+
</Button>
61+
</div>
62+
<List
63+
className="w-full h-100 border"
64+
key={key}
65+
listRef={setList}
66+
rowComponent={Row}
67+
rowCount={items.length}
68+
rowHeight={rowHeight}
69+
rowProps={{
70+
expandedSet,
71+
items,
72+
toggleExpand
73+
}}
74+
/>
75+
</div>
11576
);
11677
}
11778

118-
function CellComponent({
119-
columnIndex,
120-
focusedColumnIndex,
121-
focusedRowIndex,
122-
rowIndex,
79+
function Row({
80+
expandedSet,
81+
items,
82+
toggleExpand,
83+
index,
12384
style
124-
}: CellComponentProps<{
125-
focusedColumnIndex: number | undefined;
126-
focusedRowIndex: number | undefined;
85+
}: RowComponentProps<{
86+
expandedSet: Set<number>;
87+
items: Item[];
88+
toggleExpand: (id: number) => void;
12789
}>) {
90+
const { children, minHeight, name } = items[index];
91+
92+
const isExpanded = expandedSet.has(index);
93+
12894
return (
12995
<div
130-
className={cn("flex items-center justify-center text-xs", {
131-
"bg-slate-900": columnIndex % 2 === rowIndex % 2,
132-
"bg-slate-800":
133-
columnIndex === focusedColumnIndex || rowIndex === focusedRowIndex
134-
})}
135-
style={style}
96+
className="py-1 px-2 flex items-center gap-2"
97+
style={{ ...style, minHeight }}
13698
>
137-
row {rowIndex}, col {columnIndex}
99+
<Button disabled={!children.length} onClick={() => toggleExpand(index)}>
100+
{!children.length || isExpanded ? "–" : "+"}
101+
</Button>
102+
{name}
103+
{isExpanded && (
104+
<pre className="text-xs">{JSON.stringify(children, null, 2)}</pre>
105+
)}
138106
</div>
139107
);
140108
}
109+
110+
function createItems() {
111+
const items: Item[] = [];
112+
113+
for (let index = 0; index < 500; ++index) {
114+
items.push({
115+
children: new Array(index % 5).fill(true).map((_, index) => ({
116+
index
117+
})),
118+
id: index,
119+
minHeight: 24 + 5 * (index % 3),
120+
name: `item ${index}`
121+
});
122+
}
123+
124+
return items;
125+
}
126+
127+
function Button({
128+
className,
129+
disabled,
130+
...rest
131+
}: ButtonHTMLAttributes<HTMLButtonElement>) {
132+
return (
133+
<button
134+
className={`rounded bg-gray-700 px-2 ${disabled ? "opacity-35" : "cursor-pointer"} ${className}`}
135+
disabled={disabled}
136+
{...rest}
137+
/>
138+
);
139+
}

0 commit comments

Comments
 (0)