Skip to content

fix(keyboard): repair main's tests and the Claude-review findings on the keyboard-first pass - #1323

Merged
Juliusolsson05 merged 14 commits into
mainfrom
fix/keyboard-first-followups
Sep 27, 2026
Merged

fix(keyboard): repair main's tests and the Claude-review findings on the keyboard-first pass#1323
Juliusolsson05 merged 14 commits into
mainfrom
fix/keyboard-first-followups

Conversation

@Juliusolsson05

@Juliusolsson05 Juliusolsson05 commented Sep 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Problem

#1221 (keyboard-first UI) merged with two open problems:

  1. Main fails 9 renderer tests. Tests that landed on main alongside feat(ui): make every surface keyboard-operable with compact key hints and consistent chrome #1221 still query the accessible names feat(ui): make every surface keyboard-operable with compact key hints and consistent chrome #1221 renamed ("Add", "Press keys… (Esc)", "everywhere").
  2. A Claude review round (3 reviewers, reports in temp/review3-1221-*) found regressions and gaps in the merged code. The worst one: a pending approval in one pane disables Enter-to-send and Submit Active Composer in every pane.

This PR

Plan: docs/plans/2026-09-25-keyboard-first-followups.md.

  • Main's 9 stale test queries now use the current names. The code was correct. Main fixed the same queries in parallel, and the merge takes main's version.
  • Enter router: an inline approval list no longer blocks Enter app-wide, and Enter aimed inside a pane prompt no longer sends another pane's draft (A-F1, A-F3).
  • Pane-prompt inerting: no deadlock when two prompts mount together, late-mounted pane controls go inert too, and the pane toast stays announced (B-F1, B-F2, A-F2).
  • Goal-loop overlay: Tab recovers focus after Raise Cap at the ceiling (A-F4).
  • A visible focus ring on accent-filled controls with the default theme (C-F1).
  • Owner-checklist corrections in the feat(ui): make every surface keyboard-operable with compact key hints and consistent chrome #1221 plan (C-F2).
  • Sweep misses and stale WHY comments (C-F3…F8, B-F4).
  • Targeted tests for the review's surviving mutations.

Dispositions (Claude review round on #1221)

Finding Verdict Disposition
main: 9 renderer tests fail (stale accessible names) valid fixed in e092232c; main fixed the same queries in parallel, and the merge takes main's version
A-F1: an inline approval listbox disables Enter-to-send app-wide valid, regression fixed in 4915e869
A-F3: Enter inside a pane prompt sends another pane's hovered draft valid fixed in 4915e869
B-F1: two prompts mounting in one commit inert each other (deadlock) valid fixed in 73b7fae2 (one controller per pane)
A-F2 / B-F3: pane children mounted after the prompt stay live valid fixed in 73b7fae2 (MutationObserver)
B-F2: the pane toast is inert under a prompt, so it is silent valid fixed in 73b7fae2 (feedback layer exempt)
A-F4: Raise Cap at the ceiling strands focus in the latched overlay valid fixed in efdceaf1
C-F1: the focus ring is invisible on accent-filled controls valid fixed in b9cb9f5d
C-F2: 8 owner-checklist lines contradict the code valid fixed in 90898a6c (code or checklist, whichever was wrong)
C-F4: casing, "Couldn't" and "directory" survivors valid fixed in 90898a6c. Ruling: full-sentence titles (questions, "Grok is asking") are sentence case, and noun-phrase titles are Title Case. "Can't resume …" stays, because it is a refusal, not a failure
C-F3: 11 raw-alpha danger/warning boxes valid fixed in e1333b81
C-F5 / F6 / F8, B-F4: row recipe, bespoke widths, stale WHYs valid fixed in c2c9e97a
C-F7: the Usage chip is font-normal beside font-medium overstated declined: its WHY documents the mixed weights; only "N/A" changed
C-F7: Grok/OpenCode footers without chips overstated declined: already adjudicated in #1221 round 1 (C5). Those shells wire no Escape or dialog Enter, so chips would lie
A survivors #4, #33 valid pinned in 14af9673
A survivors #13, #14, #20, #22, #37, #41, #44–45, #50, #51, #53; B partial battery valid test gaps follow-up: each needs a TileLeaf / useKeybinds / useListNavigation harness beyond this fix's scope
B: firstRun timeout under full-suite load suspicion passes alone, and #1221 only recased that test; not treated as fixed

Verification

Focused suites pass, and each new behaviour test fails on main (checked by swapping the files back) with its guard mutation caught. The full renderer run is below.

Refs #1221, Refs #713

🤖 Generated with Claude Code

Juliusolsson05 and others added 13 commits September 25, 2026 10:52
@Juliusolsson05 @claude
docs(keyboard): plan the follow-ups to the keyboard-first pass
2280aed 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05 @claude
test(settings): query the shortcut recorder and conversation scope by…
e092232 
… their current names

These tests landed on main alongside #1221 and still queried the accessible
names #1221 renamed ("Add", "Press keys… (Esc)", "everywhere"), so 9
tests failed on main. The code is correct and only the queries were stale.
The absence checks go through one helper, so they cannot drift to a missing
name and pass vacuously.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05 @claude
fix(workspace): stop an inline approval list from disabling Enter-to-…
4915e86 
…send in every pane

The Enter router treated any role=listbox in the document as an open popup.
Since #1221 the inline condition option list is a listbox, and it lives in
long-lived approval and resume strips. So one agent waiting on an approval
disabled Enter-to-send and Submit Active Composer app-wide. A listbox now owns
Enter only while the user is in it (focus inside, or the focused combobox
controls it).

The router also leaves Enter aimed inside a pane prompt to that prompt.
Before, Enter there submitted another pane's hovered draft.

Claude review of #1221, reviewer A, F1 and F3.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05 @claude
fix(ui): inert a prompted pane from one controller, so prompts cannot…
73b7fae 
… deadlock or leak

Each pane dialog marked the pane's children inert once on mount, skipping only
its own nodes. That broke three ways:
- two prompts mounting in one commit (any TileLeaf remount) inerted each
  other, so neither could be answered;
- pane controls mounted after the prompt opened stayed live under the scrim;
- the pane toast was inerted, which silenced its live region.

One controller per pane now recomputes from DOM order on every registration
and every child-list change. Only the newest dialog, its scrim and the
feedback layer stay live, and the controller removes only the inert it set.

Claude review of #1221: reviewer B F1/F2, reviewer A F2.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05 @claude
fix(goal-loop): let Tab bring a stranded keyboard user back into the …
efdceaf 
…latched overlay

Raise Cap at the ceiling hides its own button without changing the phase, so
focus fell to <body>. The latched gate then swallowed Tab, Enter and Space, and
only Escape or the chord got out. From <body>, Tab now focuses the active
pane's overlay. A Tab from the dimmed composer is still swallowed.

Claude review of #1221, reviewer A F4.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05 @claude
fix(ui): make keyboard focus visible on accent-filled controls
b9cb9f5 
Every built-in theme's focus ring is the accent, so a flush accent ring on an
accent fill showed nothing. #1221 puts focus on exactly these controls: filled
confirm buttons, the checked choice card, the selected segment, the toast and
the settings-bar toggles. Filled controls now offset the ring from the fill,
as Switch already did. The clipped segmented control draws an inset ring in
the fill's own foreground.

Claude review of #1221, reviewer C F1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05 @claude
fix(ui): make titles, labels and the owner checklist agree with the c…
90898a6 
…asing ruling

Title casing:
- Question titles go back to sentence case, as the ruling says ("Trust this
  folder?", "Delete from disk?").
- Noun-phrase titles become Title Case ("Unsaved Changes", "File Deleted on
  Disk", "New Custom Skill").
- Full-sentence titles stay sentence case ("Grok is asking").

Labels and messages:
- "Check Updates".
- The Conversations filter chips show provider names at the segmented
  control's size.
- "Couldn't …" becomes "Could not …", and the last UI "directory" becomes
  "folder".

The #1221 owner checklist is corrected where it contradicted the code:
- Spotlight and choice arrows now select as they move (k9).
- Condition prompt labels.
- The Working Folder header.
- Text fields show the ring on any focus.
- The chart-tick exception.

Claude review of #1221, reviewer C F2 and F4.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05 @claude
fix(ui): draw the remaining error and warning boxes with the soft tokens
e1333b8 
Eleven boxes still mixed raw alpha (bg-danger/10, border-danger/40, and the
warning equivalents). A Custom Appearance theme that sets danger-soft or
warning-soft therefore restyled the Alert boxes and left these on the old
tint, so the same error looked different in two places. Each box keeps its
layout and only moves to the -soft and -border tokens.

Claude review of #1221, reviewer C F3.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05 @claude
fix(ui): converge the last sweep misses and correct stale WHY comments
c2c9e97 
- The AI Workspace active file uses the one selected-row look, the same as the
  Explorer.
- The Conventions and Custom Skill editors use the lg width preset.
- The Usage chip reads "Usage N/A".
- The DialogActions, dialog-size and condition-refusal WHY comments now say
  what the code does. The Usage chip's font-normal stays, because its own WHY
  explains the mixed weights.

Claude review of #1221, reviewer C F5/F6/F7/F8 and reviewer B F4.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05 @claude
test(keyboard): pin two #713 promises the Claude review found unguarded
14af967 
- A pane prompt that resolves while the user works in another pane leaves
  their focus alone.
- Enter and Space from the dimmed composer do not pass the latched goal-loop
  gate.

Both mutations survived the suite before these tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05 @claude
docs(keyboard): record the follow-up dispositions
7843af1 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05
Merge remote-tracking branch 'origin/main' into fix/keyboard-first-fo…
bc81b41 
…llowups

# Conflicts:
#	src/renderer/src/features/conversations/ui/ConversationsPicker.renderer.test.tsx
#	src/renderer/src/features/settings/ui/CommandKeybindingsRow.capture.renderer.test.tsx
@Juliusolsson05 @claude
docs(keyboard): note main's parallel fix of the stale test queries
f49cfac 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05
Juliusolsson05 marked this pull request as ready for review September 25, 2026 18:10
@Juliusolsson05
Juliusolsson05 merged commit 95aff39 into main Sep 27, 2026
2 checks passed
@Juliusolsson05
Juliusolsson05 deleted the fix/keyboard-first-followups branch September 27, 2026 08:56
@Juliusolsson05 Juliusolsson05 mentioned this pull request Sep 27, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Juliusolsson05