fix(grid): reset scroll to top on external result replacement
Results replaced without an internal grid action (SQL re-executed from the editor or a grid-embedded run button) kept the previous viewport, so DOM-mode grids stayed stranded at the old scroll offset after running an ORDER BY query. The result watch now resets to the first row unless the replacement is an in-place refresh (prepareFullReload markers) or an infinite-scroll append completion. A remounted grid also no longer adopts a scroll-only pending-changes snapshot; cached scroll is only replayed alongside unsaved edit state, while the KeepAlive reactivate path still restores pure scroll. Closes #7341
This commit is contained in:
4 files changed
+173
-1
No files matched your search
@@ -10061,6 +10061,11 @@ watch(
|
||||
() => props.result,
|
||||
(result, previousResult) => {
|
||||
const selectionSnapshot = preservedSelectionOnNextResult?.selection;
|
||||
// The four preserved markers are armed together by prepareFullReload (toolbar
|
||||
// refresh / rollback / row-save reload). Any of them being pending means the
|
||||
// incoming result is an in-place refresh that must keep the viewport, so the
|
||||
// check has to run before the markers are consumed below.
|
||||
const inPlaceRefreshPending = preservedSelectionOnNextResult !== null || preservedViewportAnchorOnNextResult !== null || preservedDetailsOnNextResult !== null || preserveTransposeOnNextResult.value;
|
||||
preservedSelectionOnNextResult = null;
|
||||
const viewportAnchorSnapshot = preservedViewportAnchorOnNextResult?.anchor;
|
||||
preservedViewportAnchorOnNextResult = null;
|
||||
@@ -10087,6 +10092,12 @@ watch(
|
||||
if (getResetScrollAfterResult()) {
|
||||
clearResetScrollAfterResult();
|
||||
resetGridVerticalScroll();
|
||||
} else if (!inPlaceRefreshPending) {
|
||||
// A result replaced without an internal grid action (e.g. SQL re-executed
|
||||
// from the editor or a grid-embedded run button) is a brand-new dataset:
|
||||
// start from the first row instead of stranding the viewport at the old
|
||||
// scroll offset (#7341). In-place refreshes opt out via prepareFullReload.
|
||||
resetGridVerticalScroll();
|
||||
}
|
||||
closeReadonlyCellTextSelection();
|
||||
clearCellSelection();
|
||||
|
||||
+56
@@ -0,0 +1,56 @@
|
||||
import { readFileSync } from "node:fs";
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
const dataGridSource = readFileSync(new URL("../DataGrid.vue", import.meta.url), "utf8");
|
||||
|
||||
function resultWatchSource(): string {
|
||||
const start = dataGridSource.indexOf("watch(\n () => props.result,\n (result, previousResult) => {");
|
||||
const end = dataGridSource.indexOf("// --- Context menu handlers ---");
|
||||
expect(start).toBeGreaterThanOrEqual(0);
|
||||
expect(end).toBeGreaterThan(start);
|
||||
return dataGridSource.slice(start, end);
|
||||
}
|
||||
|
||||
describe("DataGrid result-replacement vertical scroll", () => {
|
||||
it("resets to the first row when the result is replaced without an internal grid action", () => {
|
||||
const watchSource = resultWatchSource();
|
||||
|
||||
expect(watchSource).toContain("} else if (!inPlaceRefreshPending) {");
|
||||
const externalBranch = watchSource.slice(watchSource.indexOf("} else if (!inPlaceRefreshPending) {"));
|
||||
expect(externalBranch.indexOf("resetGridVerticalScroll();")).toBeGreaterThanOrEqual(0);
|
||||
expect(externalBranch).toContain("#7341");
|
||||
});
|
||||
|
||||
it("keeps the internal-action reset flag path unchanged", () => {
|
||||
const watchSource = resultWatchSource();
|
||||
|
||||
expect(watchSource).toContain("if (getResetScrollAfterResult()) {\n clearResetScrollAfterResult();\n resetGridVerticalScroll();");
|
||||
});
|
||||
|
||||
it("treats any pending prepareFullReload marker as an in-place refresh that keeps the viewport", () => {
|
||||
const watchSource = resultWatchSource();
|
||||
|
||||
expect(watchSource).toContain("preservedSelectionOnNextResult !== null || preservedViewportAnchorOnNextResult !== null || preservedDetailsOnNextResult !== null || preserveTransposeOnNextResult.value");
|
||||
// The pending-marker check must run before the watch consumes the markers.
|
||||
const pendingCheck = watchSource.indexOf("const inPlaceRefreshPending =");
|
||||
const selectionConsume = watchSource.indexOf("preservedSelectionOnNextResult = null;");
|
||||
const anchorConsume = watchSource.indexOf("preservedViewportAnchorOnNextResult = null;");
|
||||
const detailsConsume = watchSource.indexOf("preservedDetailsOnNextResult = null;");
|
||||
const transposeConsume = watchSource.indexOf("preserveTransposeOnNextResult.value = false;");
|
||||
expect(pendingCheck).toBeGreaterThanOrEqual(0);
|
||||
for (const consume of [selectionConsume, anchorConsume, detailsConsume, transposeConsume]) {
|
||||
expect(consume).toBeGreaterThan(pendingCheck);
|
||||
}
|
||||
});
|
||||
|
||||
it("returns before any scroll reset when the replacement completes an infinite-scroll append", () => {
|
||||
const watchSource = resultWatchSource();
|
||||
|
||||
const appendReturn = watchSource.indexOf("if (appendCompletion) {");
|
||||
const appendEarlyReturn = watchSource.indexOf("return;\n }");
|
||||
const resetBranch = watchSource.indexOf("if (getResetScrollAfterResult()) {");
|
||||
expect(appendReturn).toBeGreaterThanOrEqual(0);
|
||||
expect(appendEarlyReturn).toBeGreaterThan(appendReturn);
|
||||
expect(resetBranch).toBeGreaterThan(appendEarlyReturn);
|
||||
});
|
||||
});
|
||||
@@ -150,6 +150,104 @@ describe("useDataGridEditor result snapshots", () => {
|
||||
expect(oldIdentityScroller.scrollTop).toBe(0);
|
||||
expect(oldIdentityScroller.scrollLeft).toBe(0);
|
||||
});
|
||||
|
||||
it("does not adopt a scroll-only snapshot when remounting with a same-shape result (#7341)", () => {
|
||||
class TestScroller {
|
||||
scrollTop = 0;
|
||||
scrollLeft = 0;
|
||||
scrollTo({ top, left }: ScrollToOptions) {
|
||||
if (typeof top === "number") this.scrollTop = top;
|
||||
if (typeof left === "number") this.scrollLeft = left;
|
||||
}
|
||||
}
|
||||
vi.stubGlobal("HTMLElement", TestScroller);
|
||||
const rows: CellValue[][] = [
|
||||
["a", null, 1],
|
||||
["b", null, 2],
|
||||
["c", null, 3],
|
||||
];
|
||||
const key = "table-tab-scroll-only-snapshot";
|
||||
const previous = createEditor(undefined, true, key, undefined, rows);
|
||||
previous.newRows.value = [];
|
||||
const previousScroller = new TestScroller();
|
||||
previousScroller.scrollTop = 6_400;
|
||||
previousScroller.scrollLeft = 24;
|
||||
previous.scrollerRef.value = previousScroller as unknown as NonNullable<typeof previous.scrollerRef.value>;
|
||||
// Unmount path: saves a pure scroll snapshot even without pending edits.
|
||||
previous.savePendingSnapshot(true, true);
|
||||
|
||||
const remounted = createEditor(undefined, true, key, undefined, rows);
|
||||
const remountedScroller = new TestScroller();
|
||||
remounted.scrollerRef.value = remountedScroller as unknown as NonNullable<typeof remounted.scrollerRef.value>;
|
||||
remounted.restorePendingSnapshotFocus();
|
||||
expect(remountedScroller.scrollTop).toBe(0);
|
||||
expect(remountedScroller.scrollLeft).toBe(0);
|
||||
});
|
||||
|
||||
it("still adopts the cached scroll when the snapshot carries pending edits", () => {
|
||||
class TestScroller {
|
||||
scrollTop = 0;
|
||||
scrollLeft = 0;
|
||||
scrollTo({ top, left }: ScrollToOptions) {
|
||||
if (typeof top === "number") this.scrollTop = top;
|
||||
if (typeof left === "number") this.scrollLeft = left;
|
||||
}
|
||||
}
|
||||
vi.stubGlobal("HTMLElement", TestScroller);
|
||||
const rows: CellValue[][] = [
|
||||
["a", null, 1],
|
||||
["b", null, 2],
|
||||
["c", null, 3],
|
||||
];
|
||||
const key = "table-tab-edit-snapshot";
|
||||
const previous = createEditor(undefined, true, key, undefined, rows);
|
||||
previous.newRows.value = [];
|
||||
previous.dirtyRows.value.set(1, new Map([[0, "edited"]]));
|
||||
const previousScroller = new TestScroller();
|
||||
previousScroller.scrollTop = 6_400;
|
||||
previousScroller.scrollLeft = 24;
|
||||
previous.scrollerRef.value = previousScroller as unknown as NonNullable<typeof previous.scrollerRef.value>;
|
||||
previous.savePendingSnapshot(true, true);
|
||||
|
||||
const remounted = createEditor(undefined, true, key, undefined, rows);
|
||||
const remountedScroller = new TestScroller();
|
||||
remounted.scrollerRef.value = remountedScroller as unknown as NonNullable<typeof remounted.scrollerRef.value>;
|
||||
remounted.restorePendingSnapshotFocus();
|
||||
expect(remounted.dirtyRows.value.get(1)?.get(0)).toBe("edited");
|
||||
expect(remountedScroller.scrollTop).toBe(6_400);
|
||||
expect(remountedScroller.scrollLeft).toBe(24);
|
||||
});
|
||||
|
||||
it("keeps pure scroll restore for the KeepAlive reactivate path", () => {
|
||||
class TestScroller {
|
||||
scrollTop = 0;
|
||||
scrollLeft = 0;
|
||||
scrollTo({ top, left }: ScrollToOptions) {
|
||||
if (typeof top === "number") this.scrollTop = top;
|
||||
if (typeof left === "number") this.scrollLeft = left;
|
||||
}
|
||||
}
|
||||
vi.stubGlobal("HTMLElement", TestScroller);
|
||||
const rows: CellValue[][] = [
|
||||
["a", null, 1],
|
||||
["b", null, 2],
|
||||
["c", null, 3],
|
||||
];
|
||||
const editor = createEditor(undefined, true, "table-tab-activate-path", undefined, rows);
|
||||
editor.newRows.value = [];
|
||||
const scroller = new TestScroller();
|
||||
scroller.scrollTop = 6_400;
|
||||
scroller.scrollLeft = 24;
|
||||
editor.scrollerRef.value = scroller as unknown as NonNullable<typeof editor.scrollerRef.value>;
|
||||
// Deactivate path: snapshot keeps the scroll for the same instance.
|
||||
editor.savePendingSnapshot(true, true);
|
||||
// Detaching the DOM resets the element's scroll offsets.
|
||||
scroller.scrollTop = 0;
|
||||
scroller.scrollLeft = 0;
|
||||
editor.restorePendingSnapshotFocus();
|
||||
expect(scroller.scrollTop).toBe(6_400);
|
||||
expect(scroller.scrollLeft).toBe(24);
|
||||
});
|
||||
});
|
||||
|
||||
describe("useDataGridEditor row deletion confirmation", () => {
|
||||
|
||||
@@ -314,7 +314,14 @@ export function useDataGridEditor(options: UseDataGridEditorOptions) {
|
||||
editValue.value = cached.editValue ?? "";
|
||||
restoredEditingCell = !!cached.editingCell;
|
||||
restoredTransactionActive = cached.transactionActive === true;
|
||||
pendingScrollRestore = cached.scroll;
|
||||
// A scroll-only snapshot (no pending edits, draft row, or active cell editor)
|
||||
// must not drag a remounted grid back to the previous viewport: the fresh
|
||||
// result should start at the first row (#7341). Scroll is only replayed
|
||||
// alongside edit state so the user lands back on their edited rows. The
|
||||
// KeepAlive activate path keeps pure scroll restore via the in-instance
|
||||
// pendingScrollRestore, which this gate does not touch.
|
||||
const snapshotHasEditState = cached.newRows.length > 0 || cached.dirtyRows.size > 0 || cached.deletedRows.size > 0 || !!cached.editingCell || !!cached.quickEntryDraftRow;
|
||||
pendingScrollRestore = snapshotHasEditState ? cached.scroll : undefined;
|
||||
pendingChangesCache.delete(key);
|
||||
} else {
|
||||
pendingChangesCache.delete(key);
|
||||
|
||||
Reference in new issue
Block a user