Skip to content

bug(ui): Enter on a focused Cancel applies the change in Agent View Mode, Reorder Tabs and Pin Agents #867

Closed

Description

@Juliusolsson05

Expected

In every dialog, a focused footer button owns its own Enter: Tab to Cancel + Enter cancels, Tab to Done + Enter does what Done does. This is the rule src/renderer/src/components/ui/dialog-actions.tsx documents, now exported as focusedControlOwnsEnter (#860).

Actual

Three list-style dialogs handle Enter on DialogContent themselves, call preventDefault (which cancels the focused button's native click), and run the list's own action instead — the same bug #862 fixed in Switch Provider:

Dialog Where What Enter on a focused footer button does
Agent View Mode src/renderer/src/features/workspace/ui/AgentViewModePickerModal.tsx (Enter branch ~:142-145; Cancel in a DialogFooter) ArrowDown to "Terminal", Tab to Cancel, Enter → the Terminal override is applied and the dialog closes.
Reorder Tabs src/renderer/src/features/workspace/ui/ReorderTabsModal.tsx (Enter branch ~:148-158; footer is a plain <div>, not DialogFooter) Pick a tab, move it, Tab to Cancel, Enter → confirm() commits the reorder being abandoned. Tab to Done with nothing picked, Enter → enters move mode instead of confirming.
Pin Agents src/renderer/src/features/dispatch-pin/usePinAgentsKeybinds.ts (~:100-104, wired onto DialogContent in PinAgentsModal.tsx) Uncheck rows, Tab to Cancel, Enter → setPinnedSessionIds(draft) saves the unpinning; with nothing selected it commits an empty list that DialogActions' confirmDisabled exists to prevent.

Reproduction

For each dialog: open it, change the selection with the arrow keys, press Tab until Cancel is focused, press Enter.

Impact

Keyboard users who cancel get the change they were abandoning applied (view mode, tab order, pins).

Suggested fix

Use the pattern #860 established for New Agent In and Switch Provider:

  • guard the Enter branch with focusedControlOwnsEnter(event.target) (exported from components/ui/dialog-actions.tsx) — this also covers Reorder Tabs' non-DialogFooter footer, since the check is on the focused control, not the footer slot;
  • keep list rows out of the tab order (tabIndex={-1}) where rows are buttons, so a Tab-focused row cannot diverge from the arrow-driven highlight (Space would click it regardless of any Enter handling);
  • one regression test per dialog (focused Cancel + Enter must not act; fireEvent.keyDown returning true proves the native click was not prevented).

Notes

Found by the second review round of #860 (an audit of all 36 DialogContent consumers for list-level Enter handlers). The others are safe: their Enter is scoped to an input/scroller that excludes the footer, they have no footer buttons, or they trap Tab. Related: #862.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions