Repository navigation
list: Draw accent-filled rows in accent_foreground - #3774
Conversation
The cursor and hover rows of searchable lists (Select, ComboBox, MultiComboBox) and a selected ListItem without the active highlight paint the theme's `accent` fill but kept `foreground` for their content. A theme whose accent is a saturated color then puts page text on it, which can fall below the contrast the theme pairs with that fill. Use `accent_foreground` on those fills, as PopupMenu rows and a checked Toggle already do. The searchable list check mark now follows the row's text color instead of always using `foreground`.
huacnlee
left a comment
huacnlee
left a comment
There was a problem hiding this comment.
Verified locally at 34dcb07: cargo fmt --check, cargo clippy -p gpui-component --all-targets -D warnings and cargo test -p gpui-component --test integration (52 passed, incl. the two new accent_fill_rows tests) are clean; CI is green.
The change is a pure theming fix with no public API: content on an accent fill now uses accent_foreground, matching PopupMenu, Toggle and the calendar. Default theme renders the same because accent_foreground == foreground there. Disabled rows still win in searchable_list/item.rs because the disabled text_color is applied after the selected one, and the check icon now inherits the row color, which is correct.
One small note: the hover row uses accent.opacity(0.7) with full accent_foreground. On a light theme with a saturated accent, white text on a 70% fill is a bit lower contrast than on the cursor row (the description mentions this). That is acceptable, but it is the trade-off the owner should be aware of.
Looks merge-ready from my side.
🤖 Noop (reviewer) · AI agent acting for @huacnlee
Description
The cursor and hover rows of searchable lists (
Select,ComboBox,MultiComboBox) and a selectedListItemwithlist.active_highlight = falsepaint the theme'saccentfill but keepforegroundfor their content. On a theme whose accent is a saturated color, that puts page text on the fill, below the contrast the theme pairs with it.PopupMenurows, a checkedToggle, the calendar and the command palette already drawaccent_foregroundonaccent.This change uses
accent_foregroundon those three fills. The searchable list check mark now follows the row's text color instead of forcingforeground, so it stays legible on the cursor row; on a disabled row it dims with the label.The default theme is unaffected, since its
accent_foregroundequalsforeground. Among the bundled themes, Solarized Light and Alduin move from 4.2:1 and 5.3:1 to 10.2:1 on the cursor row. The hover row keeps its 70 % accent fill, so its contrast is lower than the cursor row's; the theme's pairing still applies there.Screenshot
Story:
Select› "Search and clear", dropdown open,↓once, pointer on a row.accent.background #2563eb,accent.foreground #ffffff)Cursor row text on the accent fill, Blue Accent: 3.83:1 before → 5.17:1 after. Solarized Light: 4.23:1 → 10.23:1.
How to Test
cargo test -p gpui-component --test integration accent_fill— two new tests incrates/component/tests/integration/accent_fill_rows.rs. They apply a light theme with a blue accent and read the text color handed to a row's content: the searchable list cursor row and hovered row (real pointer move) must takeaccent_foreground, a plain row keepsforeground; a selectedListItemkeepsforegroundwith the defaultlist_activehighlight and takesaccent_foregroundoncelist.active_highlightis off. Both fail onmain.cargo run -p gpui-component-story -- Selectwith a theme whoseaccent.foregrounddiffers fromforeground(for example Solarized Light from the theme picker): open a dropdown, move the cursor with↓, hover a row.cargo test -p gpui-component,cargo clippy -p gpui-component --all-targets -- -D warnings,cargo fmt --all -- --check,typos: clean on Linux.Checklist
cargo runfor story tests related to the changes.