From 502cc348ba092b7256a1e72dd2c6028b8d6860e1 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 9 Nov 2026 14:16:21 +0800 Subject: [PATCH] perf(editor): keep recently used editor views across tab switches (#2282) Switching between file tabs destroyed the hidden tab's CodeMirror view and built a new one on return: every switch re-ran the view's plugins, re-measured the height map (the scrollbar jumped) and re-requested inlay hints, usages and diagnostics, which popped in and shifted the text up to a second later. The eight most recently hidden views are now kept detached and re-attached on return, with their decorations, height map and plugin state. Edits while hidden go to the kept view; plugins that skip work while their view is detached catch up when it is shown again (`viewShown`). Switching also forces fewer layouts: the Switcher no longer gives focus back to the editor left behind when it opens another tab, the tab strip only measures the active tab when it isn't known to be in view, and focus moving between editors no longer drops the old editor's DOM caret. The tabs perf scenario counts views rebuilt when switching back (budget 0). --- perf/budgets.json | 1 + perf/scenarios.ts | 18 ++- src/editor/api.ts | 13 +- src/editor/core/model/attachment.ts | 9 +- src/editor/core/model/document.test.ts | 52 ++++++- src/editor/core/model/slot.ts | 137 +++++++++++++----- .../core/model/workspaceHistory.test.ts | 4 +- src/editor/lsp/codeLens.ts | 8 +- src/editor/lsp/diagnostics.test.ts | 29 +++- src/editor/lsp/diagnostics.ts | 11 +- src/editor/lsp/inlay.ts | 7 +- src/editor/lsp/serverCodeLens.ts | 9 +- src/editor/navigation/implementationGutter.ts | 8 +- src/features/color-preview/pluginColors.ts | 10 +- src/features/plugins/languageVisuals.ts | 9 +- src/features/plugins/lineMarkerGutter.ts | 9 +- .../search/__tests__/dialogs.test.tsx | 6 +- src/features/search/recent/RecentPopups.tsx | 32 +++- src/ui/Dialog.tsx | 10 +- src/ui/Tabs.tsx | 31 +++- src/workbench/focus.test.ts | 15 ++ src/workbench/focus.ts | 3 + 22 files changed, 344 insertions(+), 87 deletions(-) diff --git a/perf/budgets.json b/perf/budgets.json index 5f3110ab2..663902ce3 100644 --- a/perf/budgets.json +++ b/perf/budgets.json @@ -73,6 +73,7 @@ "tabs.openSmallMs": 100, "tabs.switchP95Ms": 33, "tabs.switcherP50Ms": 33, + "tabs.viewsRebuilt": 0, "agentTabs.conversationNodes": 20000, "agentTabs.rowsAlwaysRendered": 0, diff --git a/perf/scenarios.ts b/perf/scenarios.ts index 4e6473cbe..6ed8cba12 100644 --- a/perf/scenarios.ts +++ b/perf/scenarios.ts @@ -449,12 +449,24 @@ const tabs: Scenario = { await h.openFile(`${ROOT}/src/tabs/${name}`, tsSource(2_000, `tab-${i}`)); } const switches: number[] = []; + // Recently shown tabs keep their editor view: showing one again must not build a new one. + let viewsRebuilt = 0; + let switchWork = await h.counters(); for (let round = 0; round < 3; round++) { + if (round === 1) switchWork = await h.counters(); for (let i = 0; i < names.length; i++) { const tab = h.page.getByRole("tab", { name: names[i], exact: true }); switches.push(await h.measure(() => tab.click(), ".cm-content", `// tab-${i}`)); + const fresh = await h.page.evaluate(() => { + const view = document.querySelector("[data-editor-host] .cm-editor") as { perfSeen?: boolean } | null; + if (!view || view.perfSeen) return false; + view.perfSeen = true; + return true; + }); + if (round > 0 && fresh) viewsRebuilt++; } } + const work = Harness.delta(switchWork, await h.counters()); // The Switcher (Ctrl+Tab) back and forth between the two most recent tabs. const before = await h.counters(); const switcher: number[] = []; @@ -463,12 +475,14 @@ const tabs: Scenario = { switcher.push(await h.measure(() => h.pressAction("navigate.switcher"), ".cm-content", `// tab-${tag}`)); await h.page.waitForTimeout(150); } - const work = Harness.delta(before, await h.counters()); + const switcherWork = Harness.delta(before, await h.counters()); return { openSmallMs, ...latencySummary(switches, "switch"), + switchMainThreadMs: work.taskMs / (names.length * 2), + viewsRebuilt, ...latencySummary(switcher, "switcher"), - switcherMainThreadMs: work.taskMs / switcher.length, + switcherMainThreadMs: switcherWork.taskMs / switcher.length, }; }, }; diff --git a/src/editor/api.ts b/src/editor/api.ts index 6be4d04eb..25f99448d 100644 --- a/src/editor/api.ts +++ b/src/editor/api.ts @@ -28,7 +28,7 @@ import { type RangeSet, type Transaction, } from "@codemirror/state"; -import { GutterMarker, ViewPlugin, type EditorView } from "@codemirror/view"; +import { GutterMarker, ViewPlugin, type EditorView, type ViewUpdate } from "@codemirror/view"; import type { LanguageDescription } from "@codemirror/language"; import type { LineEnding } from "@/platform/ipc"; import { create } from "zustand"; @@ -402,6 +402,17 @@ export interface ContributedIntention { /** A background provider has new context actions for the current document. */ export const intentionsChanged = StateEffect.define(); +/** + * A hidden editor view is shown again. Recently used tabs keep their view, detached from the + * document, while hidden; plugins that skip work while `view.dom` is disconnected redo it when + * `shownAgain`. + */ +export const viewShown = StateEffect.define(); + +export function shownAgain(u: ViewUpdate): boolean { + return u.transactions.some((tr) => tr.effects.some((e) => e.is(viewShown))); +} + /** Local fixes of a diagnostic range (hover link and Apply Quick Fix). */ export const quickFixProviders = Facet.define<(view: EditorView, from: number, to: number) => ContributedIntention[]>(); diff --git a/src/editor/core/model/attachment.ts b/src/editor/core/model/attachment.ts index 65f496e32..1a0701cfa 100644 --- a/src/editor/core/model/attachment.ts +++ b/src/editor/core/model/attachment.ts @@ -111,14 +111,7 @@ export async function attachDocument( if (model.ctx().readOnly || released || model.closed) return false; const slot = model.primarySlot(); if (slot?.view) return command(slot.view); - if (slot) - return command({ - state: slot.state, - dispatch: (tr) => { - slot.state = tr.state; - if (tr.docChanged) model.onLocal(tr.changes, slot); - }, - }); + if (slot) return command({ state: slot.state, dispatch: (tr) => slot.dispatchLocal(tr) }); return target.view ? command(target.view) : false; }; diff --git a/src/editor/core/model/document.test.ts b/src/editor/core/model/document.test.ts index 6f9993474..30b21b5b6 100644 --- a/src/editor/core/model/document.test.ts +++ b/src/editor/core/model/document.test.ts @@ -1,7 +1,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { EditorState } from "@codemirror/state"; import { indentUnit, language as languageFacet } from "@codemirror/language"; -import { HEAVY_LIMIT, registerSaveParticipant, remoteChange } from "@/editor/api"; +import { HEAVY_LIMIT, registerSaveParticipant, remoteChange, shownAgain } from "@/editor/api"; import { createMemoryBackend, type MemoryBackend } from "../backend"; import { _resetLanguages } from "../cm/languages"; import { DocumentManager, ModifiedRanges, saveCleanupChanges, useDocMeta } from "./document"; @@ -142,9 +142,10 @@ describe("DocumentManager + slots", () => { const a = new EditorSlot(model, "t1", host); const b = new EditorSlot(model, "t2", host); let sawRemote = false; - const spy = vi.spyOn(a, "applyExternal").mockImplementation(function (this: EditorSlot, changes, annotations) { + const applyExternal = a.applyExternal.bind(a); + const spy = vi.spyOn(a, "applyExternal").mockImplementation((changes, annotations) => { sawRemote = annotations.some((x) => x.type === remoteChange); - this.state = this.state.update({ changes, annotations }).state; + applyExternal(changes, annotations); }); backend.patch(model.doc, [{ from: 3, to: 3, insert: "def" }]); expect(sawRemote).toBe(true); @@ -301,6 +302,51 @@ describe("DocumentManager + slots", () => { manager.stop(); } }); + + it("keeps a hidden tab's view and shows it again instead of building a new one", async () => { + const { backend, manager } = setup({ "/p/a.txt": "abc" }); + const model = await manager.acquire("/p/a.txt"); + const slot = new EditorSlot(model, "t1", host); + const shown: boolean[] = []; + slot.listeners.add((u) => shown.push(shownAgain(u))); + const view = slot.mount(document.body); + slot.unmount(); + expect(slot.view).toBeNull(); + expect(view.dom.isConnected).toBe(false); + // Changes while hidden go to the kept view. + backend.patch(model.doc, [{ from: 3, to: 3, insert: "def" }]); + expect(view.state.doc.toString()).toBe("abcdef"); + expect(slot.currentState()).toBe(view.state); + shown.length = 0; + expect(slot.mount(document.body)).toBe(view); + expect(view.dom.isConnected).toBe(true); + expect(shown).toContain(true); + slot.dispose(); + manager.stop(); + }); + + it("destroys the least recently hidden views beyond the kept ones, and kept views on dispose", async () => { + const { manager } = setup({ "/p/a.txt": "abc" }); + const model = await manager.acquire("/p/a.txt"); + const slots = Array.from({ length: 8 }, (_, i) => new EditorSlot(model, `t${i}`, host)); + const views = slots.map((slot) => { + const view = slot.mount(document.body); + slot.unmount(); + return view; + }); + const destroyed = views.map((view) => vi.spyOn(view, "destroy")); + slots.push(new EditorSlot(model, "t8", host)); + slots[8]!.mount(document.body); + slots[8]!.unmount(); + expect(destroyed[0]).toHaveBeenCalled(); + expect(destroyed[1]).not.toHaveBeenCalled(); + // A destroyed view leaves its state behind; showing the tab builds a new view on it. + expect(slots[0]!.mount(document.body)).not.toBe(views[0]); + slots[1]!.dispose(); + expect(destroyed[1]).toHaveBeenCalled(); + for (const slot of slots) slot.dispose(); + manager.stop(); + }); }); describe("save cleanup", () => { diff --git a/src/editor/core/model/slot.ts b/src/editor/core/model/slot.ts index 3f2467af1..77b180ae7 100644 --- a/src/editor/core/model/slot.ts +++ b/src/editor/core/model/slot.ts @@ -1,8 +1,10 @@ /** * An editor slot is one tab showing a text document in one pane. It owns the * tab's EditorState (selection, folds, undo history, scroll) and, while the - * tab is visible, an EditorView. Background tabs keep only their state; views - * are created on show and destroyed on hide, so many open tabs cost little. + * tab is visible, an EditorView. The most recently used tabs keep their view, + * detached, while hidden (`KEPT_VIEWS`): showing one again re-attaches it with + * its decorations, height map and plugin state; older background tabs keep + * only their state and get a new view when shown. * * All slots of a document share its DocModel: local edits flow * slot → model → backend and are mirrored into the other slots. @@ -23,6 +25,7 @@ import { _addEditor, _removeEditor, documentSynced, + viewShown, isExternalChange, transientChange, type EditorDocContext, @@ -53,11 +56,20 @@ export interface Reveal { center?: boolean; } +/** Hidden views kept for showing again, least recently hidden first. */ +const KEPT_VIEWS = 8; +const keptSlots: EditorSlot[] = []; + export class EditorSlot { - state: EditorState; + private stored: EditorState; private historyTarget: WorkspaceHistoryTarget; private detachHistory: () => void; + /** The view while the tab is visible. */ view: EditorView | null = null; + /** The view kept, detached from the document, while the tab is hidden. */ + private kept: EditorView | null = null; + /** The view's scroll offset, tracked so hiding it needn't read layout. */ + private scrollPos = { top: 0, left: 0 }; handle: TextEditorHandle | null = null; private scroll: StateEffect | null = null; private pendingReveal: Reveal | null = null; @@ -80,8 +92,8 @@ export class EditorSlot { path: model.path, state: () => this.currentState(), dispatch: (tr) => { - if (this.view) this.view.dispatch(tr); - else this.state = tr.state; + if (this.live) this.live.dispatch(tr); + else this.stored = tr.state; }, }; const source = init?.from; @@ -91,7 +103,7 @@ export class EditorSlot { .find((state) => state?.field(historyField, false)) : null; const history = secondary?.field(historyField, false); - this.state = EditorState.create({ + this.stored = EditorState.create({ doc: model.sync.localText, selection: source ? source.currentState().selection : undefined, extensions: [ @@ -121,8 +133,17 @@ export class EditorSlot { return this.model.ctx(); } + /** The visible or kept view: the one holding the tab's state. */ + private get live(): EditorView | null { + return this.view ?? this.kept; + } + + get state(): EditorState { + return this.live?.state ?? this.stored; + } + currentState(): EditorState { - return this.view?.state ?? this.state; + return this.state; } private slotExtensions() { @@ -137,6 +158,11 @@ export class EditorSlot { } }), Prec.highest(keymap.of([{ key: "Escape", run: () => this.host.onEscape?.(this) ?? false }])), + EditorView.domEventObservers({ + scroll: (_e, view) => { + if (view === this.view) this.scrollPos = { top: view.scrollDOM.scrollTop, left: view.scrollDOM.scrollLeft }; + }, + }), EditorView.domEventHandlers({ contextmenu: (e) => { this.host.onContextMenu(this, e); @@ -162,14 +188,28 @@ export class EditorSlot { mount(parent: HTMLElement): EditorView { if (this.view) return this.view; - this.syncContributions(); - const view = new EditorView({ - state: this.state, - parent, - dispatchTransactions: (trs, v) => this.onTransactions(trs, v), - }); - this.view = view; - if (this.scroll) view.dispatch({ effects: this.scroll }); + const kept = this.kept; + let view: EditorView; + if (kept) { + this.kept = null; + keptSlots.splice(keptSlots.indexOf(this), 1); + parent.appendChild(kept.dom); + // Detached, it lost its scroll offset. + kept.scrollDOM.scrollTop = this.scrollPos.top; + kept.scrollDOM.scrollLeft = this.scrollPos.left; + view = this.view = kept; + this.syncContributions(); + // Plugins that skipped work while it was hidden catch up. + view.dispatch({ effects: this.scroll ? [this.scroll, viewShown.of(null)] : viewShown.of(null) }); + } else { + this.syncContributions(); + view = this.view = new EditorView({ + state: this.stored, + parent, + dispatchTransactions: (trs, v) => this.onTransactions(trs, v), + }); + if (this.scroll) view.dispatch({ effects: this.scroll }); + } this.scroll = null; this.handle = { view, ctx: this.model.ctx() }; _addEditor(this.handle); @@ -181,22 +221,39 @@ export class EditorSlot { return view; } - unmount() { + /** Hides the view: keeps it detached for the most recent tabs, else (or `keep: false`) destroys it. */ + unmount(keep = true) { const view = this.view; if (!view) return; - this.savedTopLine = topLineOf(view); - this.state = view.state; - this.scroll = view.scrollSnapshot(); if (this.handle) _removeEditor(this.handle); this.handle = null; this.view = null; + this.kept = view; + if (!keep) return this.release(); + view.dom.remove(); + keptSlots.push(this); + if (keptSlots.length > KEPT_VIEWS) keptSlots.shift()!.release(); + } + + /** Destroys the kept view, keeping its state. */ + private release() { + const view = this.kept; + if (!view) return; + this.kept = null; + const i = keptSlots.indexOf(this); + if (i >= 0) keptSlots.splice(i, 1); + const top = topLineAt(view, this.scrollPos.top); + this.savedTopLine = top; + this.scroll = EditorView.scrollIntoView(view.state.doc.line(top + 1).from, { y: "start" }); + this.stored = view.state; view.destroy(); } dispose() { if (this.disposed) return; this.detachHistory(); - this.unmount(); + this.unmount(false); + this.release(); this.disposed = true; this.model.removeSlot(this); } @@ -237,13 +294,13 @@ export class EditorSlot { } /** Applies a transaction as if the user made it (synced, undoable). */ - dispatchLocal(spec: TransactionSpec) { - if (this.view) { - this.view.dispatch(spec); + dispatchLocal(spec: TransactionSpec | Transaction) { + if (this.live) { + this.live.dispatch(spec); return; } - const tr = this.state.update(spec); - this.state = tr.state; + const tr = "startState" in spec ? spec : this.stored.update(spec); + this.stored = tr.state; if (tr.docChanged) this.model.onLocal(tr.changes, this); if (tr.docChanged && tr.isUserEvent("format.reformat")) this.model.redetectIndent(); } @@ -251,15 +308,15 @@ export class EditorSlot { /** Applies changes that are already in the model (other slot / backend). */ applyExternal(changes: ChangeSet, annotations: Annotation[]) { const spec: TransactionSpec = { changes, annotations }; - if (this.view) this.view.dispatch(spec); - else this.state = this.state.update(spec).state; + if (this.live) this.live.dispatch(spec); + else this.stored = this.stored.update(spec).state; } /** Applies effects (reconfiguration) to the view or the stored state. */ reconfigure(effects: StateEffect[]) { if (!effects.length) return; - if (this.view) this.view.dispatch({ effects }); - else this.state = this.state.update({ effects }).state; + if (this.live) this.live.dispatch({ effects }); + else this.stored = this.stored.update({ effects }).state; if (this.handle) this.handle.ctx = this.model.ctx(); } @@ -278,7 +335,7 @@ export class EditorSlot { const anchor = Math.min(Math.max(0, r.anchor), len); const head = Math.min(Math.max(0, r.head ?? anchor), len); if (!this.view) { - this.state = this.state.update({ selection: EditorSelection.single(anchor, head) }).state; + this.dispatchState({ selection: EditorSelection.single(anchor, head) }); this.pendingReveal = r; return; } @@ -294,7 +351,11 @@ export class EditorSlot { /** Caret and first visible line, for the persisted editor session. */ viewState(): { anchor: number; head: number; topLine?: number } { const main = this.currentState().selection.main; - const topLine = this.view ? topLineOf(this.view) : this.savedTopLine; + const topLine = this.view + ? topLineOf(this.view) + : this.kept + ? topLineAt(this.kept, this.scrollPos.top) + : this.savedTopLine; return { anchor: main.anchor, head: main.head, ...(topLine !== undefined ? { topLine } : {}) }; } @@ -313,13 +374,19 @@ export class EditorSlot { this.view.dispatch({ selection, effects: scroll ? [scroll] : [] }); return; } - this.state = this.state.update({ selection }).state; + this.dispatchState({ selection }); if (scroll) { this.scroll = scroll; this.savedTopLine = v.topLine; } } + /** Updates the state of a hidden tab (its kept view, if any). */ + private dispatchState(spec: TransactionSpec) { + if (this.kept) this.kept.dispatch(spec); + else this.stored = this.stored.update(spec).state; + } + caret(): { line: number; col: number; offset: number } { const s = this.currentState(); const head = s.selection.main.head; @@ -329,6 +396,10 @@ export class EditorSlot { } function topLineOf(view: EditorView): number { - const block = view.lineBlockAtHeight(Math.max(0, view.scrollDOM.scrollTop)); + return topLineAt(view, view.scrollDOM.scrollTop); +} + +function topLineAt(view: EditorView, scrollTop: number): number { + const block = view.lineBlockAtHeight(Math.max(0, scrollTop)); return view.state.doc.lineAt(block.from).number - 1; } diff --git a/src/editor/core/model/workspaceHistory.test.ts b/src/editor/core/model/workspaceHistory.test.ts index fc9e29821..0cd7a0803 100644 --- a/src/editor/core/model/workspaceHistory.test.ts +++ b/src/editor/core/model/workspaceHistory.test.ts @@ -62,9 +62,7 @@ async function rename() { function historyStep(slot: EditorSlot, forward = false) { return (forward ? redo : undo)({ state: slot.currentState(), - dispatch: (tr) => { - slot.state = tr.state; - }, + dispatch: (tr) => slot.dispatchLocal(tr), }); } diff --git a/src/editor/lsp/codeLens.ts b/src/editor/lsp/codeLens.ts index b8051deac..34e7b1c5b 100644 --- a/src/editor/lsp/codeLens.ts +++ b/src/editor/lsp/codeLens.ts @@ -8,7 +8,7 @@ import { Channel } from "@tauri-apps/api/core"; import { EditorSelection, MapMode, StateEffect, StateField, type Extension, type Text } from "@codemirror/state"; import { EditorView, ViewPlugin, type ViewUpdate } from "@codemirror/view"; -import { codeLenses, type CodeLens } from "@/editor/api"; +import { codeLenses, shownAgain, type CodeLens } from "@/editor/api"; import { executeAction } from "@/platform/actions"; import { commands, nextRequestId, type OutlineItem, type RequestId, type SymbolKind } from "@/platform/ipc"; import { useProject } from "@/platform/project"; @@ -119,6 +119,7 @@ class UsagesLensPlugin { private offReady: () => void; private offSettled: () => void; private offBlame: () => void; + private skipped = false; constructor(private view: EditorView) { this.refresh(); @@ -137,7 +138,7 @@ class UsagesLensPlugin { } update(u: ViewUpdate) { - if (u.docChanged || u.viewportChanged) this.refresh(); + if (u.docChanged || u.viewportChanged || (this.skipped && shownAgain(u))) this.refresh(); } private cancel() { @@ -153,7 +154,8 @@ class UsagesLensPlugin { private async run() { const view = this.view; - if (!view.dom.isConnected) return; + this.skipped = !view.dom.isConnected; + if (this.skipped) return; const doc = docOf(view); if (!doc) return; const gen = this.generation; diff --git a/src/editor/lsp/diagnostics.test.ts b/src/editor/lsp/diagnostics.test.ts index 285602944..0b9adc8f3 100644 --- a/src/editor/lsp/diagnostics.test.ts +++ b/src/editor/lsp/diagnostics.test.ts @@ -1,7 +1,12 @@ import { describe, expect, it, vi } from "vitest"; -import { Text } from "@codemirror/state"; +import { EditorState, Text } from "@codemirror/state"; +import { EditorView } from "@codemirror/view"; +import { viewShown, type EditorDocContext } from "@/editor/api"; +import { lspDoc } from "./lsp"; import type { Diagnostic } from "@/bindings"; import { + diagnosticsAt, + diagnosticsExtension, diagnosticsFor, installDiagnosticsEvents, setFileDiagnostics, @@ -84,3 +89,25 @@ describe("toLintDiagnostics", () => { expect(toLintDiagnostics(doc, [d(2, 1, 1)])[0]).toMatchObject({ from: doc.length - 1, to: doc.length }); }); }); + +describe("editor diagnostics in a view", () => { + it("skips a hidden view and catches up when it is shown again", async () => { + const view = new EditorView({ + state: EditorState.create({ + doc: "let x = 1;", + extensions: [lspDoc.of({ doc: 1, path: "/p/hidden.rs" } as EditorDocContext), diagnosticsExtension()], + }), + parent: document.body, + }); + view.dom.remove(); + setFileDiagnostics("/p/hidden.rs", "rust-analyzer", [d(0, 4, 5)]); + await Promise.resolve(); + expect(diagnosticsAt(view, 4)).toHaveLength(0); + document.body.appendChild(view.dom); + view.dispatch({ effects: viewShown.of(null) }); + await Promise.resolve(); + expect(diagnosticsAt(view, 4)).toHaveLength(1); + view.destroy(); + setFileDiagnostics("/p/hidden.rs", "rust-analyzer", []); + }); +}); diff --git a/src/editor/lsp/diagnostics.ts b/src/editor/lsp/diagnostics.ts index 917ec8023..751b401f9 100644 --- a/src/editor/lsp/diagnostics.ts +++ b/src/editor/lsp/diagnostics.ts @@ -6,13 +6,13 @@ */ import type { Text } from "@codemirror/state"; import { EditorSelection, StateEffect, StateField } from "@codemirror/state"; -import { EditorView, ViewPlugin } from "@codemirror/view"; +import { EditorView, ViewPlugin, type ViewUpdate } from "@codemirror/view"; import { forEachDiagnostic, linter, setDiagnostics, type Diagnostic as LintDiagnostic } from "@codemirror/lint"; import type { Diagnostic, Severity } from "@/bindings"; import { commands, ideEventsReady, isTauri, onIdeEvent } from "@/platform/ipc"; import { useProject } from "@/platform/project"; import { publishLocalDiagnostics } from "@/platform/localDiagnostics"; -import { intentionsChanged, scrollbarMarks, type ScrollbarMark } from "@/editor/api"; +import { intentionsChanged, scrollbarMarks, shownAgain, type ScrollbarMark } from "@/editor/api"; import { rangeToOffsets } from "./positions"; import { lspDoc } from "./lsp"; import { showHint } from "./hint"; @@ -190,6 +190,7 @@ const diagnosticsSync = ViewPlugin.fromClass( class { private unsubscribe: () => void; private pending = false; + private skipped = false; constructor(private view: EditorView) { const path = view.state.facet(lspDoc)?.path; this.unsubscribe = path ? subscribeDiagnostics(path, () => this.schedule()) : () => {}; @@ -201,7 +202,8 @@ const diagnosticsSync = ViewPlugin.fromClass( queueMicrotask(() => { this.pending = false; const path = this.view.state.facet(lspDoc)?.path; - if (!path || !this.view.dom.isConnected) return; + this.skipped = !this.view.dom.isConnected; + if (!path || this.skipped) return; const lint = toLintDiagnostics(this.view.state.doc, diagnosticsFor(path)); const tr = setDiagnostics(this.view.state, lint); this.view.dispatch({ @@ -210,6 +212,9 @@ const diagnosticsSync = ViewPlugin.fromClass( }); }); } + update(u: ViewUpdate) { + if (this.skipped && shownAgain(u)) this.schedule(); + } destroy() { this.unsubscribe(); } diff --git a/src/editor/lsp/inlay.ts b/src/editor/lsp/inlay.ts index 49c645c92..9c0c57ffd 100644 --- a/src/editor/lsp/inlay.ts +++ b/src/editor/lsp/inlay.ts @@ -5,6 +5,7 @@ * offers to disable hints of that kind. */ import { StateEffect, StateField, type Extension } from "@codemirror/state"; +import { shownAgain } from "@/editor/api"; import { Decoration, EditorView, ViewPlugin, WidgetType, type DecorationSet, type ViewUpdate } from "@codemirror/view"; import { call, commands, isTauri, type InlayHint } from "@/platform/ipc"; import { executeAction } from "@/platform/actions"; @@ -162,6 +163,7 @@ const inlayPlugin = ViewPlugin.fromClass( private unsubscribe: () => void; private offReady: () => void; private offModifier: () => void; + private skipped = false; constructor(private view: EditorView) { this.refresh(); view.contentDOM.classList.toggle("cm-lsp-navigation-modifier", navigationModifierHeld()); @@ -178,11 +180,12 @@ const inlayPlugin = ViewPlugin.fromClass( this.offReady = onLspRefresh("inlayHints", () => this.refresh()); } update(u: ViewUpdate) { - if (u.docChanged || u.viewportChanged) this.refresh(); + if (u.docChanged || u.viewportChanged || (this.skipped && shownAgain(u))) this.refresh(); } async run() { const view = this.view; - if (!view.dom.isConnected) return; + this.skipped = !view.dom.isConnected; + if (this.skipped) return; const doc = docOf(view); if (!doc) return; if (!enabled(doc.language) || inlayBackoff.active(doc.doc)) { diff --git a/src/editor/lsp/serverCodeLens.ts b/src/editor/lsp/serverCodeLens.ts index c7ed3d7c7..a69533ab0 100644 --- a/src/editor/lsp/serverCodeLens.ts +++ b/src/editor/lsp/serverCodeLens.ts @@ -1,7 +1,7 @@ /** Language-server and plugin lenses in the shared editor code-vision renderer. */ import { StateEffect, StateField, type Extension } from "@codemirror/state"; import { ViewPlugin, type ViewUpdate } from "@codemirror/view"; -import { codeLenses, whenSynced, type CodeLens, type EditorDocContext } from "@/editor/api"; +import { codeLenses, shownAgain, whenSynced, type CodeLens, type EditorDocContext } from "@/editor/api"; import { call, commands } from "@/platform/ipc"; import { onNavigationProvidersChanged } from "@/editor/navigationProviders"; import { posToOffset } from "./positions"; @@ -24,6 +24,7 @@ export function serverCodeLenses(ctx: EditorDocContext): Extension { new (class { timer: ReturnType | undefined; generation = 0; + skipped = false; off: () => void; constructor() { const offs = [ @@ -40,7 +41,7 @@ export function serverCodeLenses(ctx: EditorDocContext): Extension { this.timer = setTimeout(() => void this.run(), 400); } update(u: ViewUpdate) { - if (u.docChanged) this.schedule(); + if (u.docChanged || (this.skipped && shownAgain(u))) this.schedule(); } async run() { const gen = this.generation; @@ -48,7 +49,9 @@ export function serverCodeLenses(ctx: EditorDocContext): Extension { try { await whenSynced(view); const result = await call(commands.lspCodeLens(ctx.doc)); - if (gen !== this.generation || text !== view.state.doc || !view.dom.isConnected) return; + if (gen !== this.generation || text !== view.state.doc) return; + this.skipped = !view.dom.isConnected; + if (this.skipped) return; view.dispatch({ effects: set.of( result.flatMap((l) => diff --git a/src/editor/navigation/implementationGutter.ts b/src/editor/navigation/implementationGutter.ts index 3a99aa62c..6455c01d4 100644 --- a/src/editor/navigation/implementationGutter.ts +++ b/src/editor/navigation/implementationGutter.ts @@ -15,7 +15,7 @@ */ import { RangeSet, StateEffect, StateField, type Extension, type Text } from "@codemirror/state"; import { EditorView, GutterMarker, ViewPlugin, gutter, type ViewUpdate } from "@codemirror/view"; -import type { EditorDocContext } from "@/editor/api"; +import { shownAgain, type EditorDocContext } from "@/editor/api"; import { docOf, debounce, isUnavailable, lspRequest } from "@/editor/lsp/lsp"; import { featureBackoff, onServerSettled } from "@/editor/lsp/availability"; import { readManyFileLines, withDocument } from "@/editor/lsp/files"; @@ -295,6 +295,7 @@ function markerSet(placed: readonly PlacedMarker[]): RangeSet { class MarkersPlugin { private refresh = debounce(() => void this.run(), DEBOUNCE_MS); private generation = 0; + private skipped = false; private request: RequestId | null = null; private offSettings: () => void; @@ -316,7 +317,7 @@ class MarkersPlugin { // Other documents' markers may depend on this one. changes++; this.refresh(); - } else if (u.viewportChanged) this.refresh(); + } else if (u.viewportChanged || (this.skipped && shownAgain(u))) this.refresh(); // Other files may have changed meanwhile (a new implementation). else if (u.focusChanged && u.view.hasFocus) { const ctx = docOf(u.view); @@ -337,7 +338,8 @@ class MarkersPlugin { private async run() { const view = this.view; - if (!view.dom.isConnected) return; + this.skipped = !view.dom.isConnected; + if (this.skipped) return; const ctx = docOf(view); if (!ctx) return; const gen = ++this.generation; diff --git a/src/features/color-preview/pluginColors.ts b/src/features/color-preview/pluginColors.ts index afc1f4dcc..87f12db89 100644 --- a/src/features/color-preview/pluginColors.ts +++ b/src/features/color-preview/pluginColors.ts @@ -1,7 +1,7 @@ /** Plugin colors use the same native picker and provider-supplied replacement strings. */ import { RangeSet, StateEffect, StateField, type Extension } from "@codemirror/state"; import { GutterMarker, ViewPlugin, gutter, type EditorView, type ViewUpdate } from "@codemirror/view"; -import { type EditorDocContext } from "@/editor/api"; +import { shownAgain, type EditorDocContext } from "@/editor/api"; import { rangeToOffsets } from "@/editor/lsp/positions"; import { pluginVisuals } from "@/editor/lsp/pluginVisuals"; import { onNavigationProvidersChanged } from "@/editor/navigationProviders"; @@ -118,6 +118,7 @@ export function pluginColorPreview(ctx: EditorDocContext): Extension { new (class { timer: ReturnType | undefined; generation = 0; + skipped = false; off: () => void; constructor() { this.off = onNavigationProvidersChanged(() => this.schedule()); @@ -129,15 +130,16 @@ export function pluginColorPreview(ctx: EditorDocContext): Extension { this.timer = setTimeout(() => void this.run(), 300); } update(u: ViewUpdate) { - if (u.docChanged) this.schedule(); + if (u.docChanged || (this.skipped && shownAgain(u))) this.schedule(); } async run() { const gen = this.generation; const text = view.state.doc; try { const r = await pluginVisuals(view, ctx); - if (gen === this.generation && view.state.doc === text && view.dom.isConnected) - view.dispatch({ effects: setColors.of(r.colors) }); + if (gen !== this.generation || view.state.doc !== text) return; + this.skipped = !view.dom.isConnected; + if (!this.skipped) view.dispatch({ effects: setColors.of(r.colors) }); } catch { if (gen === this.generation && view.dom.isConnected) view.dispatch({ effects: setColors.of([]) }); } diff --git a/src/features/plugins/languageVisuals.ts b/src/features/plugins/languageVisuals.ts index d14440932..853dd522c 100644 --- a/src/features/plugins/languageVisuals.ts +++ b/src/features/plugins/languageVisuals.ts @@ -19,7 +19,7 @@ import { } from "@codemirror/view"; import { highlightingFor } from "@codemirror/language"; import { highlightTree } from "@lezer/highlight"; -import { type EditorDocContext } from "@/editor/api"; +import { shownAgain, type EditorDocContext } from "@/editor/api"; import { rangeToOffsets } from "@/editor/lsp/positions"; import { pluginVisuals } from "@/editor/lsp/pluginVisuals"; import { findLanguageDescription, languageInfo, loadSupport } from "@/editor/core/cm/languages"; @@ -123,6 +123,7 @@ export function pluginLanguageVisuals(ctx: EditorDocContext): Extension { timer: ReturnType | undefined; generation = 0; shown = false; + skipped = false; off: () => void; constructor() { const a = onNavigationProvidersChanged(() => this.schedule(true)); @@ -147,7 +148,7 @@ export function pluginLanguageVisuals(ctx: EditorDocContext): Extension { this.timer = setTimeout(() => void this.run(), 300); } update(u: ViewUpdate) { - if (u.docChanged) this.schedule(); + if (u.docChanged || (this.skipped && shownAgain(u))) this.schedule(); } async run() { const generation = this.generation; @@ -163,7 +164,9 @@ export function pluginLanguageVisuals(ctx: EditorDocContext): Extension { marker: new ActionMarker(plugin, { ...a, icon: await resolveMarkerIcon(plugin, a.icon) }, ctx), })), ); - if (generation !== this.generation || view.state.doc !== text || !view.dom.isConnected) return; + if (generation !== this.generation || view.state.doc !== text) return; + this.skipped = !view.dom.isConnected; + if (this.skipped) return; this.shown = !!placed.length; view.dispatch({ effects: [ diff --git a/src/features/plugins/lineMarkerGutter.ts b/src/features/plugins/lineMarkerGutter.ts index 6f611ed5c..0ff26e462 100644 --- a/src/features/plugins/lineMarkerGutter.ts +++ b/src/features/plugins/lineMarkerGutter.ts @@ -6,7 +6,7 @@ */ import { Compartment, RangeSet, StateEffect, StateField, type Extension, type Text } from "@codemirror/state"; import { EditorView, GutterMarker, ViewPlugin, gutter, type ViewUpdate } from "@codemirror/view"; -import type { EditorDocContext } from "@/editor/api"; +import { shownAgain, type EditorDocContext } from "@/editor/api"; import { navigationButton } from "./navigationButton"; import { pluginsBackend } from "./backend"; import { commands, type LineMarkerInfo } from "@/platform/ipc"; @@ -79,6 +79,7 @@ class PluginMarker extends GutterMarker { class MarkersPlugin { private timer: ReturnType | null = null; private generation = 0; + private skipped = false; private shown = false; private off: () => void; @@ -106,7 +107,7 @@ class MarkersPlugin { } update(u: ViewUpdate) { - if (u.docChanged) this.schedule(); + if (u.docChanged || (this.skipped && shownAgain(u))) this.schedule(); } private schedule() { @@ -128,7 +129,9 @@ class MarkersPlugin { icon: await resolveMarkerIcon(marker.pluginId, marker.icon), })), ); - if (gen !== this.generation || this.view.state.doc !== text || !this.view.dom.isConnected) return; + if (gen !== this.generation || this.view.state.doc !== text) return; + this.skipped = !this.view.dom.isConnected; + if (this.skipped) return; const placed = placeMarkers(text, resolved); if (!placed.length && !this.view.state.field(markersField).length) return; const effects: StateEffect[] = [setMarkers.of(placed)]; diff --git a/src/features/search/__tests__/dialogs.test.tsx b/src/features/search/__tests__/dialogs.test.tsx index 8b5f528ad..ad17e6beb 100644 --- a/src/features/search/__tests__/dialogs.test.tsx +++ b/src/features/search/__tests__/dialogs.test.tsx @@ -479,7 +479,7 @@ describe("Recent Files multi-selection", () => { describe("Switcher", () => { afterEach(() => vi.restoreAllMocks()); - it("opens the selection when Ctrl is released, once the popup gave focus back, in the same task", async () => { + it("opens the selection when Ctrl is released, once the popup closed, in the same task", async () => { const tab = (key: string, lastActive: number) => ({ input: { kind: "agent-session", key }, title: key, @@ -508,7 +508,9 @@ describe("Switcher", () => { await tick(); expect(screen.getByRole("dialog", { name: "Switcher" })).toBeInTheDocument(); act(() => void fireEvent.keyUp(window, { key: "Control" })); - expect(seen).toEqual([{ key: "task-a", popup: false, focus: editor }]); + // The editor opened takes focus itself: the one left behind doesn't get it back first. + expect(seen).toEqual([{ key: "task-a", popup: false, focus: document.body }]); + expect(editor).not.toHaveFocus(); }); }); diff --git a/src/features/search/recent/RecentPopups.tsx b/src/features/search/recent/RecentPopups.tsx index 68013c4a7..47e37096a 100644 --- a/src/features/search/recent/RecentPopups.tsx +++ b/src/features/search/recent/RecentPopups.tsx @@ -62,11 +62,22 @@ export function RecentPopupsOverlay() { const NAV_KEYS = ["ArrowDown", "ArrowUp", "PageDown", "PageUp"]; +let handingFocus = false; +// Asked when the dialog's effects are cleaned up, which may come after `fn` already ran. +const keepFocus = () => { + const keep = !handingFocus; + handingFocus = false; + return keep; +}; + /** * Runs `fn` once the dialog has unmounted and given focus back, in the same frame: the * popup closing and what it opens paint together, without a frame of the old editor between. + * `takesFocus`: what `fn` opens focuses itself, so focus isn't given back first (focusing the + * editor left behind would lay out the window once more). */ -const afterClose = (fn: () => void) => { +const afterClose = (fn: () => void, takesFocus = false) => { + handingFocus = takesFocus; flushSync(hideRecentPopup); fn(); }; @@ -143,10 +154,12 @@ function RecentFilesDialog() { const openFile = (item: RecentFileItem | undefined, side = false) => { if (!item) return; const editor = item.editor; - afterClose(() => - editor - ? void maybeEditorService()?.openInput(editor.input, { side, focus: true }) - : void openLocation(item.path, null, { side }), + afterClose( + () => + editor + ? void maybeEditorService()?.openInput(editor.input, { side, focus: true }) + : void openLocation(item.path, null, { side }), + !!editor, ); }; @@ -282,7 +295,14 @@ function RecentFilesDialog() { const title = switcher ? "Switcher" : editedOnly ? "Recently Edited Files" : "Recent Files"; const footerPath = (selected?.editor ? selected.rel : selected?.path) ?? (selectedTool ? "" : (root ?? "")); return ( - +
{title} diff --git a/src/ui/Dialog.tsx b/src/ui/Dialog.tsx index 3e6d9c119..03e757e2a 100644 --- a/src/ui/Dialog.tsx +++ b/src/ui/Dialog.tsx @@ -135,6 +135,11 @@ export interface DialogProps { dismissOnBackdrop?: boolean; /** Close when the app window loses focus (default: popups only; never modal dialogs). */ closeOnDeactivate?: boolean; + /** + * Asked on close: give focus back to what had it before the dialog (default: always). False + * when what the dialog opens takes focus itself. + */ + restoreFocus?: () => boolean; /** @deprecated Dialogs never dim the window behind them; kept for call sites. */ transparentBackdrop?: boolean; /** @deprecated Every centred dialog has window chrome; kept for call sites. */ @@ -219,6 +224,7 @@ function DialogInner({ className, dismissOnBackdrop, closeOnDeactivate, + restoreFocus, platform: platformOverride, icon, ...aria @@ -231,9 +237,11 @@ function DialogInner({ const popup = position === "top" || (title == null && !icon); const onCloseRef = useRef(onClose); const buttonsRef = useRef(buttons); + const restoreFocusRef = useRef(restoreFocus); useLayoutEffect(() => { onCloseRef.current = onClose; buttonsRef.current = buttons; + restoreFocusRef.current = restoreFocus; }); const [offset, setOffset] = useState({ x: 0, y: 0 }); @@ -299,7 +307,7 @@ function DialogInner({ return () => { document.removeEventListener("keydown", onKey); layer.release(); - if (previous?.isConnected) previous.focus({ preventScroll: true }); + if (previous?.isConnected && restoreFocusRef.current?.() !== false) previous.focus({ preventScroll: true }); }; }, []); diff --git a/src/ui/Tabs.tsx b/src/ui/Tabs.tsx index 6f1f5bddb..2ad4afdbd 100644 --- a/src/ui/Tabs.tsx +++ b/src/ui/Tabs.tsx @@ -215,18 +215,43 @@ export function Tabs({ }); }, [dndType]); + // Tab slots wholly in view as last observed, which needs no layout read: switching to one of + // them doesn't measure the strip (a forced layout of the window, mid-switch). + const inView = useRef(new Set()); // Bring a tab's whole slot (label + close button) into view. Narrowing the strip clips // the active tab's close button (#1740), and clicking the already-active tab doesn't // change `activeId`, so the strip must also reveal on clicks and resizes. const revealTab = useCallback( - (id: string | null) => { + (id: string | null, always = false) => { const scroller = scrollRef.current; const tab = id ? tabRefs.current.get(id)?.parentElement : null; - if (scroller && tab) (vertical ? revealVertically : revealHorizontally)(scroller, tab); + if (!scroller || !tab || (!always && inView.current.has(tab))) return; + (vertical ? revealVertically : revealHorizontally)(scroller, tab); }, [vertical], ); + const tabIds = items.map((it) => it.id).join("\n"); + useEffect(() => { + const root = scrollRef.current; + if (!root || typeof IntersectionObserver === "undefined") return; + const seen = inView.current; + const io = new IntersectionObserver( + (entries) => { + for (const e of entries) { + if (e.intersectionRatio >= 1) seen.add(e.target); + else seen.delete(e.target); + } + }, + { root, threshold: 1 }, + ); + for (const tab of tabRefs.current.values()) if (tab.parentElement) io.observe(tab.parentElement); + return () => { + io.disconnect(); + seen.clear(); + }; + }, [tabIds]); + useLayoutEffect(() => { const el = scrollRef.current; if (!el) return; @@ -235,7 +260,7 @@ export function Tabs({ setOverflow(!vertical && el.scrollWidth > el.clientWidth + 1); // A resize (e.g. the editor area narrowing) must scroll the active tab back // into view instead of leaving its close button clipped until a tab switch. - revealTab(latest.current.activeId); + revealTab(latest.current.activeId, true); }; measure(); if (typeof ResizeObserver === "undefined") return; diff --git a/src/workbench/focus.test.ts b/src/workbench/focus.test.ts index 965176321..62ed4eae1 100644 --- a/src/workbench/focus.test.ts +++ b/src/workbench/focus.test.ts @@ -38,6 +38,21 @@ describe("focus after popups", () => { dispose(); }); + it("leaves the caret to the next editor when focus moves between editors", () => { + document.body.innerHTML = `
first
second
`; + const [first, second] = document.querySelectorAll(".cm-content"); + const dispose = installFocusTracking(() => {}); + first!.focus(); + document.getSelection()!.collapse(first!.firstChild, 3); + let caretLeft: Node | null = null; + const probe = () => (caretLeft = document.getSelection()!.anchorNode); + document.addEventListener("focusout", probe); + second!.focus(); + document.removeEventListener("focusout", probe); + expect(caretLeft).toBe(first!.firstChild); + dispose(); + }); + it("drops the editor's caret when focus moves to a menu, so focusing the editor again cannot scroll to it", () => { document.body.innerHTML = `
first line
`; const editor = document.querySelector(".cm-content")!; diff --git a/src/workbench/focus.ts b/src/workbench/focus.ts index 03b2426a7..54e133f1b 100644 --- a/src/workbench/focus.ts +++ b/src/workbench/focus.ts @@ -39,6 +39,9 @@ function dropEditorCaret(e: FocusEvent) { // The window lost focus and keeps the editor focused for its return. if (document.activeElement === editor) return; if (e.relatedTarget instanceof Node && editor.contains(e.relatedTarget)) return; + // Another editor takes the DOM selection itself; a hidden (detached) editor has none left. + // Dropping it would only lay out the window once more while switching editors. + if (!editor.isConnected || (e.relatedTarget instanceof Element && e.relatedTarget.matches(".cm-content"))) return; const selection = document.getSelection(); if (selection?.anchorNode && editor.contains(selection.anchorNode)) selection.removeAllRanges(); }