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 0e7d9f626..d0cc1cc78 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 7079770ff..eb8afa2d5 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} @@ -386,6 +387,7 @@ export function EditorContent({ { + 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 000000000..8127c86f3 --- /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.test.ts b/src/renderer/src/components/editor/rich-markdown-auto-focus.test.ts index b93bb14d5..4a1c02f80 100644 --- a/src/renderer/src/components/editor/rich-markdown-auto-focus.test.ts +++ b/src/renderer/src/components/editor/rich-markdown-auto-focus.test.ts @@ -1,11 +1,16 @@ +// @vitest-environment happy-dom import type { Editor } from '@tiptap/react' import { afterEach, describe, expect, it, vi } from 'vitest' import { autoFocusRichEditor } from './rich-markdown-auto-focus' -function createEditor(focus = vi.fn()): Editor { +function createEditor( + focus = vi.fn(), + domFocus: (options?: FocusOptions) => void = vi.fn() +): Editor { return { isDestroyed: false, - commands: { focus } + commands: { focus }, + view: { dom: { focus: domFocus } } } as unknown as Editor } @@ -37,6 +42,7 @@ function setupScheduledFocus( describe('autoFocusRichEditor', () => { afterEach(() => { vi.unstubAllGlobals() + document.body.replaceChildren() }) it('returns cleanup that cancels the pending focus frame', () => { @@ -69,6 +75,53 @@ describe('autoFocusRichEditor', () => { expect(focus).toHaveBeenCalledWith('start', { scrollIntoView: false }) }) + it('claims DOM focus synchronously on an explicit handoff', () => { + const root = document.createElement('div') + const editorDom = document.createElement('div') + editorDom.tabIndex = -1 + root.append(editorDom) + document.body.append(root) + vi.stubGlobal( + 'requestAnimationFrame', + vi.fn(() => 11) + ) + vi.stubGlobal('cancelAnimationFrame', vi.fn()) + + autoFocusRichEditor(createEditor(vi.fn(), editorDom.focus.bind(editorDom)), root, true) + + expect(root.contains(document.activeElement)).toBe(true) + }) + + it('leaves DOM focus alone for an ordinary lazy mount', () => { + const domFocus = vi.fn() + vi.stubGlobal( + 'requestAnimationFrame', + vi.fn(() => 12) + ) + vi.stubGlobal('cancelAnimationFrame', vi.fn()) + + autoFocusRichEditor(createEditor(vi.fn(), domFocus), null, false) + + expect(domFocus).not.toHaveBeenCalled() + }) + + it('does not run deferred focus after an explicit handoff expires', () => { + let runFrame: FrameRequestCallback = () => {} + let requestActive = true + const focus = vi.fn() + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + runFrame = callback + return 13 + }) + vi.stubGlobal('cancelAnimationFrame', vi.fn()) + + autoFocusRichEditor(createEditor(focus), null, true, () => requestActive) + requestActive = false + runFrame(0) + + expect(focus).not.toHaveBeenCalled() + }) + it('does not steal focus from other controls outside the editor', () => { const { focus, runFrame } = setupScheduledFocus({}) runFrame() 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 eb3aa2533..5d9a62f3d 100644 --- a/src/renderer/src/components/editor/rich-markdown-auto-focus.ts +++ b/src/renderer/src/components/editor/rich-markdown-auto-focus.ts @@ -5,15 +5,25 @@ 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, rootEl: HTMLElement | null, - force = false + force = false, + shouldFocus: () => boolean = () => true ): () => void { + // Why: Tiptap can recreate the instance before its deferred focus lands, losing explicit handoffs. + if (force && !nextEditor.isDestroyed && shouldFocus()) { + nextEditor.view?.dom?.focus?.({ preventScroll: true }) + } let frameId: number | null = requestAnimationFrame(() => { frameId = null - if (nextEditor.isDestroyed) { + if (nextEditor.isDestroyed || !shouldFocus()) { return } const active = document.activeElement diff --git a/src/renderer/src/components/editor/rich-markdown-editor-props.ts b/src/renderer/src/components/editor/rich-markdown-editor-props.ts index 6a27e813f..04a20c13c 100644 --- a/src/renderer/src/components/editor/rich-markdown-editor-props.ts +++ b/src/renderer/src/components/editor/rich-markdown-editor-props.ts @@ -3,6 +3,7 @@ import type { MarkdownDocument } from '../../../../shared/types' export type RichMarkdownEditorProps = { fileId: string + viewStateId: string content: string filePath: string worktreeId: string diff --git a/src/renderer/src/components/editor/useRichMarkdownPendingFocus.test.ts b/src/renderer/src/components/editor/useRichMarkdownPendingFocus.test.ts new file mode 100644 index 000000000..a2bbf0079 --- /dev/null +++ b/src/renderer/src/components/editor/useRichMarkdownPendingFocus.test.ts @@ -0,0 +1,163 @@ +// @vitest-environment happy-dom +import type { Editor } from '@tiptap/react' +import { act, renderHook } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { PendingEditorFocusRequest } from '@/store/slices/editor' + +type StoreFixture = { + pendingEditorFocusRequest: PendingEditorFocusRequest | null + consumeEditorFocusRequest: ReturnType +} + +const fixture = vi.hoisted(() => ({ + store: { + pendingEditorFocusRequest: null, + consumeEditorFocusRequest: vi.fn() + } as StoreFixture, + autoFocusRichEditor: vi.fn(() => vi.fn()) +})) + +vi.mock('@/store', () => ({ + useAppStore: (selector: (state: StoreFixture) => unknown) => selector(fixture.store) +})) + +vi.mock('./rich-markdown-auto-focus', () => ({ + autoFocusRichEditor: fixture.autoFocusRichEditor +})) + +import { useRichMarkdownPendingFocus } from './useRichMarkdownPendingFocus' + +type EditorFixture = { + editor: Editor + focus: () => void +} + +function createEditorFixture(destroyed = false): EditorFixture { + const focusListeners = new Set<() => void>() + let focused = false + return { + editor: { + isDestroyed: destroyed, + get isFocused() { + return focused + }, + on: vi.fn((event: string, listener: () => void) => { + if (event === 'focus') { + focusListeners.add(listener) + } + }), + off: vi.fn((event: string, listener: () => void) => { + if (event === 'focus') { + focusListeners.delete(listener) + } + }) + } as unknown as Editor, + focus: () => { + focused = true + for (const listener of focusListeners) { + listener() + } + } + } +} + +function pendingRequest(overrides: Partial = {}) { + return { + fileId: 'file-1', + worktreeId: 'worktree-1', + viewStateId: 'view-1', + expiresAt: Date.now() + 30_000, + token: 7, + ...overrides + } +} + +function renderPendingFocus(editor: Editor | null, viewStateId = 'view-1') { + return renderHook( + ({ nextEditor }) => + useRichMarkdownPendingFocus({ + editor: nextEditor, + fileId: 'file-1', + viewStateId, + worktreeId: 'worktree-1', + rootRef: { current: null }, + cancelAutoFocusRef: { current: null } + }), + { initialProps: { nextEditor: editor } } + ) +} + +describe('useRichMarkdownPendingFocus', () => { + afterEach(() => { + vi.useRealTimers() + fixture.store.pendingEditorFocusRequest = null + fixture.store.consumeEditorFocusRequest.mockReset() + fixture.autoFocusRichEditor.mockReset() + fixture.autoFocusRichEditor.mockReturnValue(vi.fn()) + }) + + it('consumes the request only after delayed editor focus lands', () => { + const editor = createEditorFixture() + fixture.store.pendingEditorFocusRequest = pendingRequest() + + const hook = renderPendingFocus(editor.editor) + + expect(fixture.store.consumeEditorFocusRequest).not.toHaveBeenCalled() + act(editor.focus) + expect(fixture.store.consumeEditorFocusRequest).toHaveBeenCalledWith(7) + + hook.unmount() + expect(editor.editor.off).toHaveBeenCalledWith('focus', expect.any(Function)) + }) + + it('retries a request when Tiptap replaces a destroyed editor', () => { + const destroyed = createEditorFixture(true) + const replacement = createEditorFixture() + fixture.store.pendingEditorFocusRequest = pendingRequest() + fixture.autoFocusRichEditor.mockImplementationOnce(() => { + replacement.focus() + return vi.fn() + }) + + const hook = renderPendingFocus(destroyed.editor) + expect(fixture.autoFocusRichEditor).not.toHaveBeenCalled() + + hook.rerender({ nextEditor: replacement.editor }) + + expect(fixture.store.consumeEditorFocusRequest).toHaveBeenCalledWith(7) + }) + + it('does not let another split pane claim the request', () => { + fixture.store.pendingEditorFocusRequest = pendingRequest() + + renderPendingFocus(createEditorFixture().editor, 'view-2') + + expect(fixture.autoFocusRichEditor).not.toHaveBeenCalled() + expect(fixture.store.consumeEditorFocusRequest).not.toHaveBeenCalled() + }) + + it('retires an expired request without stealing focus', () => { + fixture.store.pendingEditorFocusRequest = pendingRequest({ expiresAt: Date.now() - 1 }) + + renderPendingFocus(null) + + expect(fixture.autoFocusRichEditor).not.toHaveBeenCalled() + expect(fixture.store.consumeEditorFocusRequest).toHaveBeenCalledWith(7) + }) + + it('cancels a pending forced focus when its request expires', () => { + vi.useFakeTimers() + vi.setSystemTime(new Date('2026-07-25T00:00:00Z')) + const cancelFocus = vi.fn() + fixture.store.pendingEditorFocusRequest = pendingRequest({ expiresAt: Date.now() + 1_000 }) + fixture.autoFocusRichEditor.mockReturnValue(cancelFocus) + + renderPendingFocus(createEditorFixture().editor) + act(() => { + vi.advanceTimersByTime(1_000) + }) + + expect(cancelFocus).toHaveBeenCalledOnce() + expect(fixture.store.consumeEditorFocusRequest).toHaveBeenCalledWith(7) + }) +}) diff --git a/src/renderer/src/components/editor/useRichMarkdownPendingFocus.ts b/src/renderer/src/components/editor/useRichMarkdownPendingFocus.ts index 372c22e64..22635bdc7 100644 --- a/src/renderer/src/components/editor/useRichMarkdownPendingFocus.ts +++ b/src/renderer/src/components/editor/useRichMarkdownPendingFocus.ts @@ -2,38 +2,78 @@ 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 fileId: string + viewStateId: string worktreeId: string rootRef: RefObject cancelAutoFocusRef: RefObject<(() => void) | null> } /** - * 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, fileId, + viewStateId, worktreeId, rootRef, cancelAutoFocusRef }: PendingFocusOptions): void { const pendingEditorFocusRequest = useAppStore((s) => { const request = s.pendingEditorFocusRequest - return request?.fileId === fileId && request.worktreeId === worktreeId ? request : null + return matchesPendingEditorFocusRequest(request, { fileId, worktreeId, viewStateId }) + ? request + : null }) const consumeEditorFocusRequest = useAppStore((s) => s.consumeEditorFocusRequest) useEffect(() => { - if (!editor || !pendingEditorFocusRequest) { + if (!pendingEditorFocusRequest) { return } + if (pendingEditorFocusRequest.expiresAt <= Date.now()) { + consumeEditorFocusRequest(pendingEditorFocusRequest.token) + return + } + if (!editor || editor.isDestroyed) { + return + } + let consumed = false + const consumeIfFocused = (): void => { + if ( + consumed || + (rootRef.current?.contains(document.activeElement) !== true && !editor.isFocused) + ) { + return + } + consumed = true + consumeEditorFocusRequest(pendingEditorFocusRequest.token) + } + editor.on('focus', consumeIfFocused) cancelAutoFocusRef.current?.() - cancelAutoFocusRef.current = autoFocusRichEditor(editor, rootRef.current, true) - consumeEditorFocusRequest(pendingEditorFocusRequest.token) + cancelAutoFocusRef.current = autoFocusRichEditor( + editor, + rootRef.current, + true, + () => pendingEditorFocusRequest.expiresAt > Date.now() + ) + const expiryTimer = window.setTimeout(() => { + cancelAutoFocusRef.current?.() + cancelAutoFocusRef.current = null + consumeEditorFocusRequest(pendingEditorFocusRequest.token) + }, pendingEditorFocusRequest.expiresAt - Date.now()) + consumeIfFocused() + return () => { + window.clearTimeout(expiryTimer) + editor.off('focus', consumeIfFocused) + } }, [cancelAutoFocusRef, consumeEditorFocusRequest, editor, pendingEditorFocusRequest, rootRef]) } diff --git a/src/renderer/src/store/slices/editor.test.ts b/src/renderer/src/store/slices/editor.test.ts index e0860b457..89d43a388 100644 --- a/src/renderer/src/store/slices/editor.test.ts +++ b/src/renderer/src/store/slices/editor.test.ts @@ -121,7 +121,12 @@ describe('createEditorSlice right sidebar state', () => { ) const request = store.getState().pendingEditorFocusRequest - expect(request).toMatchObject({ fileId: '/repo/README.md', worktreeId: 'wt-1' }) + expect(request).toMatchObject({ + fileId: '/repo/README.md', + worktreeId: 'wt-1', + viewStateId: expect.any(String), + expiresAt: expect.any(Number) + }) store.getState().consumeEditorFocusRequest((request?.token ?? 0) + 1) expect(store.getState().pendingEditorFocusRequest).toBe(request) @@ -130,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() diff --git a/src/renderer/src/store/slices/editor.ts b/src/renderer/src/store/slices/editor.ts index dcabf350b..75be706d5 100644 --- a/src/renderer/src/store/slices/editor.ts +++ b/src/renderer/src/store/slices/editor.ts @@ -316,9 +316,13 @@ export type PendingEditorReveal = { export type PendingEditorFocusRequest = { fileId: string worktreeId: string + viewStateId: string + expiresAt: number token: number } +// Why: allow slow SSH mounts without leaving an unrelated future remount armed indefinitely. +const EDITOR_FOCUS_REQUEST_TTL_MS = 30_000 let nextEditorFocusRequestToken = 0 const pendingEditorLineRevealFrameIds = new Set() @@ -1660,18 +1664,6 @@ export const createEditorSlice: StateCreator = (s resolveEditorOpenTargetGroupId(s, worktreeId, options?.targetGroupId) ?? undefined editorItemTargetGroupId = targetGroupId const activeResult = buildEditorActiveResult(s, worktreeId, id) - // Why: the renderer may mount asynchronously after the opening control - // receives DOM focus, so carry the user's explicit focus handoff by file. - const focusRequestUpdate = options?.focusEditor - ? { - pendingEditorFocusRequest: { - fileId: id, - worktreeId, - token: ++nextEditorFocusRequestToken - } - } - : {} - if (existing) { // If opening as non-preview, also pin the existing tab const updatedPreview = isPreview ? existing.isPreview : false @@ -1702,7 +1694,7 @@ export const createEditorSlice: StateCreator = (s refreshExternalSshProvenance || existing.fileContentReloadNonce !== fileContentReloadNonce if (!needsExistingUpdate) { - return { ...activeResult, ...focusRequestUpdate } + return activeResult } // Why: `readOnly` is intentionally NOT in this override map — it's sticky, so `...f` preserves the tab's own read-only state. return { @@ -1734,8 +1726,7 @@ export const createEditorSlice: StateCreator = (s } : f ), - ...activeResult, - ...focusRequestUpdate + ...activeResult } } @@ -1813,8 +1804,7 @@ export const createEditorSlice: StateCreator = (s recentlyClosedEditorTabsByWorktree: nextRecentlyClosed, recentlyClosedTabKindsByWorktree: nextRecentlyClosedKinds, ...previewTabBarUpdate, - ...activeResult, - ...focusRequestUpdate + ...activeResult } } } @@ -1854,11 +1844,10 @@ export const createEditorSlice: StateCreator = (s } ], ...tabBarUpdate, - ...activeResult, - ...focusRequestUpdate + ...activeResult } }) - void openWorkspaceEditorItem( + const editorItemViewStateId = openWorkspaceEditorItem( get(), editorItemFileId, editorItemWorktreeId, @@ -1867,6 +1856,17 @@ export const createEditorSlice: StateCreator = (s options?.preview ?? false, editorItemTargetGroupId ) + if (options?.focusEditor) { + set({ + pendingEditorFocusRequest: { + fileId: editorItemFileId, + worktreeId: editorItemWorktreeId, + viewStateId: editorItemViewStateId, + expiresAt: Date.now() + EDITOR_FOCUS_REQUEST_TTL_MS, + token: ++nextEditorFocusRequestToken + } + }) + } }, openNewMarkdownInActiveWorkspace: async (groupId) => {