fix(ui): ring a row only while it holds the focus itself - #84
Merged
Conversation
A settings row read plain focus, which a Focus node reports for its descendants too, so tabbing to the Theme select painted a ring around the select and another around the row it sits in. The row now follows the primary focus, as TRTreeNav upstream already does. The ring moves to a foreground decoration that is always present and only changes colour. Adding and removing a border inset the content box, which re-laid-out the trailing control and destroyed its focus node as traversal stepped onto it. A row whose tap only repeats what its trailing control already does now hands the control its single tab stop, so a switch setting costs one Tab press and announces itself once. A row whose control does something else, such as a session tab and its close button, keeps both stops. The tool card in the transcript is a tab stop that drew no focus at all, which is the same invariant seen from the other side, so it draws the ring the same way.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Settings → Appearance painted two focus rings when tabbing to the Theme select: one around the
TRSelecttrigger and one around the whole row. The same doubling appeared on Language and on every settings row carrying a focusable control.Root cause
CoderListRowdrove its ring fromFocus.onFocusChange, which reportsFocusNode.hasFocus— true when the node or any descendant holds primary focus.canRequestFocus: falsestopped the row from being a tab stop but not the callback, so a focused trailing control flipped the row too.Previous reports were fixed at call sites, never at the row primitive, which is why it kept coming back. Upstream
tinyrack_uifixed the identical bug forTRTreeNavin 0.29.1 by following the primary focus; this row never got that treatment (it predates it,d974d83).Changes
coder_list_row.dart— trackhasPrimaryFocusinstead ofhasFocus.coder_list_row.dart— the ring moves to an always-presentforegroundDecorationthat only changes colour. Adding/removing an inset border shrank the content box and re-laid-out the trailing control, destroying its focus node mid-traversal — the keyboard trap upstream documented in CHANGELOG 0.30.1 / 0.31.1.controlOwnsFocus(new, plumbed throughSettingsRow) — a row whoseonTaponly repeats what its control already does hands the control its single tab stop and drops its ownbuttonsemantics. Set onCoderSwitchRow/CoderCheckboxRow; pointer taps on the row still toggle.chat_tool_card.dart— a tab stop that painted no focus indicator at all, the same invariant from the other side. Now rings the same way.Session/terminal tabs (
app.dart:1459,1498) and the skill list row keep two tab stops: selecting and closing/toggling are different actions.Audit
All 44
CoderListRow/SettingsRowcall sites were enumerated. The spurious ring affected every non-interactive row with a focusable control (general_settings_page.dart:121,177,agent_settings_page.dart:431,454,advanced_settings_page.dart:65,settings_page.dart:369,474,598,app_settings_page.dart:325,app.dart:1785); all are fixed by the primitive change with no call-site edits.Tests
New
apps/coder_app/test/focus_ring_test.dart(7 cases): four failed for the intended reason before the fix. Covers the double ring, a nav row still ringing, no layout shift on focus, one tab stop per switch row, two stops kept for a distinct trailing action, pointer toggle preserved, and tool-card focus visibility.Verification
dart run melos verify— all 12 gates pass, goldens unchanged (unfocused pixels are identical).dart run melos verify:debug— passes against the real Debug Flutter runner and embedded daemon (run underxvfb-runon this headless machine; CI's Linux Debug E2E shard covers it natively).