fix(desktop): let clarify choices and overlays keep their keys from type-to-focus (#69869)

This commit is contained in:
brooklyn! 2026-07-23 00:04:12 -05:00 committed by GitHub
parent 8e01309917
commit e1367b44bc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 90 additions and 3 deletions

View file

@ -66,6 +66,13 @@ export function OverlayView({
'p-[calc(var(--titlebar-height)+0.625rem)]',
'sm:p-[calc(var(--titlebar-height)+0.875rem)]'
)}
// Every OverlayView-based overlay (settings, command-center, agents, cron,
// profiles, star map, …) covers the chat while the composer stays mounted
// beneath it. This marker tells `composerFocusBlockedBySurface` to stand
// the global type-to-focus / soft `/` / Enter down, so keystrokes don't
// leak into the hidden composer (and the overlay's own bare-key shortcuts,
// e.g. star map's Space, keep working).
data-overlay-surface=""
onClick={event => {
if (event.target === event.currentTarget) {
closeOverlay()

View file

@ -296,3 +296,45 @@ describe('ClarifyTool keyboard navigation', () => {
expect(request).not.toHaveBeenCalled()
})
})
describe('ClarifyTool pending marker', () => {
it('marks a live choices card so type-to-focus yields its shortcut keys', () => {
renderLiveClarify()
// The marker is what `composerFocusBlockedBySurface` keys off of, so the
// global type-to-focus listener stands down and A/B/C… + 1-9 + Enter reach
// the card instead of the composer.
expect(document.querySelector('[data-clarify-choices]')).toBeTruthy()
})
it('does not mark a free-text (no-choice) pending card', () => {
$activeSessionId.set('session-1')
$gateway.set({ request: vi.fn().mockResolvedValue({ ok: true }) } as never)
setClarifyRequest({
choices: null,
question: 'Anything else?',
requestId: 'request-1',
sessionId: 'session-1'
})
const args = { question: 'Anything else?' }
renderClarify(
<ClarifyTool
addResult={vi.fn()}
args={args}
argsText={JSON.stringify(args)}
isError={false}
respondToApproval={vi.fn()}
result={undefined}
resume={vi.fn()}
status={{ type: 'running' }}
toolCallId="clarify-free"
toolName="clarify"
type="tool-call"
/>
)
// No shortcuts → nothing to protect → composer type-to-focus stays live.
expect(document.querySelector('[data-clarify-choices]')).toBeNull()
})
})

View file

@ -538,7 +538,11 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) {
}
return (
<ClarifyShell className="grid gap-2 px-2.5 py-2">
// `data-clarify-choices` marks the panel as owning printable/Enter keys
// while its A/B/C… shortcuts are live, so the global type-to-focus listener
// (`composerFocusBlockedBySurface`) stands down and the letters reach this
// card instead of being redirected into the composer.
<ClarifyShell className="grid gap-2 px-2.5 py-2" data-clarify-choices={hasChoices ? '' : undefined}>
<div className="flex items-start gap-2">
<span className="flex-1 whitespace-pre-wrap font-medium leading-(--conversation-line-height)">{question}</span>
<MessageQuestion aria-hidden className="mt-px size-4 shrink-0 text-(--ui-text-tertiary)" />

View file

@ -72,6 +72,22 @@ describe('composerFocusBlockedBySurface', () => {
expect(composerFocusBlockedBySurface()).toBe(true)
})
it('blocks while an overlay covers the chat (composer sits behind it)', () => {
const overlay = document.createElement('div')
overlay.setAttribute('data-overlay-surface', '')
document.body.append(overlay)
expect(composerFocusBlockedBySurface()).toBe(true)
})
it('blocks while a live clarify choices card owns its letter keys', () => {
const card = document.createElement('div')
card.setAttribute('data-clarify-choices', '')
document.body.append(card)
expect(composerFocusBlockedBySurface()).toBe(true)
})
it('blocks when focus is inside a terminal', () => {
const term = document.createElement('div')
term.setAttribute('data-terminal', '')
@ -146,4 +162,16 @@ describe('composerFocusKeysAllowed', () => {
expect(composerFocusKeysAllowed(keydown({ key: 'a', code: 'KeyA', target: document.body }), 'type')).toBe(false)
})
it('yields letter + Enter keys to a live clarify choices card', () => {
const card = document.createElement('div')
card.setAttribute('data-clarify-choices', '')
document.body.append(card)
// The clarify card's own A/B/C… + Enter shortcuts must win over type-to-focus.
expect(composerFocusKeysAllowed(keydown({ key: 'a', code: 'KeyA', target: document.body }), 'type')).toBe(false)
expect(composerFocusKeysAllowed(keydown({ key: 'Enter', code: 'Enter', target: document.body }), 'enter')).toBe(
false
)
})
})

View file

@ -38,7 +38,7 @@ const ENTER_ACTIVATES = [
].join(',')
const BLOCKING_SURFACE =
'[role="dialog"],[role="alertdialog"],[role="menu"],[role="listbox"],[data-radix-popper-content-wrapper]'
'[role="dialog"],[role="alertdialog"],[role="menu"],[role="listbox"],[data-radix-popper-content-wrapper],[data-overlay-surface],[data-clarify-choices]'
/** True when the focused control would normally handle Enter itself. */
export function isActivateOnEnterTarget(target: EventTarget | null): boolean {
@ -47,7 +47,13 @@ export function isActivateOnEnterTarget(target: EventTarget | null): boolean {
return Boolean(el && el !== document.body && el !== document.documentElement && el.closest(ENTER_ACTIVATES))
}
/** Dialogs, menus, terminal, full pages, session switcher — they keep their keys. */
/**
* Dialogs, menus, terminal, full pages, session switcher, any open overlay
* (settings / command-center / star map / ), and a live clarify choices card
* they keep their keys, so type-to-focus / soft `/` / Enter stand down rather
* than stealing keystrokes those surfaces own (or leaking them into the composer
* mounted behind an overlay).
*/
export function composerFocusBlockedBySurface(): boolean {
return (
switcherActive() ||