Repository navigation
Conversation
The Agent version tabs are a tablist: role=tab buttons with aria-selected and aria-controls, a tabpanel, one Tab stop on the selected tab, Left/Right/Home/End to move focus, Enter or Space to open. An edge with more tabs past it fades on a narrow screen. Tests address the tabs by their tab role. UX plan C7.
Left/Right/Home/End move focus without opening a tab, Tab leaves the list and Shift+Tab returns to the selected tab, Enter opens the focused tab, aria-selected follows tab= (Back included), and the phone strip fades the edge with more tabs from its first paint.
Tab names held in variables, the logs strip lookup (closest nav) and two aria-current checks now use the tab role and aria-selected.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
The sign-in integration test finds the Configuration tab by its tab role. The focus-out reset checks where focus went once it moved, so a window blur keeps the arrowed-to stop; a tab switch keeps the stop on a tab that has focus; a draft navigation block moves the stop off tabs it disables; the fade refreshes after a switch and a focused tab scrolls clear of the faded edge. The test checks the panel label and the stop after Back.
|
Codex review: needs changes before merge. What this changesThis PR adds accessible tab semantics, keyboard navigation, and overflow fades to Agent configuration tabs. Example: With Plugins selected, press Right and then Enter.
Review scores
ProductKind: Feature · Worth it: Yes · Fix scope: Complete Merge readiness⛔ Needs changes before merge - 3 items remain Keep open: this maintainer-directed accessibility improvement is useful, but narrow-screen navigation can leave keyboard focus offscreen. Priority: P2 Before merge
Findings
Tests
Agent review detailsHow this fits togetherThe console's Agent detail renderer translates tab clicks, keyboard input, and URL history into selected content while preserving the surrounding page and draft guards. flowchart TD
A[Keyboard or pointer input] --> B[Agent tab strip]
B --> C[Existing navigation guard]
C --> D[URL tab selection]
E[Browser history] --> D
D --> F[Tab content renderer]
F --> B
Technical reviewBest possible solution: Keep the existing tab owner, make horizontal reveal honor keyboard focus, and complete the required Storybook and flow updates. Do we have a high-confidence way to reproduce the issue? At 390 px, open Configuration, activate Workspace files, and navigate Back while focus remains on Workspace files; renderTab keeps that focus but scrolls Configuration into view, hiding the focused control. Is this the best way to solve the issue? Extending the existing builder is appropriate; its horizontal reveal logic must account for the newly separate focused and selected tabs. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 6b0a79cc6ce3. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceAcceptance criteria:
What I checked:
Likely related people:
Review metrics
LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment Reviewed October 18, 2026, 6:35 PM ET / 22:35 UTC. |
A tab focused with the arrows stays in view when Back moves the selection elsewhere or the strip resizes. The draft Storybook scenario gains the keyboard steps, and the console flow states the tablist behaviour (word-neutral). The test covers Back and resize at 390/360.
…agraph #906 rewrites the tab-reveal paragraph, so editing it there would add a conflict hunk. The focus section names the single Tab stop and arrow keys instead (word count 2495 to 2498).
Back or Forward that selects another tab while focus is in the strip moves focus to the selected tab, so the revealed tab and the focused one agree and the existing phone-width history test keeps the selected tab visible. A tab focused with the arrows still stays in view through a resize.
A workflow for the tablist (one Tab stop, arrows, Enter, Back, phone fade), and the focus-ring walkthrough now arrows through the tabs instead of tabbing through each one.
Problem
UX plan C7. The Agent version tabs (
.agent-tabs) were anavof plain buttons: notablist/tab/tabpanelsemantics, each tab its own Tab stop (six or seven stops before the panel), no arrow keys, and on a phone the strip clipped with no sign that it scrolls (audit 2.9; design language "Tabs").Change
agents/detail.mjs, in the tab-strip builder and the two places that already update the strip:role="tablist"(same accessible name, "Agent configuration views"). Each tab is arole="tab"button witharia-selected, an id andaria-controlspointing at the tab content, which is nowrole="tabpanel"labelled by the selected tab. Ids carry a per-rendercrypto.randomUUID()(precedent:slack-approvers.mjs).tabindex=0, the others-1. Left/Right (wrapping) and Home/End move focus between enabled tabs without opening one; Enter or Space opens the focused tab, which still goes through the same click handler (draft guard, URLtab=, in-place switch). Once focus has left the list, the Tab stop goes back to the selected tab, so Shift+Tab comes back to it (a window blur keeps the arrowed-to stop). When Back/Forward selects another tab while focus is in the strip, focus follows the selection. A draft navigation block that disables tabs moves the stop to the selected one.aria-selected, the Tab stop and the panel's label follow the URL'stab=.aria-current="page"is no longer set on tabs;aria-selectedreplaces it.data-more-before/data-more-after, updated on scroll, after a tab switch and by the strip's existing ResizeObserver); a tab focused with the arrows scrolls clear of the fade (scroll-padding-inline). The strip's reveal keeps a focused tab in view (else the selected one), so a resize never scrolls keyboard focus off-screen.console.css: the selected style keys on[aria-selected="true"](same look; 1440 Storybook frame is pixel-identical); the edge fade is a mask built from--space-8and an opaque token (a mask reads only alpha), with no new literals.Docs and Storybook: the draft scenario (
scenarios.mjs, Pages/Agent detail › First version) gains the keyboard steps. A newscripts/console-storybook/agent-tabs-keyboard-workflow.mdwalks the tablist (one Tab stop, arrows, Enter, Back, phone fade).focus-and-disabled-workflow.mdstep 2 now arrows through the tabs instead of tabbing through each one.docs/flows/platform-console.mdsection 7 notes the single Tab stop and arrow keys. That is word-neutral: 2495 to 2498 on the real counter, cap 2500. It goes in section 7, not the tab-reveal paragraph, because #906 rewrites that paragraph and editing it would add a conflict hunk.Tests: 18 browser files and
tests/integration/postgres-github-sign-in.test.mjsaddressed the tabs asgetByRole("button", { name: "Logs" })etc. or viaaria-current; they now usegetByRole("tab", …)/aria-selected. Mechanical, no behaviour change.Verification
tests/browser/console-agent-detail-2.test.mjs(already in a browser suite), "Agent tabs are a tablist with arrow keys, one Tab stop and the URL's selection":aria-selectedand the single Tab stop matchtab=plugins, and every tab controls the panel;tab=channels, and selection, Tab stop and panel label follow;aria-selected, the panel label, the Tab stop and focus to Plugins;tests/browser/console*.test.mjsfiles passed: 605/605 before the main merge, and 611/612 on the review-fix product code, then 615/615, 616/616, 617/617 and 629/629 after merging main (refactor(console): one focus ring and one disabled style #2876; feat(console): responsive table rows for the Agents list #2877 and fix(console): Create Agent harness outcome copy and workspace missing-file copy #2875; feat(console): dialog and drawer pattern #2879; fix(console): offer Deploy again only after the Configuration read succeeds #2883-test(console): cover overlapping legacy token aliases #2885). The one failure was the new test's own Back expectation, which then changed to match the kept focus stop and passes 20/20. The PostgreSQL sign-in integration test change is a one-line role swap; it was not run locally because it needs the postgres profile.Media (current head; media commit
e8e313e99893ebe07fc235df316cc2e2e3f53590, removed in the next commit):Phone keyboard and history walkthrough (webm, 390 then 360). The steps and the state checked after each:
tab=workspace).The Storybook "Pages/Agent detail › First version" frame at 1440 is sha256-identical before and after; at 390 only the edge fade differs.
Open PRs (3-way merge, conflict files/hunks with origin/main vs with this head):
No new conflict hunk. #2877 (C5) has landed and is merged in. One semantic follow-up: #2445 adds a "Policy tester" tab through the same builder and clicks it with
getByRole("button", { name: "Policy tester" })(2 lines). After this lands, those two lines need"tab". #2599 ("Policy denials") and #2518 ("Network") add tabs too, but their tests don't address them by role.Risks
Low to medium. Keyboard and screen-reader behaviour of the tab strip changes on purpose; pointer behaviour is unchanged. Tests or tools outside this repo that find these tabs by
buttonrole oraria-currentneed the tab role /aria-selected.