feat(a11y): show a keyboard focus indication on buttons - #285
Merged
Conversation
Tab through the login form, settings, or any modal and nothing moves — there is no way to tell where focus is. `RBX_FOCUS_RING` had no readers anywhere in the app, and the base button pinned the `focus` uniform to 0.0 in *both* animator states so the shader's `mix(color, color_focus, focus)` always collapsed to `color`. That was not an oversight. It worked around a real bug: `Button` grabs key focus on `FingerDown` when `grab_key_focus` is set, firing the same `Hit::KeyFocus` a Tab press does, so clicking a button and moving the pointer away left it stuck wearing the focus colour (see e957f84). Fix the cause instead: `grab_key_focus: false` means a click no longer takes focus, so the animator can do its job and the indication appears only when focus arrives by keyboard — what CSS calls `:focus-visible`. Several places in the app already set this flag. Focus shows two ways, because one does not fit every button: - Variants that already draw a border turn it accent (`border_color_focus`). - Flat variants tint the fill (`color_focus`, new `RBX_FOCUS_TINT` token). Driving `border_size` instead would have been wrong twice over. The shader insets its geometry by it — `sdf.box(border_size, border_size, w - 2*border_size, …)` — so giving flat buttons a width just for focus shrinks every button by 4px. And it is a scalar the shader never mixes by `focus`, so animating it per-state would have forced `border_size: 0.0` in the off state, erasing the borders of the ~130 call sites that set their own. Also drops the "Robrix currently disables button focus visuals" note in the tokens file, which no longer describes the code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Aug 5, 2026
tyreseluo
added a commit
that referenced
this pull request
Aug 5, 2026
… erased itself (#287) * fix(a11y): stop keyboard focus from repainting ghost buttons solid #285 gave the base button `color_focus: RBX_FOCUS_TINT`, but that token is an opaque `#0E8C9E` and the shader replaces the fill outright: fill = color_fill.mix(color_fill_focus, self.focus) At focus=1.0 the fill *is* the tint. Buttons that carry their own background were unaffected, which is why this got through review — but the ghost variants, whose `color` is transparent, turned into a solid accent block with a grey icon on it. Inverted, not outlined. Those are the composer tool buttons and the per-message copy button: the two highest-traffic controls in the app. The base now focuses to `RBX_ACCENT_SOFT`, a wash rather than a repaint. The token keeps its opaque value on purpose; making *it* translucent would let the page show through the solid variants instead. The four semantic variants get an explicit `color_focus` equal to their own fill. Left to inherit, a green confirm or a red destructive button would have washed teal on focus and lost the colour that identifies it — they already draw a border, so `border_color_focus` carries the signal for them. * feat(a11y): drive the room quick switcher from the keyboard Typing a room name and pressing Enter did nothing. The results list had no concept of a selection — items only responded to `click_button.clicked` — so switching rooms, the highest-frequency action in the client, always ended at the mouse. Arrow keys move a selection, Enter opens it, and the selected row is tinted so the keys are not aiming blind. Keys are claimed in `Event::KeyDown` rather than through a `Hit`: focus stays in the query field while typing, so the list never receives hits of its own. Only Up/Down/Return are taken; every other key still reaches the field. Selection is clamped at the ends instead of wrapping — holding an arrow should come to rest on the first or last result, not cycle — and resets whenever the result set changes, since index 3 of the previous query has nothing to do with index 3 of this one. The highlight is an Animator over a shader instance, not `script_apply_eval!`, which does nothing on widgets built from a FlatList template (pitfall #40). Both animator states are written on every frame: rows are reused by index, so writing only `on` would leave the old row highlighted after the selection moved. * fix(rooms-list): let a filter that matches nothing show nothing Typing a room name that matched nothing brought the entire room list back while the query was still sitting in the box — the filter looked broken at the moment it was working correctly. `update_displayed_rooms` cleared `display_filter` and regenerated whenever every category came back empty. That also made the "no matching rooms" status unreachable: by the time `update_status` ran, `display_filter` was already None, so the count fell into the unfiltered arm. The fallback dates from "improve people lookup and fallback empty local filters", when the sidebar filter and people search shared a path. They no longer do: the sidebar list is driven by `MainFilterAction`, while people results (`RoomFilterResultTarget::RemoteUser`) are rendered by the quick switcher modal, a separate surface with its own list. Nothing depends on the sidebar staying populated. Note the newly reachable status string is English, like the five siblings it sits with — this file has no i18n at all yet, so the line is no more English than before. Worth its own change.
tyreseluo
added a commit
that referenced
this pull request
Aug 5, 2026
* fix(search): submit the in-room message search on Enter
The pane emitted `QueryChanged` only from its 0.6s debounce timer, and
`TextInput` reports Return separately from `changed` — so Enter did
nothing at all, and the only way to run a search was to stop typing and
wait. A search box that ignores Enter reads as broken.
Handle `returned()` the way the public directory search already does:
cancel the pending debounce (otherwise the same query fires a second
time a moment later, costing another round-trip) and emit immediately.
Also clear the debounce timer handle when it fires, so the field doesn't
keep a stale timer id.
* fix(editing): show a saving state so an edit can't be sent twice
Saving an edit submitted the request and left the pane fully live —
there was a `TODO` here for exactly this. On a slow server the user
clicks the checkmark again, or presses Enter again, and a second edit of
the same message goes out; both land, and the room sees two edit events.
Disable the accept button while the request is in flight and say so in
the header line, which already reads "Editing:" and is the natural place
for "Saving…". An icon-only checkmark has nowhere to put that text, and a
button that silently stops responding reads as a broken button.
The flag, not the disabled button, is what actually closes the hole:
Enter in the text input never goes through the button.
On failure the pane returns to its editable state so the user can retry
or cancel, rather than being stranded on "Saving…" with a dead button.
Reopening the pane also resets it, in case an edit was abandoned while
its request was still running.
* feat(timeline): give fenced code blocks the dark code panel
The five `RBX_CODE_*` tokens had no readers: agent code output rendered
on the same light inset surface as everything else, so a block of SQL or
Rust sat in the timeline with nothing marking it as output.
Two things were in the way.
Upstream's `DrawFlowBlock` shader fills the block-code branch and the
inline-code branch from a single `code_color` uniform, so the two could
not be told apart — colouring the panel would have turned every inline
`code` span into a dark chip mid-sentence, which reads as a redaction
bar. Adding a second uniform and overriding `pixel` separates them;
every other branch is upstream's, copied unchanged.
The text colour was harder. `TextFlow` takes it from exactly one place,
the top of its `font_colors` stack, and `Html` never pushes anything for
`<pre>` — there is no per-block text colour to set, so light text has to
enter through the document itself. `<font>` is dispatched to
`MatrixHtmlSpan`, which does push `font_colors`, so the markup is the
only lever available from outside the widget.
That lever has a sharp edge: `Html` draws a `<font>` as a *leaf* — it
renders `find_text()`, the first text node and nothing more, then skips
to the close tag. Wrapping a block that contains markup (a bridge that
ships pre-highlighted code) would render its first line and silently drop
the rest. So a block is rewritten only once it is known to hold nothing
but text; anything else keeps exactly the rendering it has today, on the
light surface. Losing code is far worse than a block that stayed light.
Tests cover the wrapping cases and, more importantly, the refusals: a
block with markup, an unterminated block, and an empty one all come back
byte-for-byte unchanged.
Not included: syntax highlighting (`RBX_CODE_KEYWORD` / `_STRING` /
`_COMMENT`) and the translation footer. Those belong to the
`CodeOutputCard` component in spec §4.7, which is a build of its own.
* fix(a11y): mark mention badges with more than colour
A mention badge and an unread-message badge drew the identical glyph and
differed only in fill: red vs grey. For anyone who can't separate those
two hues the badges are the same badge — and even with normal colour
vision, a badge seen on its own has no neighbour to compare against.
Prefix the mention count with `@`. The pill widens by one glyph's worth
so the extra character isn't clipped.
* fix(ux): make failed, pending, and offline states visible and actionable
Six fixes that all touch `app.rs`, `room_screen.rs`, or
`sliding_sync.rs`, so they land together.
**A failed image printed its `mxc://` URI and stayed broken forever.**
The timeline showed `Failed to fetch image from Some("mxc://…")` — a
debug-formatted `Option` around a server-internal identifier the reader
cannot open, copy anywhere useful, or act on. Worse, a `Failed` cache
entry is sticky: `try_get_media_or_fetch` returns it forever and never
re-requests, so one network blip left the image broken for the rest of
the session. `TextOrImage` gains an error state with a Retry button that
drops the entry, which is what makes the next draw fetch again.
**Empty reactions reached the server; failed ones were silent.** The
custom-reaction field sent its text unvalidated, so an all-whitespace key
became a real event every client then had to render as an empty pill.
Trimming also fixes a quieter bug: reaction keys are opaque strings, so
" 👍 " and "👍" aggregate separately and stray whitespace splits the
tally. On the failure side the pill is repainted optimistically the
instant it's clicked, so a silent error left the UI claiming a reaction
that never landed.
**Dismissing the join/leave modal orphaned the request.** Closing the
dialog cleared `kind`, after which the result action matched nothing:
no success, no failure, nothing. The request itself keeps running — it
must, the server has already been asked. A closed `Modal` stops
forwarding events to its content, so the modal can't watch for its own
result; it hands the request to the app root, which reports the outcome
by popup from a context that stays alive.
**Buttons reached by Tab ignored every key.** Makepad's `Button`
registers a nav stop and plays `focus.on` from `Hit::KeyFocus`, so Tab
both reaches it and lights the ring added in #285 — but `handle_event`
has no `Hit::KeyDown` branch. A ring that promises "this is the control
you're about to operate" and then does nothing is worse than no ring.
Wrapping `Button` isn't available here: `WidgetRef::button()` downcasts
to the concrete type, so a wrapper would turn all ~130
`self.button(…).clicked(actions)` call sites into silent no-ops.
Instead the app root finds the focused button and emits the
`ButtonAction::Clicked` it would have raised itself — `clicked()` matches
purely on widget uid, so every existing call site works untouched.
Disabled buttons are skipped via `Button::enabled()`, the same flag the
widget gates real clicks on.
**On mobile, nothing showed you were offline.** The only connection
indicator lives in the rooms list header, which on mobile is a different
screen — once inside a room, nothing told you your messages weren't
reaching anyone. Adds a strip in the room, mobile only (on desktop the
rooms list is on screen beside it and already carries the icon). The
sync state was published only as a one-shot broadcast, which a widget can
act on but not query, so a room opened *after* the drop would never learn
about it; `is_sync_service_offline()` makes it readable.
**Clicking a search result threw away the directory hits.** Reopening the
modal rebuilt the list from local rooms alone, so a people/rooms search
the user had to request by hand vanished and the only way back was to run
it again. Local matches are still recomputed every time — rooms may have
been joined or left — but the remote ones are carried over, keyed by
query so they self-invalidate the moment the text changes.
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.
Tab through the login form, settings, or any modal and nothing moves — there is no way to tell where focus is.
RBX_FOCUS_RINGhad no readers anywhere in the app, and the base button pinned thefocusuniform to0.0in both animator states, so the shader'smix(color, color_focus, focus)always collapsed back tocolor.Why it was disabled, and why that matters
Not an oversight — it worked around a real bug.
Buttongrabs key focus onFingerDownwhengrab_key_focusis set, firing the sameHit::KeyFocusthat a Tab press does (button.rs:504-513). So clicking a button and moving the pointer away left it stuck wearing the focus colour. The fix at the time was to make the focus state a no-op (e957f84).Simply re-enabling the animator would bring that bug straight back. Instead this removes the cause:
grab_key_focus: false— a click no longer takes focus, so the animator can do its job and the indication only appears when focus arrives by keyboard. That is what CSS calls:focus-visible. Several places in the app already set this flag (agent_add_modal.rs,devices_settings.rs,agent_settings.rs).Two indications, because one does not fit every button
border_color_focus)color_focus, newRBX_FOCUS_TINT)Driving
border_sizewould have been wrong twice over, which is worth recording since it is the obvious first instinct:sdf.box(border_size, border_size, w - 2*border_size, …)(button.rs:109-113) — so giving flat buttons a width just for focus shrinks every button by 4px.focus, so animating it per-state means settingborder_size: 0.0in the off state, which would erase the borders of the ~130 call sites that set their own.Also drops the "Robrix currently disables button focus visuals" note in the tokens file, which no longer describes the code.
Testing
cargo test --lib --features agent_chat— 589 pass.typos src/clean. Ran the app and confirmed noRBX_*property fails to resolve.Checked by hand, with the regressions this could plausibly cause called out first: button sizes are unchanged (the geometry trap above), borders on bordered variants are intact, and clicking a button then moving away leaves no residue (the original bug). Tab moves a visible indication through the login form, settings and modals; hover, pressed and disabled states are unaffected.
🤖 Generated with Claude Code