diff --git a/apps/desktop/src/components/grid/DataGrid.vue b/apps/desktop/src/components/grid/DataGrid.vue index 7bb3f3990b..af695a0385 100644 --- a/apps/desktop/src/components/grid/DataGrid.vue +++ b/apps/desktop/src/components/grid/DataGrid.vue @@ -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(); diff --git a/apps/desktop/src/components/grid/__tests__/DataGridExternalResultScroll.spec.ts b/apps/desktop/src/components/grid/__tests__/DataGridExternalResultScroll.spec.ts new file mode 100644 index 0000000000..610faf996a --- /dev/null +++ b/apps/desktop/src/components/grid/__tests__/DataGridExternalResultScroll.spec.ts @@ -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); + }); +}); diff --git a/apps/desktop/src/composables/__tests__/useDataGridEditor.spec.ts b/apps/desktop/src/composables/__tests__/useDataGridEditor.spec.ts index 679fb3ce1a..7c4d6cdad3 100644 --- a/apps/desktop/src/composables/__tests__/useDataGridEditor.spec.ts +++ b/apps/desktop/src/composables/__tests__/useDataGridEditor.spec.ts @@ -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; + // 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; + 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; + previous.savePendingSnapshot(true, true); + + const remounted = createEditor(undefined, true, key, undefined, rows); + const remountedScroller = new TestScroller(); + remounted.scrollerRef.value = remountedScroller as unknown as NonNullable; + 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; + // 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", () => { diff --git a/apps/desktop/src/composables/useDataGridEditor.ts b/apps/desktop/src/composables/useDataGridEditor.ts index 8897b63f8d..c382467544 100644 --- a/apps/desktop/src/composables/useDataGridEditor.ts +++ b/apps/desktop/src/composables/useDataGridEditor.ts @@ -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);