Skip to content

Commit 07ef7e4

Browse files
authored
fix(react): preserve scroll authority across anchored rebuilds (#543)
* fix(react): preserve controller scroll authority * fix(react): synchronize anchored scroll authority
1 parent ff9332c commit 07ef7e4

9 files changed

Lines changed: 373 additions & 97 deletions

File tree

.changeset/scroll-authority.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
"@pretable/react": patch
3+
---
4+
5+
Preserve row-layout scroll anchoring across controller rebuilds. Grid/DOM
6+
viewport changes now enter the layout controller as explicit inputs, while
7+
anchor-adjusted controller publications flow back through grid state to the
8+
real scroll element without re-feeding the stale pre-anchor offset.

packages/react/src/__tests__/page-key-navigation.test.tsx

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,14 @@ describe("a page key while the cursor's row is evicted", () => {
438438
expect(
439439
view.container.querySelector('[data-pretable-row-id="row-30"]'),
440440
).not.toBeNull();
441+
const viewport = view.container.querySelector<HTMLElement>(
442+
"[data-pretable-scroll-viewport]",
443+
);
444+
if (viewport === null) throw new Error("no scroll viewport");
445+
// The controller anchored the replacement window in global coordinates;
446+
// its output must reach the real scroller before the DOM can publish
447+
// another input. This is the integration half of the #524 authority seam.
448+
expect(viewport.scrollTop).toBeGreaterThan(0);
441449

442450
return { view, changes, seen, cursor };
443451
}
@@ -448,6 +456,16 @@ describe("a page key while the cursor's row is evicted", () => {
448456
seen: PretableSelectionState[],
449457
cursor: string,
450458
) {
459+
// Window 30 was reached by an anchored controller publication, which now
460+
// correctly moves the real viewport to that window's global offset. A
461+
// server-window consumer only loads window 0 again after the user scrolls
462+
// back there, so model that input explicitly instead of relying on the old
463+
// stale-DOM re-feed to reset the controller as a side effect.
464+
const viewport = view.container.querySelector<HTMLElement>(
465+
"[data-pretable-scroll-viewport]",
466+
);
467+
if (viewport === null) throw new Error("no scroll viewport");
468+
fireEvent.scroll(viewport, { target: { scrollTop: 0 } });
451469
view.rerender(
452470
<WindowedGrid
453471
windowStart={0}
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
// @vitest-environment jsdom
2+
import { act, cleanup, renderHook } from "@testing-library/react";
3+
import { afterEach, describe, expect, test, vi } from "vitest";
4+
5+
import { createColumnHelper, createLocalRowModel } from "@pretable/core";
6+
import type { RowLayoutController } from "@pretable-internal/renderer-dom";
7+
8+
type Row = { id: number; label: string };
9+
10+
const column = createColumnHelper<Row>();
11+
const columns = [
12+
column.accessor("label", { type: "text", wrap: true, widthPx: 180 }),
13+
] as const;
14+
15+
type Controller = RowLayoutController<Row, number, typeof columns>;
16+
17+
const controllerRecords: Array<{
18+
readonly controller: Controller;
19+
readonly setViewport: ReturnType<typeof vi.fn<Controller["setViewport"]>>;
20+
}> = [];
21+
22+
vi.mock("@pretable-internal/renderer-dom", async (importOriginal) => {
23+
const actual =
24+
await importOriginal<typeof import("@pretable-internal/renderer-dom")>();
25+
const createRowLayoutController: typeof actual.createRowLayoutController = (
26+
options,
27+
) => {
28+
const controller = actual.createRowLayoutController(options);
29+
const setViewport = vi.fn(controller.setViewport);
30+
const wrapped = { ...controller, setViewport };
31+
controllerRecords.push({
32+
controller: wrapped as unknown as Controller,
33+
setViewport: setViewport as unknown as ReturnType<
34+
typeof vi.fn<Controller["setViewport"]>
35+
>,
36+
});
37+
return wrapped;
38+
};
39+
return { ...actual, createRowLayoutController };
40+
});
41+
42+
const { usePretable } = await import("../use-pretable");
43+
44+
afterEach(() => {
45+
cleanup();
46+
controllerRecords.length = 0;
47+
vi.clearAllMocks();
48+
});
49+
50+
describe("scroll authority", () => {
51+
test("a controller rebuild does not re-feed an unchanged grid viewport", async () => {
52+
const rows = Array.from({ length: 2_000 }, (_, id): Row => ({
53+
id,
54+
label: `row ${id}`,
55+
}));
56+
const model = createLocalRowModel({ rows, columns });
57+
const view = renderHook(() =>
58+
usePretable({ model, overscan: 0, viewportHeight: 120 }),
59+
);
60+
61+
expect(controllerRecords).toHaveLength(1);
62+
const { controller, setViewport } = controllerRecords[0]!;
63+
await expect.poll(() => controller.getState().status.kind).toBe("ready");
64+
65+
act(() => {
66+
view.result.current.grid.setViewport({
67+
scrollTop: 800,
68+
scrollLeft: 0,
69+
height: 120,
70+
width: 600,
71+
});
72+
});
73+
await expect.poll(() => controller.getState().scrollTop).toBe(800);
74+
setViewport.mockClear();
75+
76+
await act(async () => {
77+
model.setRows(
78+
rows.map((_, offset): Row => ({
79+
id: rows.length + offset,
80+
label: `replacement ${offset}`,
81+
})),
82+
);
83+
expect(controller.getState().status.kind).toBe("rebuilding");
84+
await expect.poll(() => controller.getState().status.kind).toBe("ready");
85+
});
86+
87+
expect(setViewport).not.toHaveBeenCalled();
88+
89+
view.unmount();
90+
model.dispose();
91+
});
92+
});

packages/react/src/pretable-model.ts

Lines changed: 81 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
createRowLayoutController,
44
type DomLayoutColumn,
55
} from "@pretable-internal/renderer-dom";
6+
import { ɵqueriesSemanticallyEqual } from "@pretable-internal/row-model/query-equality";
67
import {
78
ɵcreateGridUiCore as createGridUiCore,
89
type PretableGridUiState,
@@ -1038,21 +1039,93 @@ export function usePretableModelInternal<
10381039
previousPresentationColumns.current = columns;
10391040
}, [columns, stores.autoWidths, stores.gridCore]);
10401041

1042+
const viewportAuthorityRef = useRef<{
1043+
readonly gridScrollTop: number;
1044+
readonly controllerScrollTop: number;
1045+
readonly viewportHeight: number;
1046+
readonly overscan: number;
1047+
readonly observedQuery: typeof observedQuery;
1048+
readonly renderColumns: typeof renderColumns;
1049+
} | null>(null);
1050+
const controllerScrollTop = renderControllerSnapshot.scrollTop;
1051+
const viewportOverscan = options.overscan ?? 6;
10411052
useLayoutEffect(() => {
1042-
if (renderControllerSnapshot.status.kind === "disposed") return;
1043-
stores.controller.setColumns(renderColumns);
1044-
stores.controller.setViewport({
1045-
scrollTop: gridSnapshot.viewport.scrollTop,
1053+
if (stores.controller.getState().status.kind === "disposed") return;
1054+
1055+
const previous = viewportAuthorityRef.current;
1056+
const gridChanged =
1057+
previous === null ||
1058+
previous.gridScrollTop !== gridSnapshot.viewport.scrollTop;
1059+
const viewportShapeChanged =
1060+
previous === null ||
1061+
previous.viewportHeight !== gridSnapshot.viewport.height ||
1062+
previous.overscan !== viewportOverscan;
1063+
const queryChanged =
1064+
previous !== null &&
1065+
!ɵqueriesSemanticallyEqual(previous.observedQuery, observedQuery);
1066+
const columnsChanged =
1067+
previous === null || previous.renderColumns !== renderColumns;
1068+
const controllerChanged =
1069+
previous !== null && previous.controllerScrollTop !== controllerScrollTop;
1070+
1071+
// Record the committed pair before publishing either side. A synchronous
1072+
// external-store notification can render immediately, and that render
1073+
// must classify the publication as the echo of this decision rather than
1074+
// as a second source of authority.
1075+
viewportAuthorityRef.current = {
1076+
gridScrollTop: gridSnapshot.viewport.scrollTop,
1077+
controllerScrollTop,
10461078
viewportHeight: gridSnapshot.viewport.height,
1047-
overscan: options.overscan ?? 6,
1048-
});
1079+
overscan: viewportOverscan,
1080+
observedQuery,
1081+
renderColumns,
1082+
};
1083+
1084+
if (columnsChanged) {
1085+
stores.controller.setColumns(renderColumns);
1086+
}
1087+
1088+
if (gridChanged || viewportShapeChanged || queryChanged) {
1089+
// A real grid/DOM change is an input. When it races an anchored
1090+
// controller publication, the newer external input deliberately wins.
1091+
// A semantic query change also starts from the grid's position: sort,
1092+
// filter and grouping transitions keep the user's DOM offset rather
1093+
// than following an old row to its new dataset rank.
1094+
stores.controller.setViewport({
1095+
scrollTop: gridSnapshot.viewport.scrollTop,
1096+
viewportHeight: gridSnapshot.viewport.height,
1097+
overscan: viewportOverscan,
1098+
});
1099+
return;
1100+
}
1101+
1102+
if (
1103+
controllerChanged &&
1104+
controllerScrollTop !== gridSnapshot.viewport.scrollTop
1105+
) {
1106+
// Anchor restoration is a controller output, not permission to re-feed
1107+
// the grid's previous offset. Publish it outward so the grid and DOM
1108+
// converge on the controller before another viewport input is possible.
1109+
const viewport = stores.gridCore.getState().viewport;
1110+
viewportAuthorityRef.current = {
1111+
...viewportAuthorityRef.current,
1112+
gridScrollTop: controllerScrollTop,
1113+
controllerScrollTop,
1114+
};
1115+
stores.gridCore.setViewport({
1116+
...viewport,
1117+
scrollTop: controllerScrollTop,
1118+
});
1119+
}
10491120
}, [
1121+
controllerScrollTop,
10501122
gridSnapshot.viewport.height,
10511123
gridSnapshot.viewport.scrollTop,
1052-
options.overscan,
1124+
observedQuery,
10531125
renderColumns,
1054-
renderControllerSnapshot.status.kind,
10551126
stores.controller,
1127+
stores.gridCore,
1128+
viewportOverscan,
10561129
]);
10571130

10581131
const observedRevision = renderControllerSnapshot.observedRevision;

packages/react/src/pretable-surface.tsx

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2825,6 +2825,22 @@ export function PretableSurface<
28252825
};
28262826
}, [indexedSnapshot, rowModelSnapshot]);
28272827

2828+
// The DOM is the source of user scroll INPUT, but it is not the only writer:
2829+
// row anchoring can legitimately adjust the controller's global offset
2830+
// after content above the viewport changes. `usePretable` publishes that
2831+
// adjustment into the grid snapshot; apply it to the actual scroller before
2832+
// paint so its next scroll event cannot reassert the stale pre-anchor value.
2833+
useLayoutEffect(() => {
2834+
const viewport = viewportRef.current;
2835+
if (viewport === null) return;
2836+
if (viewport.scrollTop !== snapshot.viewport.scrollTop) {
2837+
viewport.scrollTop = snapshot.viewport.scrollTop;
2838+
}
2839+
if (viewport.scrollLeft !== snapshot.viewport.scrollLeft) {
2840+
viewport.scrollLeft = snapshot.viewport.scrollLeft;
2841+
}
2842+
}, [snapshot.viewport.scrollLeft, snapshot.viewport.scrollTop]);
2843+
28282844
const surfaceContextRef = useRef({
28292845
snapshot,
28302846
rowModelSnapshot,

packages/react/vitest.config.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ export default defineConfig({
3838
__dirname,
3939
"../renderer-dom/src/index.ts",
4040
),
41+
"@pretable-internal/row-model/query-equality": resolve(
42+
__dirname,
43+
"../row-model/src/query-equality.ts",
44+
),
4145
"@pretable-internal/row-model": resolve(
4246
__dirname,
4347
"../row-model/src/index.ts",

packages/row-model/package.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@
1919
"./diagnostics": {
2020
"types": "./dist/diagnostics.d.ts",
2121
"import": "./dist/diagnostics.js"
22+
},
23+
"./query-equality": {
24+
"types": "./dist/query-equality.d.ts",
25+
"import": "./dist/query-equality.js"
2226
}
2327
},
2428
"scripts": {

0 commit comments

Comments
 (0)