From 5cf4ac5efaf7eb2bc3ef10efa1cd94ef6eb161ef Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Wed, 23 Sep 2026 11:02:43 +0800 Subject: [PATCH] revert(sidebar): paint the agent disc solid again, mark in white MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last two passes at the avatar — a flat theme disc, then a bare mark painted in the brand colour — each gave something up the solid disc had: the flat disc lost the hue that tells one agent from another down a column of rows, and the bare mark read lighter and less settled than the disc beside the status dot. Neither was better than where it started. Back to a solid fill of the agent's brand with the mark in its own ink, and the hairline `needs_edge` adds for Codex and Grok's pure black on a dark window. The shell avatar goes back to its muted disc. `mark_ink` goes with the style it served; the toolbar changes from the same PR stay. --- src/ui/presets.rs | 17 ++++---- src/ui/tab_strip.rs | 95 +++++++++++++++++++++++---------------------- 2 files changed, 55 insertions(+), 57 deletions(-) diff --git a/src/ui/presets.rs b/src/ui/presets.rs index df9697e1..e4ae4f79 100644 --- a/src/ui/presets.rs +++ b/src/ui/presets.rs @@ -530,16 +530,13 @@ pub(crate) fn caret_ink(caret: Hsla, background: Hsla, foreground: Hsla) -> Hsla } } -/// A brand colour painted straight onto a theme surface, as ink. -/// -/// A brand colour is a fixed value; the surface under it is not. Codex and Grok -/// are both pure black and would be nothing at all on a dark window, so the -/// value is walked toward whichever end of the range reads on that surface -/// until it clears `ACCENT_FLOOR` — hue first, legibility enforced. The walk is -/// a no-op for the colours that already clear it, which is most of them, so an -/// agent's mark is its own orange or blue in every theme. -pub(crate) fn mark_ink(brand: u32, surface: Hsla) -> Hsla { - gpui::rgb(legible_accent(pack(surface), brand)).into() +/// Whether a filled shape needs a hairline to stay a shape. A brand colour is a +/// fixed value; a theme background is not, and pure black on a dark window is +/// no shape at all. +pub(crate) fn needs_edge(fill: u32, surface: Hsla) -> bool { + let rgb = crate::terminal::palette::hsla_to_rgb(surface); + let packed = (rgb.r as u32) << 16 | (rgb.g as u32) << 8 | rgb.b as u32; + contrast(fill, packed) < 1.25 } /// Whether a surface is dark enough that a halo cut in its own colour stops diff --git a/src/ui/tab_strip.rs b/src/ui/tab_strip.rs index db3d84e5..962f2466 100644 --- a/src/ui/tab_strip.rs +++ b/src/ui/tab_strip.rs @@ -1316,14 +1316,22 @@ impl Tty7App { size: f32, cx: &App, ) -> gpui::AnyElement { - // The wrapper positions; the slot below centres the mark in it. + // The wrapper positions; the disc below carries the radius. // `status_dot` hangs itself off the edge with negative offsets — that // overhang is what makes it a badge on the avatar rather than a notch - // in it — so nothing here may clip its own children. + // in it — and as a child of the rounded element the overhang was + // clipped along the arc, leaving a crescent. let base = div().id(id).flex_shrink_0().relative().size(px(size)); - // The mark is the whole avatar now, so the slot is only ever geometry: - // it holds the column's width whatever the glyph inside it is. - let slot = || div().size(px(size)).flex().items_center().justify_center(); + // Fill, hairline and mark all live here, so the radius only ever clips + // the disc's own paint. + let disc = || { + div() + .size(px(size)) + .flex() + .items_center() + .justify_center() + .rounded_full() + }; match agent { Some(agent) => { let hollow = status == Some(crate::core::cli_agent::AgentStatus::Waiting); @@ -1336,23 +1344,32 @@ impl Tty7App { Some(state) => format!("{} — {state}", agent.display_name()), None => agent.display_name().to_string(), }; - // Hue says *who*, the dot says *what it wants*. Both readings - // are worth having down a column of twenty rows, and a filled - // brand disc took the first at the price of the second: a - // saturated circle on every row is three times the coloured - // area of the badge that is actually about state, and the eye - // goes to area. Painting the mark itself keeps the hue — and - // the marks are silhouettes, so this is the shape either way. + // The disc is a solid fill of the agent's brand on every + // row, lit or not: a tint reads as a disabled tab, and the + // colour is how the eye tells one agent from another down a + // column of twenty. + let accent = agent.accent_rgb(); + let surface = cx.theme().background; base.child( - slot().child( - gpui::svg() - .path(agent.icon_path()) - .size(px(size * 0.72)) - .text_color(crate::ui::presets::mark_ink( - agent.accent_rgb(), - cx.theme().background, - )), - ), + disc() + .bg(gpui::rgb(accent)) + // Codex and Grok are both pure black, which is the + // window fill on a dark theme — the disc dissolves and + // leaves the glyph floating. A hairline keeps it a disc + // in any theme. + .when(crate::ui::presets::needs_edge(accent, surface), |d| { + d.border_1().border_color(cx.theme().border) + }) + .child( + gpui::svg() + .path(agent.icon_path()) + .size(px(size * 0.54)) + // SVG assets render as a single-colour mask, so + // the mark's colour comes from the agent rather + // than from the file. The tray icon reads the + // same answer. + .text_color(gpui::rgb(agent.icon_rgb())), + ), ) .when_some(dot, |b, dot| b.child(dot)) .tooltip(move |window, cx| { @@ -1360,15 +1377,13 @@ impl Tty7App { }) .into_any_element() } - // A shell is the absence of an agent, and it reads as one: no hue - // to spend, and quieter than the marks it shares the column with. None => base .child( - slot().child( + disc().bg(cx.theme().muted).child( gpui::svg() .path("icons/terminal.svg") - .size(px(size * 0.72)) - .text_color(cx.theme().muted_foreground), + .size(px(size * 0.56)) + .text_color(cx.theme().foreground.opacity(0.65)), ), ) .when_some(ssh, |b, rgb| { @@ -2437,34 +2452,20 @@ mod tests { } #[test] - fn a_brand_mark_keeps_its_hue_and_stays_visible() { - use crate::ui::presets::mark_ink; + fn a_brand_disc_that_matches_the_window_gets_an_edge() { + use crate::ui::presets::needs_edge; let dark: gpui::Hsla = gpui::rgb(0x111111).into(); let light: gpui::Hsla = gpui::rgb(0xffffff).into(); let codex = crate::core::cli_agent::CLIAgent::Codex.accent_rgb(); let claude = crate::core::cli_agent::CLIAgent::Claude.accent_rgb(); - // The hue is the identity: it survives both ends of the theme range, - // or the mark stops saying which agent this is. - let seed: gpui::Hsla = gpui::rgb(claude).into(); - for surface in [dark, light] { - let ink = mark_ink(claude, surface); - assert!( - (ink.h - seed.h).abs() < 0.02 && ink.s > 0.3, - "Claude's mark went grey on {surface:?}" - ); - } - - // Codex is pure black, which is the window fill on a dark theme. With - // no disc under it, an unlifted mark is not a mark. - assert_eq!(codex, 0x000000, "Codex's brand colour is pure black"); + assert_eq!(codex, 0x000000, "Codex's disc is pure black"); assert!( - mark_ink(codex, dark).l > dark.l + 0.25, - "a black mark on a dark window is not there" + needs_edge(codex, dark), + "a black disc on a dark window is not a disc" ); - // And it is only lifted where it has to be: on a light window black - // is the brand, and stays it. - assert_eq!(mark_ink(codex, light).l, 0.0); + assert!(!needs_edge(codex, light)); + assert!(!needs_edge(claude, dark) && !needs_edge(claude, light)); } #[test]