Przeglądaj źródła

fix(client): answer the review bot's findings on the keyboard hand-offs

- The model seat decides before consuming: a focused control that is not a row
  (a retry button) keeps the browser's traversal, and Tab enters the list only
  from the trigger.
- A launcher-opened menu is a fresh intent, so the dismissal that closed the
  same token earlier no longer silences it.
- Comments and JSDoc now match the implementation: the near-end entry resumes
  from the walk's last index, Shift+Tab leaves whenever the menu is open, and
  the anchor hand-back names the control that opened the menu.
- The e2e focus assertions all go through `expect.poll`.
- Regression coverage for a rowless pane (the trigger keeps the keyboard) and
  for a focused retry keeping its Tab, in the seat and the popup shell.
- The PR records the two public-surface changes (`SessionInput.focus()` in,
  `bindComposerFocus` out) and the row-fill focus decision.
liukx0205 3 tygodni temu
rodzic
commit
b263581e92

+ 2 - 2
.agents/notes/implemented/feature/2026-09-14-composer-selection-keyboard.i18n.yaml

@@ -2,5 +2,5 @@
 # side as of the last confirmed-consistent state. Both languages carry equal authority;
 # after editing either side, bring the other along and re-record with:
 #   pnpm run verify-translation-pairing --write .agents/notes/implemented/feature/2026-09-14-composer-selection-keyboard.md
-2026-09-14-composer-selection-keyboard.md: 9d7f8da91bc69b333ab690a32ea74e219961b6e9
-2026-09-14-composer-selection-keyboard.zh.md: c86e42857f66ff5efb0c6a57dce46e3df79cda0c
+2026-09-14-composer-selection-keyboard.md: fb353a7072e7c2245aeeb22af9018edd9a6009f0
+2026-09-14-composer-selection-keyboard.zh.md: f51e5ca5096785ae6d059e3c8497292483490437

+ 1 - 1
.agents/notes/implemented/feature/2026-09-14-composer-selection-keyboard.md

@@ -20,7 +20,7 @@ The model seat's dropdown (`ModelSelect`, whose Effort row opens the reasoning-l
 
 `ModelSelect` keeps `↑`/`↓` as its focus walk over the rows of the shown pane (wrapping; a step taken while the trigger still holds focus enters at the near end). Tab settles: it activates the row the keyboard is on, and with focus still on the trigger it enters the menu at the row of the value in use instead. Escape and Shift+Tab leave a drilled pane first and otherwise close back to the trigger. A pane switch replaces the row that had focus, so each switch names where the keyboard lands: drilling focuses the row of the value in use (its checked row, or the first row when none is marked), and returning to the root pane focuses the cell that opened the pane left. Without that handoff the focus the unmounted row left on the page body sits outside the card's subtree, where its key handling never sees a keystroke.
 
-`Menu`, the shared anchored dropdown behind the composer's permission seat, the preset chip, file cards, and the settings rows, carries the same pairing: while its list is open, Tab settles the focused row — from the anchor's button, Tab enters the list instead — and Escape or Shift+Tab close it and return focus to the anchor's first button. Only a keyboard already on the anchor or inside the list is intercepted, so Tab presses elsewhere on the page keep the browser's traversal even while a menu is open. Selecting a row hands the keyboard back to the anchor unless the owner moved it itself — a card that focuses its own preview keeps it. The arrows, Home, and End walk the list whether or not `autoFocus` is set — that option now decides only whether opening focuses the first row — the row the keyboard is on carries the same fill a hovered row gets, and closing on Escape or Shift+Tab returns focus to the anchor whenever the menu held it.
+`Menu`, the shared anchored dropdown behind the composer's permission seat, the preset chip, file cards, and the settings rows, carries the same pairing: while its list is open, Tab settles the focused row — from the anchor's button, Tab enters the list instead — and Escape or Shift+Tab close it and return focus to the anchor's first button. Only a keyboard already on the anchor or inside the list is intercepted, so Tab presses elsewhere on the page keep the browser's traversal even while a menu is open. The row's focus fill is the row's own indication: the browser's default ring would double it, and the fill is the same one the row shows under the pointer, so the visual language stays one per state. Selecting a row hands the keyboard back to the anchor unless the owner moved it itself — a card that focuses its own preview keeps it. The arrows, Home, and End walk the list whether or not `autoFocus` is set — that option now decides only whether opening focuses the first row — the row the keyboard is on carries the same fill a hovered row gets, and closing on Escape or Shift+Tab returns focus to the anchor whenever the menu held it.
 
 A dismissed menu stays dismissed: `InputTriggerController` records the identity of the hit the user closed — Escape or Shift+Tab, a pointer dismissal, or a settling pick — and a re-track of that same token with the same query leaves the menu closed, so restoring the caret after a dismissal (or a settled command closing the surface it opened) cannot revive it. A new query or another token re-arms it.
 

+ 1 - 1
.agents/notes/implemented/feature/2026-09-14-composer-selection-keyboard.zh.md

@@ -20,7 +20,7 @@ composer 内有两个选择界面在打开期间持有焦点,两者都把 Tab
 
 `ModelSelect` 的 `↑`/`↓` 仍是在所显示面板行间移动焦点(循环;焦点仍在触发器上时从近端进入)。Tab 执行接受:它激活键盘所在的那一行;焦点仍在触发器上时,改为进入菜单并把焦点放在正在使用的那一行。Escape 与 Shift+Tab 都先退出已下钻的面板,否则关闭并回到触发器。面板切换会替换持有焦点的那一行,因此每次切换都指明键盘的落点:下钻落在正在使用的值那一行(其选中行;没有标记行时为首行),返回根面板则落在打开该面板的那个格子上。没有这一步,已卸载的行留在页面 body 上的焦点就位于卡片自身子树之外,挂在那里的按键处理永远收不到键盘事件。
 
-`Menu`——权限档位、预设芯片、文件卡与设置行背后的共享锚定下拉——沿用同一组配对:列表打开期间,Tab 选定聚焦行(键盘在锚点按钮上时,Tab 改为进入列表),Escape 或 Shift+Tab 关闭并把焦点还给锚点的第一个按钮。只拦截已经位于锚点或列表内的键盘,因此页面其它位置按下的 Tab 即使在菜单打开时也仍归浏览器。选定一行会把键盘还给锚点——除非拥有者自己移动了焦点(例如文件卡把自己的预览按钮设为焦点)。`↑`/`↓` 与 Home/End 无论是否设置 `autoFocus` 都在列表中走位——该选项如今只决定打开时是否聚焦首行——键盘所在的那一行使用与悬停行相同的填充(否则键盘走位看不出当前行),而 Escape 或 Shift+Tab 关闭时,只要键盘原本在菜单里就把焦点归还锚点。
+`Menu`——权限档位、预设芯片、文件卡与设置行背后的共享锚定下拉——沿用同一组配对:列表打开期间,Tab 选定聚焦行(键盘在锚点按钮上时,Tab 改为进入列表),Escape 或 Shift+Tab 关闭并把焦点还给锚点的第一个按钮。只拦截已经位于锚点或列表内的键盘,因此页面其它位置按下的 Tab 即使在菜单打开时也仍归浏览器。行的焦点填充就是行自己的焦点提示:浏览器默认轮廓会与之叠加,而这一填充与指针悬停时用的是同一个,视觉语言因此一态一义。选定一行会把键盘还给锚点——除非拥有者自己移动了焦点(例如文件卡把自己的预览按钮设为焦点)。`↑`/`↓` 与 Home/End 无论是否设置 `autoFocus` 都在列表中走位——该选项如今只决定打开时是否聚焦首行——键盘所在的那一行使用与悬停行相同的填充(否则键盘走位看不出当前行),而 Escape 或 Shift+Tab 关闭时,只要键盘原本在菜单里就把焦点归还锚点。
 
 被关闭的菜单保持关闭:`InputTriggerController` 记下用户关闭的那个命中(Escape/Shift+Tab、指针关闭或一次落定选定)的身份,同一 token 以同一查询重新 track 时菜单保持关闭,因此关闭后恢复光标、或已选定命令关闭它打开的界面,都无法把它召回来;新的查询或另一个 token 才会重新武装。
 

+ 8 - 2
apps/web/tests/declared-reasoning.e2e.ts

@@ -84,9 +84,15 @@ describe.skipIf(MODE === 'record')('web e2e: declared reasoning efforts reach th
       { timeout: 10_000 },
     ).toBe(true)
     await page.keyboard.press('ArrowDown')
-    expect(await levels.nth(1).evaluate(element => element === document.activeElement)).toBe(true)
+    await expect.poll(
+      () => levels.nth(1).evaluate(element => element === document.activeElement),
+      { timeout: 10_000 },
+    ).toBe(true)
     await page.keyboard.press('ArrowDown')
-    expect(await levels.nth(2).evaluate(element => element === document.activeElement)).toBe(true)
+    await expect.poll(
+      () => levels.nth(2).evaluate(element => element === document.activeElement),
+      { timeout: 10_000 },
+    ).toBe(true)
 
     // Settling with Tab is the same gesture that saves the default selection, so
     // the effort lands in the Agent default Settings section beside provider/model.

+ 12 - 0
packages/client/ui-commands/tests/popup-view.client.spec.tsx

@@ -157,6 +157,18 @@ describe('PopupSelectView', () => {
     expect(screen.getByText('正在加载选项…')).toBeTruthy()
   })
 
+  it('Tab stays the browser\'s on a failed load, so the retry stays reachable', async () => {
+    const popup = new PopupSelectController<string>({ consume: () => true, focusComposer: () => {} })
+    render(<PopupSelectView popup={popup} t={t} />)
+    await act(async () => {
+      popup.open('theme', spec({ options: () => Promise.reject(new Error('directory down')) }), 'ctx-A', SEGMENT)
+      await Promise.resolve()
+    })
+    const search = screen.getByRole('textbox', { name: '筛选选项' })
+    expect(fireEvent.keyDown(search, { key: 'Tab' })).toBe(true)
+    expect(screen.getByRole('button', { name: '重试' })).toBeTruthy()
+  })
+
   it('scrolls the highlighted row into view when the highlight moves', async () => {
     const { search } = await mountOpen()
     scrollIntoView.mockClear()

+ 4 - 3
packages/client/ui-conversation/src/client/input/editor/keymap.ts

@@ -100,9 +100,10 @@ export function registerComposerKeymap(editor: LexicalEditor, handlers: Composer
     editor.registerUpdateListener(syncComposition),
     editor.registerCommand(KEY_ARROW_UP_COMMAND, arrow('up'), COMMAND_PRIORITY_CRITICAL),
     editor.registerCommand(KEY_ARROW_DOWN_COMMAND, arrow('down'), COMMAND_PRIORITY_CRITICAL),
-    // Tab settles the highlighted completion; Shift+Tab leaves the menu like
-    // Escape, so the two Tab gestures never disagree about consuming the draft.
-    // Without a highlight both pass, keeping native focus traversal.
+    // Tab settles the highlighted completion and passes without one, keeping
+    // native focus traversal; Shift+Tab leaves the menu like Escape whenever it
+    // is open, highlight or not, so the two Tab gestures never disagree about
+    // consuming the draft.
     editor.registerCommand(
       KEY_TAB_COMMAND,
       event => arrow(event.shiftKey ? 'tabBack' : 'tab')(event),

+ 3 - 0
packages/client/ui-input-trigger/src/client/controller.ts

@@ -139,6 +139,9 @@ export class InputTriggerController {
       return
     }
     const hit: TriggerHit = { ...raw, span: { ...raw.span, draftRev } }
+    // A launcher-opened menu is a fresh intent: the dismissal that closed the
+    // same token earlier must not silence it.
+    if (launched) this.dismissed = null
     if (this.dismissed !== null) {
       if (!dismissedHit(this.dismissed, hit)) this.dismissed = null
       else {

+ 7 - 4
packages/client/ui-model-selection/src/client/ModelSelect.tsx

@@ -238,24 +238,27 @@ export function ModelSelect(
     // keys mean what they mean in the composer. Both are consumed: the card
     // keeps the browser's focus traversal out while it is open.
     if (event.key === 'Tab') {
-      event.preventDefault()
       if (event.shiftKey) {
+        event.preventDefault()
         if (pane !== 'root') back(pane)
         else close(true)
         return
       }
       // Settling activates the row the keyboard is on; with focus still on the
       // trigger, Tab enters the menu at the value in use instead. Any other
-      // control inside the card (a retry button) keeps the browser's traversal.
+      // control inside the card (a retry button) keeps the browser's traversal,
+      // so the keystroke stays unconsumed there.
       const focused = document.activeElement
       const rows = itemRefs.current.filter((item): item is HTMLButtonElement => item !== null)
       if (focused instanceof HTMLButtonElement && rows.includes(focused)) {
+        event.preventDefault()
         focused.click()
         return
       }
       if (focused !== triggerRef.current) return
-      const checked = menuRef.current?.querySelector<HTMLElement>('[role="menuitemradio"][aria-checked="true"]')
-      ;(checked ?? itemRefs.current.find(item => item !== null))?.focus()
+      event.preventDefault()
+      const checked = menuRef.current?.querySelector<HTMLElement>('[role="menuitemradio"][aria-checked="true"]:not([disabled])')
+      ;(checked ?? rows.find(item => !item.disabled))?.focus()
       return
     }
     if (event.key === 'ArrowDown' || event.key === 'ArrowUp') {

+ 33 - 0
packages/client/ui-model-selection/tests/model-select.client.spec.tsx

@@ -358,6 +358,39 @@ describe('ModelSelect keyboard walk', () => {
     expect(document.activeElement).toBe(rows[2])
   })
 
+  it('keeps the card navigable when a pane has no rows, and leaves a retry its Tab', () => {
+    const directory = createSnapshotStore<ModelDirectoryState>(state({
+      groups: [], failures: [], status: 'error', error: 'catalog down',
+    }))
+    render(<ModelSelect
+      locked={false}
+      available
+      directory={directory}
+      load={vi.fn()}
+      select={vi.fn().mockResolvedValue(true)}
+      t={t}
+    />)
+    const trigger = screen.getByRole('button', { name: /选择模型/ })
+    // A real click focuses the trigger first; jsdom's does not.
+    trigger.focus()
+    fireEvent.click(trigger)
+    fireEvent.click(screen.getByRole('menuitem', { name: /^模型/ }))
+    // No rows to hand the keyboard to: the trigger keeps it, so the card's
+    // keys still reach the menu.
+    expect(document.activeElement).toBe(trigger)
+
+    const retry = screen.getByRole('button', { name: '重试' })
+    retry.focus()
+    // A control that is not a row keeps the browser's traversal.
+    expect(fireEvent.keyDown(retry, { key: 'Tab' })).toBe(true)
+    // Escape still backs out of the pane and then closes the card.
+    fireEvent.keyDown(retry, { key: 'Escape' })
+    // Back on the root pane, whose only cell remains (no model means no effort row).
+    const cell = screen.getAllByRole('menuitem')[0]!
+    fireEvent.keyDown(cell, { key: 'Escape' })
+    expect(screen.queryByRole('menu')).toBeNull()
+  })
+
   it('drills into the model list on the selected model', () => {
     mountOpen()
     fireEvent.click(screen.getByRole('menuitem', { name: /^模型/ }))

+ 2 - 1
packages/client/ui-primitives/src/Menu.tsx

@@ -287,7 +287,8 @@ export function Menu({ open, anchor, items, selectedId, selectedIds, onSelect, o
       }
       // Arrows walk the list whether or not the menu focused its first item on
       // open, so `autoFocus` chooses only that entry behavior. A keyboard still
-      // on the anchor enters at the end the step comes from. The walk resumes
+      // on the anchor enters at the end the step comes from — unless it already
+      // walked, in which case the walk resumes where it left off. The walk resumes
       // from where it last put focus, not from `document.activeElement`: a row
       // that refused focus (a hidden portal frame, a detached node) would
       // otherwise re-enter at the near end on every press and the walk would