From d1518ce9f6fb48c52fa98635fa7ad59342dd44d4 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sat, 25 Jul 2026 15:21:28 -0700 Subject: [PATCH] fix(editor): scope focus requests to panes via viewStateId When opening a file to focus it, tag the pending request with the pane's viewStateId. This prevents split siblings from claiming each other's requests and stops later remounts from stealing focus. Both Monaco and rich-markdown editors now retire requests on mount. --- .../EditorContent.monaco-lifecycle.test.tsx | 23 +++++++-- .../src/components/editor/EditorContent.tsx | 1 + .../src/components/editor/MonacoEditor.tsx | 15 ++++++ .../pending-editor-focus-request.test.ts | 47 +++++++++++++++++++ .../editor/pending-editor-focus-request.ts | 20 ++++++++ .../editor/rich-markdown-auto-focus.ts | 5 ++ .../editor/useRichMarkdownPendingFocus.ts | 11 +++-- src/renderer/src/store/slices/editor.test.ts | 27 +++++++++++ 8 files changed, 141 insertions(+), 8 deletions(-) create mode 100644 src/renderer/src/components/editor/pending-editor-focus-request.test.ts create mode 100644 src/renderer/src/components/editor/pending-editor-focus-request.ts diff --git a/src/renderer/src/components/editor/EditorContent.monaco-lifecycle.test.tsx b/src/renderer/src/components/editor/EditorContent.monaco-lifecycle.test.tsx index 0e7d9f6262a..d0cc1cc78d9 100644 --- a/src/renderer/src/components/editor/EditorContent.monaco-lifecycle.test.tsx +++ b/src/renderer/src/components/editor/EditorContent.monaco-lifecycle.test.tsx @@ -7,7 +7,12 @@ const lifecycle = vi.hoisted(() => ({ events: [] as string[], diffModelKeys: [] as string[], models: new Map(), - mountedProps: [] as { filePath: string; readOnly?: boolean; liveTail?: boolean }[] + mountedProps: [] as { + filePath: string + readOnly?: boolean + liveTail?: boolean + viewStateId?: string + }[] })) vi.mock('@/lib/lazy-with-retry', async () => { @@ -37,6 +42,7 @@ vi.mock('@/lib/lazy-with-retry', async () => { content: string readOnly?: boolean liveTail?: boolean + viewStateId?: string }) { /* oxlint-disable react-hooks/exhaustive-deps -- Mount-only by design: a prop-effect would hide a missing outer React remount. */ React.useEffect(() => { @@ -44,7 +50,8 @@ vi.mock('@/lib/lazy-with-retry', async () => { lifecycle.mountedProps.push({ filePath: props.filePath, readOnly: props.readOnly, - liveTail: props.liveTail + liveTail: props.liveTail, + viewStateId: props.viewStateId }) const retained = lifecycle.models.get(props.filePath) ?? { content: '', undo: [] } const model = { @@ -241,7 +248,17 @@ describe('EditorContent Monaco lifecycle boundary', () => { render() expect(lifecycle.mountedProps).toEqual([ - { filePath: liveLog.filePath, readOnly: true, liveTail: true } + { filePath: liveLog.filePath, readOnly: true, liveTail: true, viewStateId: 'same-pane' } ]) }) + + it('tells Monaco which pane it is so it can retire an explicit focus handoff', () => { + const source = file('/repo/main.ts') + + render() + + // Why: without the pane id Monaco cannot match the handoff, so it silently leaves the request + // armed and a later rich-mode remount of this pane steals focus back. + expect(lifecycle.mountedProps.at(0)?.viewStateId).toBe('same-pane') + }) }) diff --git a/src/renderer/src/components/editor/EditorContent.tsx b/src/renderer/src/components/editor/EditorContent.tsx index 35ce23f5b0b..eb8afa2d5c9 100644 --- a/src/renderer/src/components/editor/EditorContent.tsx +++ b/src/renderer/src/components/editor/EditorContent.tsx @@ -307,6 +307,7 @@ export function EditorContent({ fileId={activeFile.id} filePath={activeFile.filePath} viewStateKey={editorViewStateKey} + viewStateId={viewStateScopeId} relativePath={activeFile.relativePath} content={editBuffers[activeFile.id] ?? fc.content} language={monacoLanguage} diff --git a/src/renderer/src/components/editor/MonacoEditor.tsx b/src/renderer/src/components/editor/MonacoEditor.tsx index c6996ad341e..417aaccf62e 100644 --- a/src/renderer/src/components/editor/MonacoEditor.tsx +++ b/src/renderer/src/components/editor/MonacoEditor.tsx @@ -66,11 +66,14 @@ import { } from './monaco-auto-height' import { installMonacoE2EProbe } from './monaco-e2e-probe' import { monacoFindOptions } from './monaco-find-options' +import { matchesPendingEditorFocusRequest } from './pending-editor-focus-request' type MonacoEditorProps = { fileId: string filePath: string viewStateKey: string + // Why: identifies the pane for explicit open focus handoffs; omit on surfaces that never receive one. + viewStateId?: string relativePath: string content: string language: string @@ -96,6 +99,7 @@ export default function MonacoEditor({ fileId, filePath, viewStateKey, + viewStateId, relativePath, content, language, @@ -559,6 +563,16 @@ export default function MonacoEditor({ editorInstance.focus() } } + + // Why: every mount path above focuses, so an explicit open handoff is already satisfied here. + // Retiring it stops a later rich-mode remount of this same pane from stealing focus back. + const focusRequest = useAppStore.getState().pendingEditorFocusRequest + if ( + focusRequest && + matchesPendingEditorFocusRequest(focusRequest, { fileId, worktreeId, viewStateId }) + ) { + useAppStore.getState().consumeEditorFocusRequest(focusRequest.token) + } }, [ queueReveal, @@ -568,6 +582,7 @@ export default function MonacoEditor({ setEditorCursorLine, updateMarkdownCompletionDocuments, viewStateKey, + viewStateId, autoHeight, autoHeightLineHeight, worktreeId diff --git a/src/renderer/src/components/editor/pending-editor-focus-request.test.ts b/src/renderer/src/components/editor/pending-editor-focus-request.test.ts new file mode 100644 index 00000000000..e3625800a6f --- /dev/null +++ b/src/renderer/src/components/editor/pending-editor-focus-request.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from 'vitest' +import type { PendingEditorFocusRequest } from '@/store/slices/editor' +import { matchesPendingEditorFocusRequest } from './pending-editor-focus-request' + +const request: PendingEditorFocusRequest = { + fileId: 'file-1', + worktreeId: 'worktree-1', + viewStateId: 'view-1', + expiresAt: 0, + token: 3 +} + +const pane = { fileId: 'file-1', worktreeId: 'worktree-1', viewStateId: 'view-1' } + +describe('matchesPendingEditorFocusRequest', () => { + it('matches the pane the handoff was opened into', () => { + expect(matchesPendingEditorFocusRequest(request, pane)).toBe(true) + }) + + it('rejects a missing request', () => { + expect(matchesPendingEditorFocusRequest(null, pane)).toBe(false) + expect(matchesPendingEditorFocusRequest(undefined, pane)).toBe(false) + }) + + it('rejects a split sibling showing the same file', () => { + expect(matchesPendingEditorFocusRequest(request, { ...pane, viewStateId: 'view-2' })).toBe( + false + ) + }) + + it('rejects the same pane id in another file or worktree', () => { + expect(matchesPendingEditorFocusRequest(request, { ...pane, fileId: 'file-2' })).toBe(false) + expect(matchesPendingEditorFocusRequest(request, { ...pane, worktreeId: 'worktree-2' })).toBe( + false + ) + }) + + it('rejects a pane that cannot identify itself', () => { + // Why: surfaces that omit the props must never swallow another pane's handoff. + expect(matchesPendingEditorFocusRequest(request, { ...pane, viewStateId: undefined })).toBe( + false + ) + expect(matchesPendingEditorFocusRequest(request, { ...pane, worktreeId: undefined })).toBe( + false + ) + }) +}) diff --git a/src/renderer/src/components/editor/pending-editor-focus-request.ts b/src/renderer/src/components/editor/pending-editor-focus-request.ts new file mode 100644 index 00000000000..8127c86f3df --- /dev/null +++ b/src/renderer/src/components/editor/pending-editor-focus-request.ts @@ -0,0 +1,20 @@ +import type { PendingEditorFocusRequest } from '@/store/slices/editor' + +/** + * True when an explicit open focus handoff belongs to this editor pane. Both the rich Markdown and + * Monaco surfaces gate on this, so a handoff is claimed (and retired) by exactly one pane — split + * siblings share a file id, and only `viewStateId` tells them apart. + */ +export function matchesPendingEditorFocusRequest( + request: PendingEditorFocusRequest | null | undefined, + pane: { fileId: string; worktreeId: string | undefined; viewStateId: string | undefined } +): boolean { + if (!request || pane.worktreeId === undefined || pane.viewStateId === undefined) { + return false + } + return ( + request.fileId === pane.fileId && + request.worktreeId === pane.worktreeId && + request.viewStateId === pane.viewStateId + ) +} diff --git a/src/renderer/src/components/editor/rich-markdown-auto-focus.ts b/src/renderer/src/components/editor/rich-markdown-auto-focus.ts index f19d293cc95..5d9a62f3d71 100644 --- a/src/renderer/src/components/editor/rich-markdown-auto-focus.ts +++ b/src/renderer/src/components/editor/rich-markdown-auto-focus.ts @@ -5,6 +5,11 @@ import type { Editor } from '@tiptap/react' * immediately (matching MonacoEditor's behavior). Guards against focus theft * from modals/dialogs and skips scrollIntoView to avoid racing with * useEditorScrollRestore. + * + * `force` marks an explicit user handoff (Explorer open): it bypasses the theft + * guard and claims DOM focus in this tick, because `commands.focus()` defers the + * real `view.focus()` by a further frame. `shouldFocus` lets the caller retire a + * handoff that expired while the frame was pending. */ export function autoFocusRichEditor( nextEditor: Editor, diff --git a/src/renderer/src/components/editor/useRichMarkdownPendingFocus.ts b/src/renderer/src/components/editor/useRichMarkdownPendingFocus.ts index 16603e789f5..22635bdc7a6 100644 --- a/src/renderer/src/components/editor/useRichMarkdownPendingFocus.ts +++ b/src/renderer/src/components/editor/useRichMarkdownPendingFocus.ts @@ -2,6 +2,7 @@ import { useEffect, type RefObject } from 'react' import type { Editor } from '@tiptap/react' import { useAppStore } from '@/store' import { autoFocusRichEditor } from './rich-markdown-auto-focus' +import { matchesPendingEditorFocusRequest } from './pending-editor-focus-request' type PendingFocusOptions = { editor: Editor | null @@ -13,8 +14,10 @@ type PendingFocusOptions = { } /** - * Focuses the editor when the Explorer opens this document for find (issue #8083), then consumes - * the request so a later remount of the same file does not steal focus again. + * Focuses the editor when the Explorer opens this document for find (issue #8083). The request is + * scoped to one pane (`viewStateId`) so split siblings can't claim it, and stays armed until focus + * actually lands — Tiptap can replace the instance first — or until its TTL retires it, so a later + * unrelated remount of the same file never steals focus. */ export function useRichMarkdownPendingFocus({ editor, @@ -26,9 +29,7 @@ export function useRichMarkdownPendingFocus({ }: PendingFocusOptions): void { const pendingEditorFocusRequest = useAppStore((s) => { const request = s.pendingEditorFocusRequest - return request?.fileId === fileId && - request.worktreeId === worktreeId && - request.viewStateId === viewStateId + return matchesPendingEditorFocusRequest(request, { fileId, worktreeId, viewStateId }) ? request : null }) diff --git a/src/renderer/src/store/slices/editor.test.ts b/src/renderer/src/store/slices/editor.test.ts index 3621dcc3e0f..89d43a38873 100644 --- a/src/renderer/src/store/slices/editor.test.ts +++ b/src/renderer/src/store/slices/editor.test.ts @@ -135,6 +135,33 @@ describe('createEditorSlice right sidebar state', () => { expect(store.getState().pendingEditorFocusRequest).toBeNull() }) + it('scopes the focus request to the unified tab that will render the file', () => { + const store = createEditorTabsStore() + const sourceTab = store.getState().createUnifiedTab('wt-1', 'terminal', { id: 'terminal-1' }) + const targetGroupId = store.getState().createEmptySplitGroup('wt-1', sourceTab.groupId, 'right') + if (!targetGroupId) { + throw new Error('expected split group') + } + + store.getState().openFile( + { + filePath: '/repo/README.md', + relativePath: 'README.md', + worktreeId: 'wt-1', + language: 'markdown', + mode: 'edit' + }, + { focusEditor: true, targetGroupId } + ) + + const editorTab = store + .getState() + .unifiedTabsByWorktree['wt-1']?.find((tab) => tab.contentType === 'editor') + expect(editorTab?.groupId).toBe(targetGroupId) + // Why: the pane matches the handoff on its own tab id, so a drifting id silently drops it. + expect(store.getState().pendingEditorFocusRequest?.viewStateId).toBe(editorTab?.id) + }) + it('does not record markdown-file-created when opening an existing markdown file', () => { const store = createEditorStore()