Skip to content

button: Keep the focus line off text and legible on fills - #3300

Merged
madcodelife merged 1 commit into
mainfrom
button-focus-line-placement
Sep 28, 2026
Merged

madcodelife merged 1 commit into
mainfrom
button-focus-line-placement

Conversation

@madcodelife

@madcodelife madcodelife commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

Follow-up to #3299 (#3298). When Theme::focus_ring is off, #3299 draws a 1px ring line on a borderless button's edge. That has two visual problems:

  • Text / Link: these variants have no padding, so the line sits on the label and its underline.
  • Filled variants (Primary, Secondary, Danger, …): a ring-coloured line on the edge barely shows against dark fills. In the default light theme it is neutral-400 on neutral-900.

Change

Each borderless button now picks where its focus line goes, through a crate-private FocusLine passed to styled::focus_style:

Variant Line Colour
Ghost on the edge (unchanged) ring
Text, Link 2px (0.125rem) outside the edge, radius = element radius + 2px ring
Primary / Secondary / Danger / Info / Success / Warning / Custom 2px inside the fill, radius = max(element radius − 2px, 0) the variant's own foreground at 60%
Default, any .outline() tinted border (unchanged) ring
  • Filled variants use their own foreground. The theme already keeps a button's foreground legible against its fill, in every variant and in both modes. The ring token is tuned for the page background, not for a primary fill.
  • Why not a 2px ring line? It is still low-contrast on fills.
  • Why not an outside offset ring (background-coloured gap plus a ring line)? It is exactly what focus_ring = false exists to avoid: containers clip it.
  • Text/Link outside is safe in practice. The outset is only 3px including antialiasing. Callers place these buttons with padding around them; the ai-chat HITL card's "other option" text button renders unclipped. A clipping ancestor flush with such a button would cut the line.
  • Layout is unchanged in every case. The line is an absolutely positioned child and no border width is added.
  • Nothing else changes. focus_ring = true, bordered variants, and other callers of focus_ring_style behave as before.

Tests

crates/kit/tests/rendering.rs. These run on the main thread (harness = false, Metal), with Ghost / Text / Link / Primary buttons and focus_ring = false.

focus_lines_stay_off_content_and_contrast_with_fills runs in light and dark. Focus moves with Tab through Root. For each button:

  • Every button: no pixel changes outside the button (outside the button plus 3px for Text/Link).
  • Ghost: ring-coloured pixels on the top edge.
  • Text / Link: no pixel changes inside the button's bounds, which is where the label is (the four corner squares crossed by the line's arc are excluded). There are ring-coloured pixels 2px above the edge.
  • Primary: the line row 2px inside differs from the fill by ≥ 96/255 in some channel.

clicking_a_button_draws_no_focus_line: clicking a hovered button leaves focus empty and the capture identical to the hovered one.

These tests fail on origin/main before #3299 at ghost, and on #3299 alone at text (288 device pixels over the label).

cargo test -p gpui-kit --features test-support,component,assets --test rendering
cargo test -p gpui-component
cargo clippy -p gpui-component -p gpui-kit --features gpui-kit/test-support,gpui-kit/component,gpui-kit/assets --all-targets -- -D warnings

Text and Link buttons have no padding, so a line on their edge sat on
the label; draw it 2px outside instead. On filled variants a `ring`
line on the edge barely showed against dark fills; draw it 2px inside
in the button's foreground at 60%, which the theme keeps legible
against the fill. Ghost keeps the edge line, bordered variants keep
the tinted border.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@madcodelife
madcodelife merged commit eb6c13a into main Sep 28, 2026
11 checks passed
@madcodelife
madcodelife deleted the button-focus-line-placement branch September 28, 2026 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant