fix(cmd-j): show cross-project dialog on paste + restore focus on Escape (#8931)
This commit is contained in:
parent
78d2b958bf
commit
49e2ac6f59
|
|
@ -105,16 +105,14 @@ import {
|
|||
CREATE_WORKSPACE_QUICK_ACTION_ID
|
||||
} from '@/components/cmd-j/quick-actions'
|
||||
import { buildWorktreeChecksReviewIndex } from '@/components/cmd-j/worktree-checks-review-index'
|
||||
import { resolvePaletteFocusRestoreTarget } from '@/components/cmd-j/palette-focus-restore-target'
|
||||
import { selectWorktreePaletteCacheInputs } from '@/components/cmd-j/worktree-palette-cache-inputs'
|
||||
import { getRepoHostIdentity } from '@/store/slices/repo-host-identity'
|
||||
import {
|
||||
getComposerEligibleRepos,
|
||||
resolveComposerGitRepoId
|
||||
} from '@/lib/new-workspace-composer-repo'
|
||||
import {
|
||||
lookupGitHubWorkItemByOwnerRepoForSource,
|
||||
lookupGitHubWorkItemForSource
|
||||
} from '@/lib/github-work-item-source-lookup'
|
||||
import { lookupGitHubWorkItemForSource } from '@/lib/github-work-item-source-lookup'
|
||||
import type { SettingsNavTarget } from '@/lib/settings-navigation-types'
|
||||
import { getHostDisplayLabelOverrides } from '../../../shared/host-setting-overrides'
|
||||
import { isRuntimeOwnedSshTargetId } from '../../../shared/execution-host'
|
||||
|
|
@ -433,6 +431,11 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
|||
)
|
||||
const previousBrowserPageIdRef = useRef<string | null>(null)
|
||||
const previousBrowserFocusTargetRef = useRef<'webview' | 'address-bar'>('webview')
|
||||
// Why: the exact element focused before Cmd+J opened (e.g. the terminal
|
||||
// textarea the user was typing in) so Escape restores it precisely instead
|
||||
// of the first matching surface in the DOM, which may be a background
|
||||
// worktree's mounted-but-hidden terminal.
|
||||
const previousFocusElementRef = useRef<HTMLElement | null>(null)
|
||||
const activeGroupSnapshotRef = useRef<CmdJActiveGroupSnapshot | null>(null)
|
||||
const wasVisibleRef = useRef(false)
|
||||
const skipRestoreFocusRef = useRef(false)
|
||||
|
|
@ -1177,6 +1180,13 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
|||
document.activeElement.closest('[data-orca-browser-address-bar="true"]')
|
||||
? 'address-bar'
|
||||
: 'webview'
|
||||
// Why: same timing constraint — capture the pre-dialog focus target now
|
||||
// so Escape can return focus to the exact input the user left (excluding
|
||||
// document.body, which isn't a meaningful restore target).
|
||||
previousFocusElementRef.current =
|
||||
document.activeElement instanceof HTMLElement && document.activeElement !== document.body
|
||||
? document.activeElement
|
||||
: null
|
||||
skipRestoreFocusRef.current = false
|
||||
setQuery('')
|
||||
setSelectedItemId('')
|
||||
|
|
@ -1243,24 +1253,19 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
|||
|
||||
useEffect(() => cancelFallbackFocusFrames, [cancelFallbackFocusFrames])
|
||||
|
||||
const focusFallbackSurface = useCallback(() => {
|
||||
cancelFallbackFocusFrames()
|
||||
fallbackFocusOuterFrameRef.current = requestAnimationFrame(() => {
|
||||
fallbackFocusOuterFrameRef.current = null
|
||||
fallbackFocusInnerFrameRef.current = requestAnimationFrame(() => {
|
||||
fallbackFocusInnerFrameRef.current = null
|
||||
const xterm = document.querySelector('.xterm-helper-textarea') as HTMLElement | null
|
||||
if (xterm) {
|
||||
xterm.focus()
|
||||
return
|
||||
}
|
||||
const monaco = document.querySelector('.monaco-editor textarea') as HTMLElement | null
|
||||
if (monaco) {
|
||||
monaco.focus()
|
||||
}
|
||||
const focusFallbackSurface = useCallback(
|
||||
(preferredTarget?: HTMLElement | null) => {
|
||||
cancelFallbackFocusFrames()
|
||||
fallbackFocusOuterFrameRef.current = requestAnimationFrame(() => {
|
||||
fallbackFocusOuterFrameRef.current = null
|
||||
fallbackFocusInnerFrameRef.current = requestAnimationFrame(() => {
|
||||
fallbackFocusInnerFrameRef.current = null
|
||||
resolvePaletteFocusRestoreTarget(preferredTarget ?? null)?.focus({ preventScroll: true })
|
||||
})
|
||||
})
|
||||
})
|
||||
}, [cancelFallbackFocusFrames])
|
||||
},
|
||||
[cancelFallbackFocusFrames]
|
||||
)
|
||||
|
||||
const requestBrowserFocus = useCallback(
|
||||
(detail: { pageId: string; target: 'webview' | 'address-bar' }) => {
|
||||
|
|
@ -1294,7 +1299,10 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
|||
return
|
||||
}
|
||||
if (previousWorktreeIdRef.current) {
|
||||
focusFallbackSurface()
|
||||
// Why: dismissing Cmd+J should return to whatever the user was doing —
|
||||
// restore the exact previously-focused surface (e.g. the terminal they
|
||||
// were typing in) rather than an arbitrary first match.
|
||||
focusFallbackSurface(previousFocusElementRef.current)
|
||||
}
|
||||
},
|
||||
[closeModal, focusFallbackSurface, requestBrowserFocus]
|
||||
|
|
@ -1535,13 +1543,11 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
|||
|
||||
// Case 1: user pasted a GH issue/PR URL.
|
||||
if (ghLink) {
|
||||
const { slug, number } = ghLink
|
||||
const { number } = ghLink
|
||||
const state = useAppStore.getState()
|
||||
|
||||
// Why: the existing-worktree check only needs the issue/PR number, which
|
||||
// is repo-agnostic on the worktree meta side. We don't currently cache a
|
||||
// repo-slug map, so slug-matching against a specific repo happens
|
||||
// implicitly when we pick a repo for the `gh workItem` lookup below.
|
||||
// is repo-agnostic on the worktree meta side.
|
||||
const matches = allWorktrees.filter(
|
||||
(w) => !w.isArchived && (w.linkedIssue === number || w.linkedPR === number)
|
||||
)
|
||||
|
|
@ -1553,73 +1559,21 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
|||
return
|
||||
}
|
||||
|
||||
// Resolve via gh.workItem: prefer the active repo, else the first eligible.
|
||||
// Why: hand the raw URL to the composer's name field so it runs the same
|
||||
// cross-project detection as Cmd+N — surfacing the "Switch project?"
|
||||
// dialog when the URL targets a different project. Pre-resolving here
|
||||
// against an arbitrary repo silently linked cross-project items to the
|
||||
// wrong project and skipped that prompt. Seed the active repo so the
|
||||
// field compares the URL against the project the user is currently in.
|
||||
const eligibleRepos = state.repos.filter((r) => isGitRepoKind(r))
|
||||
const repoForLookup =
|
||||
(state.activeRepoId && eligibleRepos.find((r) => r.id === state.activeRepoId)) ||
|
||||
eligibleRepos[0]
|
||||
if (!repoForLookup) {
|
||||
openComposer({ prefilledName: trimmed })
|
||||
return
|
||||
}
|
||||
|
||||
prefetchCreateWorkspaceBaseForComposer(repoForLookup.id)
|
||||
const sourceContext = buildTaskSourceContextFromRepo({
|
||||
provider: 'github',
|
||||
projectId: repoForLookup.id,
|
||||
repo: repoForLookup
|
||||
})
|
||||
// Why: awaiting inside the user gesture would leave the palette open
|
||||
// indefinitely on slow networks. Close immediately and populate the
|
||||
// composer once the lookup returns.
|
||||
const lookupToken = createLookupGuard.start()
|
||||
preserveCreateLookupOnCloseRef.current = true
|
||||
recordFeatureInteraction('cmd-j-create-workspace')
|
||||
closeModal()
|
||||
void lookupGitHubWorkItemByOwnerRepoForSource({
|
||||
repoPath: repoForLookup.path,
|
||||
repoId: repoForLookup.id,
|
||||
sourceContext,
|
||||
owner: slug.owner,
|
||||
repo: slug.repo,
|
||||
number,
|
||||
type: ghLink.type
|
||||
})
|
||||
.then((item) => {
|
||||
if (!createLookupGuard.isCurrent(lookupToken)) {
|
||||
return
|
||||
}
|
||||
const data: Record<string, unknown> = { initialRepoId: repoForLookup.id }
|
||||
if (item) {
|
||||
const linkedWorkItem: LinkedWorkItemSummary = {
|
||||
type: item.type,
|
||||
number: item.number,
|
||||
title: item.title,
|
||||
url: item.url
|
||||
}
|
||||
data.linkedWorkItem = linkedWorkItem
|
||||
data.prefilledName =
|
||||
getLinkedWorkItemWorkspaceName(linkedWorkItem)?.seedName ??
|
||||
getLinkedWorkItemSuggestedName({ title: item.title })
|
||||
} else {
|
||||
// Fallback: we couldn't resolve the URL, just seed the name.
|
||||
data.prefilledName = `${slug.owner}-${slug.repo}-${number}`
|
||||
}
|
||||
queueMicrotask(() =>
|
||||
openModal('new-workspace-composer', { ...data, telemetrySource: 'command_palette' })
|
||||
)
|
||||
})
|
||||
.catch(() => {
|
||||
if (!createLookupGuard.isCurrent(lookupToken)) {
|
||||
return
|
||||
}
|
||||
queueMicrotask(() =>
|
||||
openModal('new-workspace-composer', {
|
||||
initialRepoId: repoForLookup.id,
|
||||
telemetrySource: 'command_palette'
|
||||
})
|
||||
)
|
||||
})
|
||||
openComposer(
|
||||
repoForLookup
|
||||
? { prefilledName: trimmed, initialRepoId: repoForLookup.id }
|
||||
: { prefilledName: trimmed }
|
||||
)
|
||||
return
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,60 @@
|
|||
// @vitest-environment happy-dom
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { resolvePaletteFocusRestoreTarget } from './palette-focus-restore-target'
|
||||
|
||||
afterEach(() => {
|
||||
document.body.innerHTML = ''
|
||||
})
|
||||
|
||||
function addTerminal(id: string): HTMLTextAreaElement {
|
||||
const textarea = document.createElement('textarea')
|
||||
textarea.className = 'xterm-helper-textarea'
|
||||
textarea.dataset.terminal = id
|
||||
document.body.appendChild(textarea)
|
||||
return textarea
|
||||
}
|
||||
|
||||
describe('resolvePaletteFocusRestoreTarget', () => {
|
||||
it('returns the exact previously-focused element when it is still connected', () => {
|
||||
// Two terminals mounted (e.g. a background worktree comes first in the DOM);
|
||||
// the user was typing in the second one before opening Cmd+J.
|
||||
addTerminal('background')
|
||||
const active = addTerminal('active')
|
||||
|
||||
expect(resolvePaletteFocusRestoreTarget(active)).toBe(active)
|
||||
})
|
||||
|
||||
it('falls back to the first terminal when the previous element is gone', () => {
|
||||
const first = addTerminal('first')
|
||||
const detached = document.createElement('textarea')
|
||||
detached.className = 'xterm-helper-textarea'
|
||||
// Never appended → not connected (e.g. its pane unmounted while Cmd+J was open).
|
||||
|
||||
expect(detached.isConnected).toBe(false)
|
||||
expect(resolvePaletteFocusRestoreTarget(detached)).toBe(first)
|
||||
})
|
||||
|
||||
it('falls back to the editor textarea when no preferred target and no terminal exist', () => {
|
||||
const editor = document.createElement('div')
|
||||
editor.className = 'monaco-editor'
|
||||
const textarea = document.createElement('textarea')
|
||||
editor.appendChild(textarea)
|
||||
document.body.appendChild(editor)
|
||||
|
||||
expect(resolvePaletteFocusRestoreTarget(null)).toBe(textarea)
|
||||
})
|
||||
|
||||
it('prefers the terminal over the editor when both are present and nothing was captured', () => {
|
||||
const terminal = addTerminal('only')
|
||||
const editor = document.createElement('div')
|
||||
editor.className = 'monaco-editor'
|
||||
editor.appendChild(document.createElement('textarea'))
|
||||
document.body.appendChild(editor)
|
||||
|
||||
expect(resolvePaletteFocusRestoreTarget(null)).toBe(terminal)
|
||||
})
|
||||
|
||||
it('returns null when nothing is focusable', () => {
|
||||
expect(resolvePaletteFocusRestoreTarget(null)).toBeNull()
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,19 @@
|
|||
// Why: when Cmd+J closes it must hand focus back to whatever the user was
|
||||
// doing. Prefer the exact element focused before the palette opened (e.g. the
|
||||
// specific terminal textarea they were typing in); the querySelector fallbacks
|
||||
// grab the first match in the DOM, which can be a background worktree's
|
||||
// mounted-but-hidden terminal rather than the visible one.
|
||||
export function resolvePaletteFocusRestoreTarget(
|
||||
preferredTarget: HTMLElement | null,
|
||||
doc: Document = document
|
||||
): HTMLElement | null {
|
||||
if (preferredTarget && preferredTarget.isConnected) {
|
||||
return preferredTarget
|
||||
}
|
||||
const xterm = doc.querySelector('.xterm-helper-textarea')
|
||||
if (xterm instanceof HTMLElement) {
|
||||
return xterm
|
||||
}
|
||||
const monaco = doc.querySelector('.monaco-editor textarea')
|
||||
return monaco instanceof HTMLElement ? monaco : null
|
||||
}
|
||||
|
|
@ -13,14 +13,21 @@ function sourceBetween(startPattern: string, endPattern: string): string {
|
|||
}
|
||||
|
||||
describe('WorktreeJumpPalette source-context boundaries', () => {
|
||||
it('resolves typed GitHub issue/PR entries through the lookup repo source host', () => {
|
||||
expect(source).toContain('buildTaskSourceContextFromRepo')
|
||||
|
||||
it('defers pasted GitHub URL resolution to the composer so cross-project detection runs', () => {
|
||||
// Why: pasting a cross-project URL must surface the same "Switch project?"
|
||||
// prompt as Cmd+N. The palette hands the raw URL to the composer's name
|
||||
// field instead of pre-resolving it against an arbitrary repo, which
|
||||
// silently linked cross-project items to the wrong project.
|
||||
const githubLinkSection = sourceBetween(
|
||||
'void lookupGitHubWorkItemByOwnerRepoForSource({',
|
||||
'// Case 1: user pasted a GH issue/PR URL.',
|
||||
'// Case 2: user typed a raw issue number.'
|
||||
)
|
||||
expect(githubLinkSection).toContain('sourceContext')
|
||||
expect(githubLinkSection).toContain('prefilledName: trimmed')
|
||||
expect(githubLinkSection).not.toContain('lookupGitHubWorkItemByOwnerRepoForSource')
|
||||
})
|
||||
|
||||
it('resolves typed raw issue/PR numbers through the lookup repo source host', () => {
|
||||
expect(source).toContain('buildTaskSourceContextFromRepo')
|
||||
|
||||
const rawNumberSection = sourceBetween(
|
||||
'void lookupGitHubWorkItemForSource({',
|
||||
|
|
|
|||
Loading…
Reference in New Issue