From d6cf1d1f8ad26a1701eae76e14580a4d307e28ed Mon Sep 17 00:00:00 2001 From: Ethan Graf Date: Sat, 1 Aug 2026 15:35:57 -0400 Subject: [PATCH] Restore scroll and cursor when returning to a tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switching tabs threw away where you were. `DocumentSlotView` keys `DocumentPane` by document id and only the active tab's slot is rendered, so every tab switch unmounts CodeMirror and rebuilds it from scratch — landing you back at the top of the document with the cursor at position 0. Each tab now keeps an `EditorViewSnapshot` (cursor + scroll). The editor reports one as it tears down and consumes one when it is built. The capture is a *layout* effect cleanup, which matters more than it looks: `scrollSnapshot()` reads `scrollDOM.scrollTop`, and React runs passive (`useEffect`) cleanups for a deleted subtree only after its DOM nodes are detached. A detached element reports `scrollTop` 0, so capturing there records "top of document" every time and the feature silently does nothing. Layout cleanups run during the mutation phase, while the node is still attached and still scrolled. Restoration happens at construction rather than by dispatching after mount: `EditorViewConfig.scrollTo` accepts the effect from `view.scrollSnapshot()`, so the position is right before the first paint and the editor never visibly jumps from the top. The snapshot is document-anchored, not a pixel offset, so it survives content arriving late. The restored selection is clamped to the document length, because a synced vault's document can still be empty at mount and an out-of-range selection throws. The capture closure binds the tab id at render time. That is load-bearing and easy to get wrong: when the active tab changes, the outgoing editor is unmounted without ever being re-rendered, so its teardown must report against the tab being LEFT rather than the one arriving. `TileTab.view` is in-memory only — it holds a live CodeMirror object. `serializeLayout` already picks fields explicitly, so it cannot reach localStorage; a test pins that, since a future field could otherwise leak silently. Scroll position is therefore not restored across app restarts, which is a reasonable follow-up rather than part of this. Undo history is still lost on a tab switch — restoring it means serialising `historyField`, which is deliberately out of scope here. Co-Authored-By: Claude Opus 5 --- src/editors/DocumentSlotView.tsx | 10 +++- .../automerge/automergeDocumentEditor.tsx | 56 ++++++++++++++++++- src/editors/types.ts | 27 +++++++++ src/workspaces/tiling/tabsModel.test.ts | 51 +++++++++++++++++ src/workspaces/tiling/tabsModel.ts | 41 +++++++++++++- src/workspaces/tiling/tiling.tsx | 20 ++++++- .../tiling/tilingPersistence.test.ts | 26 +++++++++ .../tiling/useTilingDocumentSlots.ts | 18 +++++- src/workspaces/zen/zen.tsx | 11 +++- 9 files changed, 254 insertions(+), 6 deletions(-) diff --git a/src/editors/DocumentSlotView.tsx b/src/editors/DocumentSlotView.tsx index 21dcb59..f4353c5 100644 --- a/src/editors/DocumentSlotView.tsx +++ b/src/editors/DocumentSlotView.tsx @@ -3,7 +3,7 @@ import { resolveDocumentLink } from '../documents/resolveDocumentLink'; import type { OpenDocRequest } from '../workspaces/workspace'; import { useWikiLinkDocument } from '../wikilinks/WikiLinkVaultContext'; import { DocumentPane } from './DocumentPane'; -import type { EditorMode } from './types'; +import type { EditorMode, EditorViewSnapshot } from './types'; const DEFAULT_EMPTY_HINT = 'Select a file from the sidebar to open it here.'; @@ -20,6 +20,10 @@ type DocumentSlotViewProps = { * workspace gets file-to-file navigation just by forwarding its open handler. */ onOpenEntry?: (request: OpenDocRequest) => void; + /** Position to restore — where this slot's tab was when it was last shown. */ + viewSnapshot?: EditorViewSnapshot | null; + /** Called as the editor tears down, with the position to restore next time. */ + onViewSnapshot?: (snapshot: EditorViewSnapshot) => void; }; function SlotPlaceholder({ message }: { message: string }) { @@ -48,6 +52,8 @@ export function DocumentSlotView({ emptyHint = DEFAULT_EMPTY_HINT, mode, onOpenEntry, + viewSnapshot, + onViewSnapshot, }: DocumentSlotViewProps) { // Hooks must run before the early returns below (rules of hooks). Only the // `empty` state lacks a request — and it has no document to link from. @@ -101,6 +107,8 @@ export function DocumentSlotView({ onOpenLink={onOpenLink} wikiLinkVault={vault} onOpenWikiLink={openWikiLink} + viewSnapshot={viewSnapshot} + onViewSnapshot={onViewSnapshot} /> ); } diff --git a/src/editors/automerge/automergeDocumentEditor.tsx b/src/editors/automerge/automergeDocumentEditor.tsx index a80efb9..831a6ef 100644 --- a/src/editors/automerge/automergeDocumentEditor.tsx +++ b/src/editors/automerge/automergeDocumentEditor.tsx @@ -1,7 +1,8 @@ -import { useRef, useEffect, useState } from 'react'; +import { useEffect, useLayoutEffect, useRef, useState } from 'react'; import { EditorState, Compartment } from '@codemirror/state'; import { EditorView, + type EditorViewConfig, drawSelection, keymap, placeholder, @@ -51,6 +52,8 @@ export function AutomergeDocumentEditor({ onOpenLink, wikiLinkVault = null, onOpenWikiLink, + viewSnapshot = null, + onViewSnapshot, }: EditorProps) { const { getDoc, applyChange, subscribeToChanges } = getAutomergePayload(binding); @@ -66,6 +69,15 @@ export function AutomergeDocumentEditor({ const onOpenWikiLinkRef = useRef(onOpenWikiLink); onOpenWikiLinkRef.current = onOpenWikiLink; + // Read during the teardown layout effect below. A replaced editor is never + // re-rendered before it unmounts, so this still holds the closure from its + // last render — reporting against the tab being LEFT, not the one arriving. + const onViewSnapshotRef = useRef(onViewSnapshot); + onViewSnapshotRef.current = onViewSnapshot; + // Mount-time only: the snapshot is consumed when the view is built, and a + // later prop change must not retroactively move the cursor. + const initialSnapshotRef = useRef(viewSnapshot); + const modeCompartment = useRef(new Compartment()); // The vault index arrives asynchronously and changes as files come and go, // so it lives in its own compartment rather than being baked in at mount. @@ -117,6 +129,31 @@ export function AutomergeDocumentEditor({ contentRef.current = newContent; }, [docVersion, docContent]); + /** + * Record where this tab was, on the way out. + * + * A *layout* effect specifically. `scrollSnapshot()` reads + * `scrollDOM.scrollTop`, and React runs passive (`useEffect`) cleanups for a + * deleted subtree only after its DOM nodes have been detached — a detached + * element reports `scrollTop` 0, so capturing there silently recorded "top of + * document" every time. Layout cleanups run during the mutation phase, while + * the node is still in the document and still scrolled. + * + * The view itself is created and destroyed by the passive effect below, which + * runs later; by then this has already taken its measurement. + */ + useLayoutEffect(() => { + return () => { + const view = viewRef.current; + if (!view) return; + const { anchor, head } = view.state.selection.main; + onViewSnapshotRef.current?.({ + selection: { anchor, head }, + scroll: view.scrollSnapshot(), + }); + }; + }, []); + // Create EditorView on mount. useEffect(() => { if (!containerRef.current) return; @@ -142,8 +179,21 @@ export function AutomergeDocumentEditor({ } }); + // Restore the cursor from the last time this tab was shown. Clamped: for a + // synced vault the document can still be empty at mount and fill in later, + // and an out-of-range selection would throw. + const snapshot = initialSnapshotRef.current; + const clamp = (n: number) => + Math.max(0, Math.min(n, initialContent.length)); + const state = EditorState.create({ doc: initialContent, + selection: snapshot + ? { + anchor: clamp(snapshot.selection.anchor), + head: clamp(snapshot.selection.head), + } + : undefined, extensions: [ // Core history(), @@ -183,6 +233,10 @@ export function AutomergeDocumentEditor({ const view = new EditorView({ state, parent: containerRef.current, + // Applying the scroll position at construction (rather than dispatching + // it after mount) restores it before the first paint, so returning to a + // tab doesn't visibly jump from the top. + scrollTo: snapshot?.scroll as EditorViewConfig['scrollTo'], }); viewRef.current = view; diff --git a/src/editors/types.ts b/src/editors/types.ts index 65d01b0..39822a7 100644 --- a/src/editors/types.ts +++ b/src/editors/types.ts @@ -19,6 +19,23 @@ export type EditorBinding = { payload: unknown; }; +/** + * Where a tab was when you last left it. + * + * Switching tabs unmounts the editor (`DocumentSlotView` keys `DocumentPane` by + * document id, and only the active tab's slot is rendered), so scroll position + * and cursor would otherwise be lost every time. The workspace holds one of + * these per tab and hands it back on the next mount. + * + * `scroll` is a CodeMirror scroll effect — opaque to everything but the editor + * that produced it, and holding a live object, so this is in-memory only and is + * deliberately never persisted. + */ +export type EditorViewSnapshot = { + selection: { anchor: number; head: number }; + scroll: unknown; +}; + /** Where a `[[wikilink]]` target points, and whether that document exists yet. */ export type WikiLinkResolution = { /** @@ -90,6 +107,16 @@ export type EditorProps = { * `wikiLinkVault`) and handles creating the note when it does not exist. */ onOpenWikiLink?: (activation: WikiLinkActivation) => void; + /** + * Position to restore on mount — where this tab was when it was last shown. + * Absent for a document being opened for the first time. + */ + viewSnapshot?: EditorViewSnapshot | null; + /** + * Called as the editor tears down, with the position to restore next time. + * The workspace stores it against the tab being left. + */ + onViewSnapshot?: (snapshot: EditorViewSnapshot) => void; }; /** diff --git a/src/workspaces/tiling/tabsModel.test.ts b/src/workspaces/tiling/tabsModel.test.ts index c3f74a4..6231d6b 100644 --- a/src/workspaces/tiling/tabsModel.test.ts +++ b/src/workspaces/tiling/tabsModel.test.ts @@ -16,11 +16,13 @@ import { findTabForRequest, getActiveSlot, getActiveTabId, + getTabView, getTabs, moveTab, planOpen, setActiveTab, setTabSlot, + setTabView, type TileTabsMap, } from './tabsModel'; @@ -451,3 +453,52 @@ describe('activeDocument', () => { expect(activeDocument(errored, 'tile-1')).toMatchObject({ entryId: 'a' }); }); }); + +describe('setTabView / getTabView', () => { + const snap = (anchor: number) => ({ + selection: { anchor, head: anchor }, + scroll: null, + }); + + it('stores a position against one tab and reads it back', () => { + const map = tileWithDocs('tile-1', ['a', 'b'], 'a'); + const next = setTabView(map, 'tile-1', 'tab-b', snap(42)); + expect(getTabView(next, 'tile-1', 'tab-b')).toEqual(snap(42)); + }); + + it('leaves other tabs untouched', () => { + const map = tileWithDocs('tile-1', ['a', 'b'], 'a'); + const next = setTabView(map, 'tile-1', 'tab-b', snap(42)); + expect(getTabView(next, 'tile-1', 'tab-a')).toBeNull(); + }); + + it('replaces an existing position rather than merging', () => { + const map = setTabView( + tileWithDocs('tile-1', ['a']), + 'tile-1', + 'tab-a', + snap(1), + ); + const next = setTabView(map, 'tile-1', 'tab-a', snap(2)); + expect(getTabView(next, 'tile-1', 'tab-a')).toEqual(snap(2)); + }); + + it('does not disturb the slot, so the editor is not remounted', () => { + const map = tileWithDocs('tile-1', ['a']); + const next = setTabView(map, 'tile-1', 'tab-a', snap(5)); + expect(getActiveSlot(next, 'tile-1')).toBe(getActiveSlot(map, 'tile-1')); + }); + + it('is a no-op (same reference) for a missing tile or tab', () => { + const map = tileWithDocs('tile-1', ['a']); + expect(setTabView(map, 'other', 'tab-a', snap(1))).toBe(map); + expect(setTabView(map, 'tile-1', 'missing', snap(1))).toBe(map); + }); + + it('reads null for an unknown tile, tab, or a null tab id', () => { + const map = tileWithDocs('tile-1', ['a']); + expect(getTabView(map, 'tile-1', null)).toBeNull(); + expect(getTabView(map, 'tile-1', 'missing')).toBeNull(); + expect(getTabView(map, 'other', 'tab-a')).toBeNull(); + }); +}); diff --git a/src/workspaces/tiling/tabsModel.ts b/src/workspaces/tiling/tabsModel.ts index 0a3f3b6..de1f769 100644 --- a/src/workspaces/tiling/tabsModel.ts +++ b/src/workspaces/tiling/tabsModel.ts @@ -13,7 +13,7 @@ * release. */ import type { DocumentSlotState } from '../../documents/useDocumentSlot'; -import type { EditorMode } from '../../editors/types'; +import type { EditorMode, EditorViewSnapshot } from '../../editors/types'; import type { OpenDocRequest } from '../workspace'; export type TileTab = { @@ -21,6 +21,13 @@ export type TileTab = { slot: DocumentSlotState; /** Rendering mode for this tab's editor. Rides along on move/persist. */ mode: EditorMode; + /** + * Scroll/cursor position from the last time this tab was shown, so switching + * away and back returns you where you were. In-memory only: it holds a live + * CodeMirror object, and `serializeLayout` picks fields explicitly so it + * never reaches localStorage. + */ + view?: EditorViewSnapshot; }; const DEFAULT_MODE: EditorMode = 'edit'; @@ -171,6 +178,38 @@ export function setTabMode( return { ...map, [tileId]: { ...tile, tabs } }; } +/** + * Store one tab's last scroll/cursor position. No-op if the tile/tab is + * missing — a tab can be closed while its editor is tearing down. + */ +export function setTabView( + map: TileTabsMap, + tileId: string, + tabId: string, + view: EditorViewSnapshot, +): TileTabsMap { + const tile = map[tileId]; + if (!tile) return map; + let changed = false; + const tabs = tile.tabs.map((t) => { + if (t.tabId !== tabId) return t; + changed = true; + return { ...t, view }; + }); + if (!changed) return map; + return { ...map, [tileId]: { ...tile, tabs } }; +} + +/** The active tab's stored position, if it has been shown before. */ +export function getTabView( + map: TileTabsMap, + tileId: string, + tabId: string | null, +): EditorViewSnapshot | null { + if (!tabId) return null; + return map[tileId]?.tabs.find((t) => t.tabId === tabId)?.view ?? null; +} + /** The active tab's rendering mode for a tile (defaults to `edit`). */ export function getActiveMode(map: TileTabsMap, tileId: string): EditorMode { const tile = map[tileId]; diff --git a/src/workspaces/tiling/tiling.tsx b/src/workspaces/tiling/tiling.tsx index 3f7b738..252045b 100644 --- a/src/workspaces/tiling/tiling.tsx +++ b/src/workspaces/tiling/tiling.tsx @@ -2,7 +2,7 @@ import { ReactNode, useEffect, useRef, useState, type DragEvent } from 'react'; import { DocumentSlotView } from '../../editors/DocumentSlotView'; import { EditorModeToggle } from '../../components/EditorModeToggle'; -import type { EditorMode } from '../../editors/types'; +import type { EditorMode, EditorViewSnapshot } from '../../editors/types'; import type { DocumentSlotState } from '../../documents/useDocumentSlot'; import { clampRatioForAxis, @@ -165,6 +165,10 @@ type TilingTileProps = { onNewFile?: () => void; /** Open a document in the active tile (used to follow internal links). */ onOpenEntry?: (request: OpenDocRequest) => void; + /** Position to restore for the tab this tile is showing. */ + viewSnapshot?: EditorViewSnapshot | null; + /** Called as this tile's editor tears down, with the position to restore. */ + onViewSnapshot?: (snapshot: EditorViewSnapshot) => void; }; function TilingTile({ @@ -182,6 +186,8 @@ function TilingTile({ onNewTab, onNewFile, onOpenEntry, + viewSnapshot, + onViewSnapshot, }: TilingTileProps) { // Zone shown while a tab is dragged over this tile's body (null = no drag). const [dropZone, setDropZone] = useState(null); @@ -244,6 +250,8 @@ function TilingTile({ className="min-h-0 flex-1" mode={mode} onOpenEntry={onOpenEntry} + viewSnapshot={viewSnapshot} + onViewSnapshot={onViewSnapshot} /> )} {dropZone !== null ? ( @@ -311,6 +319,8 @@ export function TilingWorkspace({ getActiveTabId, getActiveMode, setTabMode, + getTabView, + setTabView, ensureTile, setActiveTab, cycleTab, @@ -516,6 +526,10 @@ export function TilingWorkspace({ const renderNode = (node: TilingNode): ReactNode => { if (node.kind === 'tile') { const tileId = node.id; + // Captured at render time: when the active tab changes, the outgoing + // editor is unmounted without ever being re-rendered, so its teardown + // still reports against the tab being LEFT rather than the one arriving. + const shownTabId = getActiveTabId(tileId); return ( setActiveTileId(tileId)} onOpenEntry={onOpenEntry} + viewSnapshot={getTabView(tileId, shownTabId)} + onViewSnapshot={(snap) => { + if (shownTabId) setTabView(tileId, shownTabId, snap); + }} onSelectTab={(tabId) => setActiveTab(tileId, tabId)} onTabAction={(action, tabId) => handleTabAction(tileId, action, tabId) diff --git a/src/workspaces/tiling/tilingPersistence.test.ts b/src/workspaces/tiling/tilingPersistence.test.ts index cf8edba..8dcd997 100644 --- a/src/workspaces/tiling/tilingPersistence.test.ts +++ b/src/workspaces/tiling/tilingPersistence.test.ts @@ -22,6 +22,32 @@ function splitRow( } describe('serializeLayout', () => { + it('excludes a tab’s scroll/cursor snapshot from the persisted layout', () => { + // `TileTab.view` holds a live CodeMirror object and is in-memory only. + // serializeLayout picks fields explicitly, which is what keeps it out — + // this pins that, so a future field cannot leak into localStorage silently. + const tileTabs: TileTabsMap = { + 'tile-1': { + tabs: [ + { + tabId: 'tab-a', + slot: { kind: 'loading', request: req('a') }, + mode: 'edit', + view: { selection: { anchor: 12, head: 12 }, scroll: {} }, + }, + ], + activeTabId: 'tab-a', + }, + }; + const layout = serializeLayout( + { kind: 'tile', id: 'tile-1' }, + 'tile-1', + tileTabs, + ); + expect(layout.tiles[0].tabs).toEqual([{ request: req('a'), mode: 'edit' }]); + expect(JSON.stringify(layout)).not.toContain('selection'); + }); + it('captures each tile’s documents and active index, skipping empty tabs', () => { const tree = splitRow( 'split-1', diff --git a/src/workspaces/tiling/useTilingDocumentSlots.ts b/src/workspaces/tiling/useTilingDocumentSlots.ts index 7baca4b..272e5d8 100644 --- a/src/workspaces/tiling/useTilingDocumentSlots.ts +++ b/src/workspaces/tiling/useTilingDocumentSlots.ts @@ -3,7 +3,7 @@ import { useCallback, useEffect, useRef, useState } from 'react'; import type { DocumentSlotState } from '../../documents/useDocumentSlot'; import { startDocumentOpen } from '../../documents/documentSlotOpen'; import type { DocumentHandle } from '../../documents/types'; -import type { EditorMode } from '../../editors/types'; +import type { EditorMode, EditorViewSnapshot } from '../../editors/types'; import type { OpenDocRequest, WorkspaceProps } from '../workspace'; import * as tabsModel from './tabsModel'; import type { TileTab, TileTabsMap } from './tabsModel'; @@ -201,6 +201,20 @@ export function useTilingDocumentSlots({ [], ); + /** Where a tab was when it was last shown, for restoring on the next mount. */ + const getTabView = useCallback( + (tileId: string, tabId: string | null): EditorViewSnapshot | null => + tabsModel.getTabView(tileTabs, tileId, tabId), + [tileTabs], + ); + + const setTabView = useCallback( + (tileId: string, tabId: string, view: EditorViewSnapshot) => { + setTileTabs((prev) => tabsModel.setTabView(prev, tileId, tabId, view)); + }, + [], + ); + const setActiveTab = useCallback((tileId: string, tabId: string) => { setTileTabs((prev) => tabsModel.setActiveTab(prev, tileId, tabId)); }, []); @@ -325,6 +339,8 @@ export function useTilingDocumentSlots({ getActiveTabId, getActiveMode, setTabMode, + getTabView, + setTabView, ensureTile, setActiveTab, cycleTab, diff --git a/src/workspaces/zen/zen.tsx b/src/workspaces/zen/zen.tsx index 7c7924c..a5213dd 100644 --- a/src/workspaces/zen/zen.tsx +++ b/src/workspaces/zen/zen.tsx @@ -1,10 +1,11 @@ -import { useEffect } from 'react'; +import { useEffect, useState } from 'react'; import { DocumentSlotView } from '../../editors/DocumentSlotView'; import { useDocumentSlot } from '../../documents/useDocumentSlot'; import { useDocumentTitle } from '../../documents/useDocumentTitle'; import { WorkspaceBarSlot } from '../barSlot'; import type { WorkspaceProps } from '../workspace'; +import type { EditorViewSnapshot } from '../../editors/types'; export function ZenWorkspace({ active, @@ -21,6 +22,12 @@ export function ZenWorkspace({ accepts: active, }); + // Zen shows one document at a time, but opening another and coming back + // still remounts the editor — so it keeps a snapshot the same way tiling does. + const [viewSnapshot, setViewSnapshot] = useState( + null, + ); + const handle = state.kind === 'open' ? state.handle : null; const title = useDocumentTitle(handle); @@ -38,6 +45,8 @@ export function ZenWorkspace({ state={state} className="min-h-0 flex-1" onOpenEntry={onOpenEntry} + viewSnapshot={viewSnapshot} + onViewSnapshot={setViewSnapshot} /> -- 2.51.2