mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 08:03:13 +00:00
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.
This commit is contained in:
@@ -7,7 +7,12 @@ const lifecycle = vi.hoisted(() => ({
|
||||
events: [] as string[],
|
||||
diffModelKeys: [] as string[],
|
||||
models: new Map<string, { content: string; undo: string[] }>(),
|
||||
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(<EditorContent {...props(liveLog, 'session content')} />)
|
||||
|
||||
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(<EditorContent {...props(source, 'source content')} />)
|
||||
|
||||
// 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')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -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
|
||||
)
|
||||
}
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
})
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user