From 9e378ff7ffef14333e510195bbbb1da45be463ba Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sat, 7 Nov 2026 13:24:31 +0700 Subject: [PATCH 01/14] update docs --- docs/CHECKLIST.md | 8 + docs/platform-metrics-audit.md | 266 +++++++++++++++++++++++++++++++++ 2 files changed, 274 insertions(+) create mode 100644 docs/platform-metrics-audit.md diff --git a/docs/CHECKLIST.md b/docs/CHECKLIST.md index a05caa4..3f134d2 100644 --- a/docs/CHECKLIST.md +++ b/docs/CHECKLIST.md @@ -13,6 +13,14 @@ is the first downstream consumer; its screens drive the order. Gaps surfaced by the signed comparison; ordered by signed's usage. +- [ ] **Platform metric fidelity** — native-sizing pass over the theme and + components: stated `control_height` per platform (macOS 24 measured / + Windows 32 / GNOME ≈34 / KDE ≈33), Linux root font sizes (GNOME 14.67px, + KDE 13.33px), platform overlay/sm radii, per-source border + focus + policy (macOS/GNOME borderless fills, Windows/KDE 1px), titlebar + heights (GNOME 47), a theme spacing scale, switch/icon calibration; + ranked findings in + [platform-metrics-audit.md](platform-metrics-audit.md). - [ ] **Virtual list** — generic virtualized variable-height rows with a scroll handle, promoted from the editor views' pattern over gpui's `list`; signed's PR lists and commit diff drive it. diff --git a/docs/platform-metrics-audit.md b/docs/platform-metrics-audit.md new file mode 100644 index 0000000..232b102 --- /dev/null +++ b/docs/platform-metrics-audit.md @@ -0,0 +1,266 @@ +# Platform metrics audit — Apple HIG, GNOME HIG, Fluent, KDE HIG + +Why our controls read small and foreign next to native ones. Assessed 2026-11 +against the primary sources: the libadwaita stylesheet (`libadwaita` `main`, +`src/stylesheet/**.scss`), WinUI 3 theme resources (`microsoft-ui-xaml` +CommonStyles) and the Learn design pages, Kirigami `units.cpp` + Breeze +`breezemetrics.h`/`breezestyle.cpp`, and the Apple HIG pages. Apple publishes +no pixel metrics, so macOS numbers are **measured on this machine via AppKit +`fittingSize`** — note the OS-version variance called out below. Queue entry: +[CHECKLIST.md](CHECKLIST.md) — "Platform metric fidelity". + +## Verdict + +Three real problems, none of them "spacing values are off by a bit": + +1. **Heights.** `control_height` is derived as `1.75 × root_font_size`. + Native platforms do not size controls that way — Windows fixes buttons at + 32px, GNOME and Breeze add fixed padding to the font height. Result: our + buttons are −23% vs native on Windows, −18% on GNOME. The main "looks + small" driver. +2. **Radii.** Only `radius.control` adapts per platform; `sm`/`overlay` are + frozen at 4/10. Native overlay radii span 5px (KDE) to 15px (GNOME) — we + are 5px off in both directions. +3. **Borders.** We paint `border_1()` on nearly everything (shadcn-style). + That matches Windows and KDE and contradicts macOS and GNOME/libadwaita, + whose controls are borderless fills at normal contrast. The "feels weird" + driver. + +Structural gap underneath the spacing complaint: **the theme has no spacing +tokens** — components hardcode gpui utilities (`p_1`, `px_2`, `gap_1p5`, +`min_w_56`), so platform rhythm (GNOME's 6px grid, KDE's 4/6/8) cannot be +expressed. + +## What the platforms specify + +### Apple / macOS + +The HIG publishes no pixel metrics for macOS — verified across the buttons, +text-fields, toolbars, toggles, menus, windows, layout, sidebars, sheets, +materials and designing-for-macos pages. What is published: the type ramp +(body 13pt, min 10pt) and the 44×44pt hit-region rule. Everything else below +was measured locally (AppKit, regular control size, this macOS build): + +| Control | Measured | Confidence | +|---|---|---| +| Push button | **24pt** (small 20, mini 16) | measured | +| Rounded text field | **24pt** | measured | +| Pop-up button / segmented | **24pt** / **24pt** | measured | +| Switch | **54×24** | measured | +| Checkbox (empty control) | **16×16** | measured | +| Slider control height | **16pt** | measured | +| Window titlebar | **32pt** on this build; 28pt on older macOS | measured, version-dependent | +| Body text | 13pt | published | + +Border language: toolbar items and image buttons are *explicitly borderless* +("toolbar items don't include a bezel"); hierarchy is expressed by fill, not +borders; fields keep a hairline bezel; menus/popovers separate by shadow and +material, not borders. Concentricity rule: "buttons, text fields, headers, +and footers have corner radii that are concentric with bar corners" (rule +only, no values). + +### GNOME / libadwaita + +Exact values from the stylesheet; GTK px are logical pixels at 96dpi. + +| Metric | Value | +|---|---| +| Button total height | min-height 24 + padding 5px 10px ⇒ **≈34px** | +| Entry | **34px** (border-box, no vertical padding) | +| Header bar | **47px** | +| Menu item | min-height **32px**, padding `0 12px`; container padding 6px | +| Switch | knob 20px, trough ≈**46×26** | +| Checkbox | 14px + 3px pad each side ⇒ **20px**, radius **6px** | +| Radius ladder | button/entry **9px** (`$button_radius`), checkbox 6px, cards 12px, menu/popover/dialog/window **15px**, menu items 9px | +| Font | Adwaita Sans 11pt = **14.67px** | +| Focus | **2px accent outline** (accent @50% default, 80% high contrast) | + +Borders: **controls are borderless at normal contrast** — raised buttons, +entries, switches, menus draw shape from translucent `currentColor`-mix +fills. 1px borders are a high-contrast affordance only. Always-on hairlines +are reserved for the window outline, headerbar bottom shade, frames, and +boxed-list row separators. + +### Windows 11 / WinUI 3 (Fluent 2) + +Published tokens from the theme-resource XAML and Learn pages, at 100% +scaling: + +| Metric | Value | +|---|---| +| Button | **32px** (padding 11,5,11,6 + 1px border + Body 14/20); `ButtonHeight=32` lives in OS generic.xaml | +| TextBox | **32px** (`TextControlThemeMinHeight`, OS generic.xaml — not fetched) | +| CheckBox box | **20×20**, 1px stroke (row 32) | +| ToggleSwitch | track **40×20** pill; knob **12×12** resting → 14 hover | +| Radius | ControlCornerRadius **4px**, OverlayCornerRadius **8px** | +| Title bar | **32px** standard, 48px "Tall" | +| Type ramp | Caption 12, Body **14**, Body Large 18, Subtitle 20, Title 28 | +| Spacing | 8 between buttons, 12 control↔label, 16 edge↔text — a **4px grid** | +| Focus | system ring drawn outside the control (negative focus margin) | + +Borders: buttons and textboxes carry a **1px elevation border** (gradient +stroke, lighter top + darker bottom); the accent button keeps a border too; +checkbox box 1px; switch track 1px while off; slider borderless except the +thumb. + +### KDE Plasma 6 / Breeze + +Source constants (`breezemetrics.h`, `units.cpp`); the HIG names the usage +rules, not px. + +| Metric | Value | +|---|---| +| Kirigami units | gridUnit **18**, smallSpacing **4**, mediumSpacing **6**, largeSpacing **8**, cornerRadius **5** | +| Button | fontHeight + 2×(6+2) ⇒ **≈33px** at the default 10pt font | +| Line edit | fontHeight + 12 ⇒ **≈29px** | +| Checkbox | **20×20**, radius 4 | +| Switch (qqc2) | **36×18**, knob 18 | +| Radius | **5px** everywhere — buttons, line edits, menus, popups, windows | +| Font | Noto Sans 10pt = **13.33px** | +| Titlebar (KWin) | max(fontHeight, 2×gridUnit = 36) + margins ⇒ **≈44px** | +| Focus | rounded ring filled highlight @30%; item views get a focus underline | + +Borders: 1px outline (`PenWidth::Frame`) on all controls; **flat buttons are +borderless until hover/press/check**; menus and popups framed 1px. + +## What our tokens resolve to today + +`SizeTable::with_root_font_size` medium class (height 1.75rem, padding +1rem) fed by each adapter's inputs in `crates/theme/src/source/`: + +| | macOS (root 13) | Windows (root 14) | GNOME (fallback 16) | KDE (fallback 16) | +|---|---|---|---|---| +| control_height | 22.75 | 24.5 | 28 | 28 | +| input_height | 24 (stated) | 32 (stated) | 30 | 30 | +| padding_x | 13 | 14 | 16 | 16 | +| text / icon / indicator | 13 / 13 / 15.9 | 14 / 14 / 17.2 | 16 / 16 / 19.6 | 16 / 16 / 19.6 | +| radius sm / control / overlay | 4 / 6 (stated) / 10 | 4 / 4 (stated) / 10 | 4 / 6 / 10 | 4 / 6 / 10 | +| titlebar | max(22.75, 34 floor) = 34 | 32 (stated) | 34 | 34 | +| switch (1.65×h × h, knob 1.225rem) | 37.5×22.75 | 40.4×24.5 | 46.2×28 | 46.2×28 | + +## Gap analysis + +### Heights — the "looks small" bug + +| Button height | Native | Ours | Delta | +|---|---|---|---| +| Windows | 32 | 24.5 | **−7.5px (−23%)** | +| GNOME | ≈34 | 28 | **−6px (−18%)** | +| KDE | ≈33 | 28 | −5px | +| macOS | 24 | 22.75 | −1.25pt | + +Root cause: native heights are not `k × font`. Windows fixes 32px; GNOME is +font + 10px padding; Breeze is fontHeight + 16px. One rem factor cannot hit +any of them — and the trap is that reading the real Linux fonts (14.67 / +13.33px) without changing the model makes our buttons *smaller* (24.4 / +23.3). The codebase already has the right seam: `SystemTheme.input_height`, +an optional platform-stated value with ratio scaling for the other classes. +`control_height` needs the same treatment. Our stated `input_height` values +(macOS 24, Windows 32) are already exactly right — the derived button height +next to them is what is off. + +Text compounds it on Linux: the GNOME/KDE adapters never state +`root_font_size`, so the 16px fallback makes labels oversized (16 vs 14.67 / +13.33) while controls read undersized. + +### Radii + +| Radius | macOS | Windows | GNOME | KDE | +|---|---|---|---|---| +| control | 6 ~ bezel ≈6 | 4 ✓ | **6 vs 9** | 6 vs 5 ~ | +| overlay (menu/popover/dialog) | 10 ~ (unpublished, ≈10–12) | **10 vs 8** | **10 vs 15** | **10 vs 5** | +| sm (checkbox) | 4 ~ | 4 ~ | 4 vs 6 | 4 ✓ | + +Only `control` is platform-overridable (`resolve`); `Radius::default` +freezes sm/overlay/full. GNOME's whole ladder sits 3–5px above ours; KDE's +overlays sit 5px below. + +### Borders — the "feels weird" bug + +Our current policy: `border_1()` on secondary button, input, select, +checkbox, radio, menu, popover, dialog, combobox popup, command palette, +toast, tooltip, alert, even slider/switch knobs. Primary button is +borderless. + +| Element | macOS | GNOME | Windows | KDE | Ours | +|---|---|---|---|---|---| +| secondary button | fill, **no border** | fill, **no border** | 1px elevation border | 1px border | 1px — ✗mac/GNOME | +| primary button | fill, no border | fill, no border | **accent fill + border** | accent fill + border | no border — ✗Win/KDE | +| input | hairline bezel | **borderless**, 2px accent outline on focus | 1px border → accent on focus | 1px border → focus color | 1px always | +| menu / popover | **no border** (shadow/material) | no border (shadow + 1px black@5% ring) | framed flyout | 1px frame | 1px | +| separator roles | hairline ✓ | hairline ✓ | ✓ | ✓ | ✓ | + +Half-right on Windows/KDE, systematically wrong on macOS/GNOME — and even on +Windows/KDE our primary button drops a border the natives keep. Our +high-contrast mode sharpening borders toward the foreground is exactly +GNOME's model (borders as a high-contrast affordance); it just never +switches off at normal contrast. + +### Spacing — structural, not numeric + +No spacing tokens exist in `Theme`; components hardcode a fixed 4px grid: +menu `p_1` + items `px_2 py_1`, popover `mt_1 min_w_32`, menu `min_w_56`, +toast/tooltip `px_3`/`px_2 py_1`, button `gap_1p5`, hardcoded +`max_w(480)`/`max_w(360)`. Native rhythms: GNOME = 6px base (menu padding +12px, item min-height 32px — our GNOME menu rows ≈24px), KDE = 4/6/8, +Windows and macOS ≈ 4px grid (we happen to match those two). Menu/popover +density is frozen at a macOS-ish tuning. + +### Per-component calibration + +- **Switch**: macOS native 54×24 (ours 37.5 wide — too narrow); Windows + 40×20 with a 12px knob (ours 24.5 tall, 17px knob — too big); KDE 36×18 + (ours 46×28); only GNOME ≈ matches, by accident. +- **Checkbox**: Windows/GNOME/KDE ≈20 (ours 17.2–19.6); macOS 16 ✓. +- **Icons**: macOS ≈16 (ours 13), Windows 16 (ours 14); GNOME/KDE 16 ✓. +- **Titlebar**: measured 32 on this macOS build vs our 34 floor (fine-ish; + the floor silently drifted past older macOS's 28); GNOME headerbar 47 vs + 34 — a visible −13px; KDE ≈44 vs 34; Windows 32 stated ✓. + +## What is already right + +- The `input_height` stated-value seam — the pattern to generalize. +- Windows `ControlCornerRadius` 4; stated titlebar 32. +- Body text on macOS (13) and Windows (14); scrollbar autohide mapping; + per-platform elevation stacks; high-contrast border sharpening mirrors + GNOME's model. +- GNOME accent via portal/gsettings, KDE `DecorationFocus`/`DecorationHover` + role mapping. + +## Recommended changes, in priority order + +1. **State `control_height` per platform** — add `control_height: + Option` beside `input_height` on `SystemTheme`, reuse the + ratio-scaling pattern: macOS 24 (measured), Windows 32, GNOME ≈34, KDE + ≈33. Fixes the smallness everywhere in one change. +2. **State `root_font_size` from Linux font settings** — GNOME + `font-name` (11pt ⇒ 14.67px), KDE `kdeglobals [General] font=` (10pt ⇒ + 13.33px); the parsing seams already exist for colors. +3. **Make the radius ladder platform-overridable** — stated overlay/sm + radius on `SystemTheme`: GNOME 15/6, Windows 8/4, KDE 5/4, macOS ≈10/4; + keep preset > system precedence. +4. **Per-source border and focus policy** — a theme-level token consumed by + components: secondary button and inputs drop borders on macOS/GNOME + (fill tints + 2px accent focus outline), keep 1px on Windows/KDE; + primary gains a border on Windows/KDE; menus/popovers shadow-only on + macOS/GNOME. +5. **State `titlebar_height` per platform** — GNOME 47, KDE ≈44; macOS + queried rather than a drifted floor (32 on current builds, 28 older); + Windows stays stated. +6. **Add a minimal spacing scale to `Theme`** driven by a per-platform base + unit (GNOME 6, KDE 4/6/8, others 4); route menu/popover paddings, menu + item min-height (GNOME 32), and the hardcoded `min_w_`/`max_w` values + through it. +7. **Calibrate switches and icons per platform** — macOS 54×24, Windows + 40×20 with 12px knob, KDE 36×18; icon 16 on macOS/Windows. + +## Source confidence notes + +Everything in the GNOME/Windows/KDE tables is quoted from the cited +stylesheets, sources, or Learn pages (published). KDE button/line-edit/title +heights are formulas from source constants (derived). Windows TextBox height +token and the focus-stroke pixel spec live in the OS `generic.xaml` and were +not retrievable — low confidence, commonly cited as 32. Apple publishes no +macOS control metrics; all macOS control numbers here are AppKit +measurements from 2026-11 on this machine and may shift across macOS +releases (titlebar already differs from older builds). -- 2.54.0 From d696b1e707e1b42ec13ce2411a72ff2105c1aed3 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sat, 7 Nov 2026 14:13:15 +0700 Subject: [PATCH 02/14] feat: state platform metrics and chrome in theme --- crates/core/tests/control_sizes.rs | 13 +-- crates/theme/src/definition.rs | 6 +- crates/theme/src/lib.rs | 137 ++++++++++++++++++++++--- crates/theme/src/resolve.rs | 155 ++++++++++++++++++++++++++++- crates/theme/src/source/kde.rs | 64 +++++++++++- crates/theme/src/source/linux.rs | 137 +++++++++++++++++++------ crates/theme/src/source/macos.rs | 19 +++- crates/theme/src/source/omarchy.rs | 28 +++++- crates/theme/src/source/windows.rs | 17 ++++ crates/theme/src/system.rs | 34 ++++++- docs/CHECKLIST.md | 25 +++-- docs/platform-metrics-audit.md | 12 ++- 12 files changed, 574 insertions(+), 73 deletions(-) diff --git a/crates/core/tests/control_sizes.rs b/crates/core/tests/control_sizes.rs index 792693b..e83662b 100644 --- a/crates/core/tests/control_sizes.rs +++ b/crates/core/tests/control_sizes.rs @@ -36,10 +36,11 @@ impl Render for Harness { } } -/// A medium input is exactly `input_height`, and sits taller than the -/// medium button: native text fields are a touch taller than push buttons. +/// The medium input renders at the theme's stated field height and the +/// medium button at the stated control height; the two match wherever the +/// platform states equal values (macOS, Windows, GNOME). #[gpui::test] -fn medium_input_is_taller_than_the_button(cx: &mut TestAppContext) { +fn controls_render_at_the_stated_heights(cx: &mut TestAppContext) { cx.update(Theme::init); let state = cx.update(|cx| InputState::new_with_text("", cx)); let state_for_window = state.clone(); @@ -58,9 +59,9 @@ fn medium_input_is_taller_than_the_button(cx: &mut TestAppContext) { .debug_bounds("input-wrapper") .expect("input rendered"); - // The input's height is the theme table's (platform-stated) field height; - // both bounds are snapped to device pixels, so allow half a pixel. + // Both bounds are snapped to device pixels, so allow half a pixel. let input_height = cx.update(|cx| cx.theme().sizes.medium.input_height); - assert!(input.size.height > button.size.height); + let button_height = cx.update(|cx| cx.theme().sizes.medium.control_height); assert!((input.size.height - input_height).abs() <= gpui::px(0.5)); + assert!((button.size.height - button_height).abs() <= gpui::px(0.5)); } diff --git a/crates/theme/src/definition.rs b/crates/theme/src/definition.rs index 8bb8f4e..d697acd 100644 --- a/crates/theme/src/definition.rs +++ b/crates/theme/src/definition.rs @@ -11,6 +11,8 @@ use crate::ThemeVariant; pub struct ThemeDefinition { pub mode: Option, pub control_radius: Option, + pub overlay_radius: Option, + pub sm_radius: Option, pub accent: Option, pub accent_foreground: Option, pub background: Option, @@ -221,13 +223,15 @@ mod tests { #[test] fn parses_a_full_definition() { let def = ThemeDefinition::parse_toml( - "mode = \"dark\"\naccent = \"#7aa2f7\"\nbackground = \"#1a1b26\"\ninfo = \"#38bdf8\"\n", + "mode = \"dark\"\naccent = \"#7aa2f7\"\nbackground = \"#1a1b26\"\ninfo = \"#38bdf8\"\noverlay_radius = 15.0\nsm_radius = 6.0\n", ) .unwrap(); assert_eq!(def.mode, Some(crate::ThemeVariant::Dark)); assert!(def.accent().is_some()); assert!(def.info().is_some()); assert!(def.foreground().is_none()); + assert_eq!(def.overlay_radius, Some(15.0)); + assert_eq!(def.sm_radius, Some(6.0)); } #[test] diff --git a/crates/theme/src/lib.rs b/crates/theme/src/lib.rs index 3f84858..d4d69fa 100644 --- a/crates/theme/src/lib.rs +++ b/crates/theme/src/lib.rs @@ -48,6 +48,9 @@ pub struct Theme { pub colors: ColorTokens, pub ramps: Ramps, pub radius: Radius, + /// Per-platform border and focus chrome. + pub borders: Borders, + pub spacing: Spacing, pub elevations: Elevations, /// Overlay transition timing; gpui's animation primitives do the rest. pub motion: Motion, @@ -88,6 +91,59 @@ impl Default for Radius { } } +/// Which surfaces keep a 1px border, per the platform's chrome: Windows and +/// KDE frame their controls, macOS and GNOME paint fills. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct Borders { + /// Filled actions keep a border (Windows, KDE). + pub buttons: bool, + /// Text fields and selects keep a border (Windows, KDE; the macOS + /// hairline bezel). + pub fields: bool, + /// Floating layers keep a border (Windows, KDE). + pub overlays: bool, + /// Keyboard focus paints an accent ring around the control (macOS, + /// GNOME); otherwise focus re-colors the border (Windows, KDE). + pub focus_ring: bool, +} + +impl Borders { + pub(crate) fn for_platform(source: SourceKind) -> Self { + match source { + SourceKind::Windows | SourceKind::Kde => Self { + buttons: true, + fields: true, + overlays: true, + focus_ring: false, + }, + SourceKind::Macos => Self { + buttons: false, + fields: true, + overlays: false, + focus_ring: true, + }, + SourceKind::Omarchy | SourceKind::Gnome => Self { + buttons: false, + fields: false, + overlays: false, + focus_ring: true, + }, + SourceKind::Fallback => Self { + buttons: true, + fields: true, + overlays: true, + focus_ring: false, + }, + } + } +} + +/// The spacing rhythm of the platform: paddings and gaps step by this unit. +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct Spacing { + pub unit: Pixels, +} + /// One layer of a floating panel's drop shadow. #[derive(Clone, Copy, Debug, PartialEq)] pub struct Shadow { @@ -289,6 +345,12 @@ pub struct ControlMetrics { pub plate_inset: Pixels, /// The band height of multi-segment strips. pub strip_height: Pixels, + /// The height of one menu row. + pub menu_item_height: Pixels, + /// Switch track height, width, and knob size. + pub switch_height: Pixels, + pub switch_width: Pixels, + pub switch_knob: Pixels, } /// The control metrics for every size class. @@ -309,6 +371,7 @@ impl SizeTable { avatar: f32, strip: f32, inset: f32, + floor_icon: bool, } let small = ClassFactors { @@ -319,6 +382,7 @@ impl SizeTable { avatar: 1.5, strip: 1.5, inset: 0.125, + floor_icon: false, }; let medium = ClassFactors { @@ -329,6 +393,7 @@ impl SizeTable { avatar: 2.0, strip: 2.0, inset: 0.25, + floor_icon: true, }; let large = ClassFactors { @@ -339,21 +404,36 @@ impl SizeTable { avatar: 3.0, strip: 3.0, inset: 0.375, + floor_icon: true, }; - let metrics = |f: &ClassFactors| ControlMetrics { - control_height: rem_size * f.height, - input_height: rem_size * (f.height + 0.125), - padding_x: rem_size * f.padding, - text_size: rem_size * f.text, - icon_size: rem_size * f.icon, - dense_icon_size: rem_size * f.icon * 0.75, - indicator_size: rem_size * f.height * 0.7, - avatar_size: rem_size * f.avatar, - caption_size: rem_size * f.text, - dense_padding_x: rem_size * f.padding * 0.75, - plate_inset: rem_size * f.inset, - strip_height: rem_size * f.strip, + let metrics = |f: &ClassFactors| { + // Standard controls carry 16px glyphs on every platform, fixed + // against the root font the way the platforms fix them. + let icon_size = if f.floor_icon { + (rem_size * f.icon).max(px(16.)) + } else { + rem_size * f.icon + }; + let switch_height = rem_size * f.height * 0.75; + ControlMetrics { + control_height: rem_size * f.height, + input_height: rem_size * (f.height + 0.125), + padding_x: rem_size * f.padding, + text_size: rem_size * f.text, + icon_size, + dense_icon_size: icon_size * 0.75, + indicator_size: rem_size * f.height * 0.7, + avatar_size: rem_size * f.avatar, + caption_size: rem_size * f.text, + dense_padding_x: rem_size * f.padding * 0.75, + plate_inset: rem_size * f.inset, + strip_height: rem_size * f.strip, + menu_item_height: rem_size * f.height * 0.85, + switch_height, + switch_width: switch_height * 2.25, + switch_knob: switch_height * 0.85, + } }; let mut table = Self { @@ -383,6 +463,35 @@ impl SizeTable { self.medium.input_height = medium_input_height; self.large.input_height *= ratio; } + + /// Sets the medium class's control height to the platform's stated value. + pub fn set_control_height(&mut self, medium_control_height: Pixels) { + let ratio = medium_control_height / self.medium.control_height; + self.small.control_height *= ratio; + self.medium.control_height = medium_control_height; + self.large.control_height *= ratio; + } + + /// Sets the medium class's menu row height to the platform's stated value. + pub fn set_menu_item_height(&mut self, medium_menu_item_height: Pixels) { + let ratio = medium_menu_item_height / self.medium.menu_item_height; + self.small.menu_item_height *= ratio; + self.medium.menu_item_height = medium_menu_item_height; + self.large.menu_item_height *= ratio; + } + + /// Sets the medium class's switch dimensions to the platform's stated values. + pub fn set_switch_metrics(&mut self, track_height: Pixels, track_width: Pixels, knob: Pixels) { + let ratio = track_height / self.medium.switch_height; + for metrics in [&mut self.small, &mut self.large] { + metrics.switch_height *= ratio; + metrics.switch_width *= ratio; + metrics.switch_knob *= ratio; + } + self.medium.switch_height = track_height; + self.medium.switch_width = track_width; + self.medium.switch_knob = knob; + } } /// Reads the resolved theme global from any app context. @@ -408,6 +517,8 @@ impl Theme { colors: resolved.colors, ramps: resolved.ramps, radius: resolved.radius, + borders: resolved.borders, + spacing: resolved.spacing, elevations: resolved.elevations, motion: resolved.motion, scrollbar: resolved.scrollbar, diff --git a/crates/theme/src/resolve.rs b/crates/theme/src/resolve.rs index 725824a..7155736 100644 --- a/crates/theme/src/resolve.rs +++ b/crates/theme/src/resolve.rs @@ -1,7 +1,9 @@ use crate::definition::ThemeDefinition; use crate::ramps::{AccentShades, Oklch, contrast_foreground, derive_shades, mix}; use crate::system::SystemTheme; -use crate::{ColorTokens, Elevations, Motion, Radius, Scrollbar, SizeTable, ThemeVariant}; +use crate::{ + Borders, ColorTokens, Elevations, Motion, Radius, Scrollbar, SizeTable, Spacing, ThemeVariant, +}; use gpui::px; /// The floor of a custom titlebar's height, in px at 100% scaling: the height @@ -26,6 +28,8 @@ pub struct ResolvedTheme { pub colors: ColorTokens, pub ramps: Ramps, pub radius: Radius, + pub borders: Borders, + pub spacing: Spacing, pub elevations: Elevations, pub motion: Motion, pub scrollbar: Scrollbar, @@ -198,18 +202,35 @@ pub(crate) fn resolve( // // Fallback: 16 (the web default gpui also uses for rems). let root_font_size = system.root_font_size.unwrap_or(px(16.)); - let mut sizes = SizeTable::with_root_font_size(root_font_size); + + if let Some(control_height) = system.control_height { + sizes.set_control_height(control_height); + } + if let Some(input_height) = system.input_height { sizes.set_input_height(input_height); } + if let Some(switch_height) = system.switch_height { + sizes.set_switch_metrics( + switch_height, + system.switch_width.unwrap_or(switch_height * 2.25), + system.switch_knob.unwrap_or(switch_height * 0.85), + ); + } + + if let Some(menu_item_height) = system.menu_item_height { + sizes.set_menu_item_height(menu_item_height); + } + // A platform that states its titlebar height wins. let titlebar_height = system .titlebar_height .unwrap_or_else(|| sizes.medium.control_height.max(px(TITLEBAR_HEIGHT_MIN_PX))); let mut radius = Radius::default(); + if let Some(control) = preset .and_then(|def| def.control_radius) .map(px) @@ -218,6 +239,22 @@ pub(crate) fn resolve( radius.control = control; } + if let Some(overlay) = preset + .and_then(|def| def.overlay_radius) + .map(px) + .or(system.overlay_radius) + { + radius.overlay = overlay; + } + + if let Some(sm) = preset + .and_then(|def| def.sm_radius) + .map(px) + .or(system.sm_radius) + { + radius.sm = sm; + } + ResolvedTheme { variant, ramps: Ramps { @@ -238,6 +275,10 @@ pub(crate) fn resolve( }, colors, radius, + borders: Borders::for_platform(system.source), + spacing: Spacing { + unit: system.source.spacing_unit(), + }, elevations: Elevations::for_platform(system.source, variant), motion: Motion::default(), scrollbar: Scrollbar { @@ -283,7 +324,14 @@ mod tests { accent, palette, control_radius: None, + overlay_radius: None, + sm_radius: None, + control_height: None, input_height: None, + menu_item_height: None, + switch_height: None, + switch_width: None, + switch_knob: None, titlebar_height: None, root_font_size: None, accessibility: AccessibilityPreferences::default(), @@ -308,7 +356,10 @@ mod tests { fn empty_system_resolves_to_the_neutral_fallback() { for variant in [ThemeVariant::Light, ThemeVariant::Dark] { let resolved = resolve( - &system(variant, PartialPalette::default(), None), + &SystemTheme { + source: SourceKind::Fallback, + ..system(variant, PartialPalette::default(), None) + }, None, None, ); @@ -333,6 +384,21 @@ mod tests { assert_eq!(resolved.sizes.medium.dense_padding_x, px(16. * 0.75)); assert_eq!(resolved.sizes.medium.plate_inset, px(4.)); assert_eq!(resolved.sizes.medium.strip_height, px(32.)); + assert_eq!(resolved.sizes.medium.icon_size, px(16.)); + assert_eq!(resolved.sizes.medium.menu_item_height, px(28. * 0.85)); + assert_eq!(resolved.sizes.medium.switch_height, px(21.)); + assert_eq!(resolved.sizes.medium.switch_width, px(21. * 2.25)); + assert_eq!(resolved.sizes.medium.switch_knob, px(21. * 0.85)); + assert_eq!( + resolved.borders, + Borders { + buttons: true, + fields: true, + overlays: true, + focus_ring: false, + } + ); + assert_eq!(resolved.spacing.unit, px(4.)); assert_eq!(resolved.titlebar_height, px(34.)); } } @@ -396,6 +462,89 @@ mod tests { assert!((resolved.sizes.small.input_height - expected_small).abs() < px(0.01)); } + #[test] + fn a_stated_control_height_overrides_the_medium_class_and_scales() { + let mut stated = system(ThemeVariant::Light, PartialPalette::default(), None); + stated.control_height = Some(px(32.)); + let resolved = resolve(&stated, None, None); + assert_eq!(resolved.sizes.medium.control_height, px(32.)); + + let derived = SizeTable::with_root_font_size(px(16.)); + let scale = px(32.) / derived.medium.control_height; + assert!( + (resolved.sizes.small.control_height - derived.small.control_height * scale).abs() + < px(0.01) + ); + assert!( + (resolved.sizes.large.control_height - derived.large.control_height * scale).abs() + < px(0.01) + ); + } + + #[test] + fn stated_switch_and_menu_metrics_override_the_derived_ones() { + let mut stated = system(ThemeVariant::Light, PartialPalette::default(), None); + stated.switch_height = Some(px(20.)); + stated.switch_width = Some(px(40.)); + stated.switch_knob = Some(px(12.)); + stated.menu_item_height = Some(px(33.)); + let resolved = resolve(&stated, None, None); + + assert_eq!(resolved.sizes.medium.switch_height, px(20.)); + assert_eq!(resolved.sizes.medium.switch_width, px(40.)); + assert_eq!(resolved.sizes.medium.switch_knob, px(12.)); + assert_eq!(resolved.sizes.medium.menu_item_height, px(33.)); + } + + #[test] + fn the_radius_ladder_follows_stated_values_with_preset_priority() { + let mut stated = system(ThemeVariant::Light, PartialPalette::default(), None); + stated.control_radius = Some(px(9.)); + stated.overlay_radius = Some(px(15.)); + stated.sm_radius = Some(px(6.)); + let resolved = resolve(&stated, None, None); + assert_eq!(resolved.radius.control, px(9.)); + assert_eq!(resolved.radius.overlay, px(15.)); + assert_eq!(resolved.radius.sm, px(6.)); + + let def = ThemeDefinition::parse_toml("overlay_radius = 12.0\n").unwrap(); + let overridden = resolve(&stated, Some(&def), None); + assert_eq!(overridden.radius.overlay, px(12.)); + assert_eq!(overridden.radius.sm, px(6.)); + } + + #[test] + fn borders_and_spacing_follow_the_platform_source() { + let chrome = |source| { + let mut stated = system(ThemeVariant::Light, PartialPalette::default(), None); + stated.source = source; + let resolved = resolve(&stated, None, None); + (resolved.borders, resolved.spacing.unit) + }; + + let (borders, unit) = chrome(SourceKind::Windows); + assert!(borders.buttons && borders.fields && borders.overlays && !borders.focus_ring); + assert_eq!(unit, px(4.)); + + let (borders, unit) = chrome(SourceKind::Kde); + assert!(borders.buttons && borders.fields && borders.overlays && !borders.focus_ring); + assert_eq!(unit, px(4.)); + + let (borders, unit) = chrome(SourceKind::Macos); + assert!(!borders.buttons && borders.fields && !borders.overlays && borders.focus_ring); + assert_eq!(unit, px(4.)); + + for source in [SourceKind::Gnome, SourceKind::Omarchy] { + let (borders, unit) = chrome(source); + assert!(!borders.buttons && !borders.fields && !borders.overlays && borders.focus_ring); + assert_eq!(unit, px(6.)); + } + + let (borders, unit) = chrome(SourceKind::Fallback); + assert!(borders.buttons && borders.fields && borders.overlays && !borders.focus_ring); + assert_eq!(unit, px(4.)); + } + #[test] fn overlay_is_translucent_and_overridable() { let light = resolve( diff --git a/crates/theme/src/source/kde.rs b/crates/theme/src/source/kde.rs index 48720ab..e9a6acd 100644 --- a/crates/theme/src/source/kde.rs +++ b/crates/theme/src/source/kde.rs @@ -4,7 +4,7 @@ use std::collections::HashMap; -use gpui::Hsla; +use gpui::{Hsla, Pixels, px}; use crate::system::{PartialPalette, SourceKind, SystemTheme}; @@ -14,6 +14,9 @@ pub(crate) struct KdeScheme { pub(crate) accent: Option, pub(crate) background: Option, pub(crate) palette: PartialPalette, + /// The root font size parsed from `[General] font=` (family, pointSize, + /// pixelSize, ...). + pub(crate) root_font_size: Option, } impl KdeScheme { @@ -54,6 +57,7 @@ impl KdeScheme { accent, background, palette, + root_font_size: root_font_size(groups.get("General")), } } @@ -77,10 +81,17 @@ impl KdeScheme { .unwrap_or(crate::ThemeVariant::Light), accent: self.accent.or(portal_accent), palette: self.palette, - control_radius: None, - input_height: None, - titlebar_height: None, - root_font_size: None, + control_radius: Some(px(5.)), + overlay_radius: Some(px(5.)), + sm_radius: None, + control_height: Some(px(33.)), + input_height: Some(px(29.)), + menu_item_height: Some(px(29.)), + switch_height: Some(px(18.)), + switch_width: Some(px(36.)), + switch_knob: Some(px(18.)), + titlebar_height: Some(px(44.)), + root_font_size: Some(self.root_font_size.unwrap_or(px(13.33))), accessibility, source: SourceKind::Kde, } @@ -95,6 +106,22 @@ fn variant_by_lightness(background: Hsla) -> crate::ThemeVariant { } } +/// The root font size of a `[General]` group's `font=` QFont string. +fn root_font_size(group: Option<&HashMap<&str, &str>>) -> Option { + let fields: Vec<&str> = group?.get("font")?.split(',').map(str::trim).collect(); + let points = fields.get(1)?.parse::().ok()?; + let pixels = fields + .get(2) + .and_then(|size| size.parse::().ok()) + .unwrap_or(-1.0); + let size = if pixels > 0.0 { + pixels + } else { + points * 96.0 / 72.0 + }; + (size > 0.0).then(|| px(size)) +} + fn parse_groups(text: &str) -> HashMap<&str, HashMap<&str, &str>> { let mut groups: HashMap<&str, HashMap<&str, &str>> = HashMap::new(); let mut section: Option<&str> = None; @@ -305,4 +332,31 @@ activeBackground=39,44,49 let system = scheme.into_system(Default::default(), None, Some(crate::ThemeVariant::Light)); assert_eq!(system.variant, crate::ThemeVariant::Light); } + + #[test] + fn the_scheme_states_breeze_metrics_and_the_default_font() { + let system = KdeScheme::parse(BREEZE_DARK).into_system(Default::default(), None, None); + + assert_eq!(system.control_height, Some(px(33.))); + assert_eq!(system.input_height, Some(px(29.))); + assert_eq!(system.menu_item_height, Some(px(29.))); + assert_eq!(system.control_radius, Some(px(5.))); + assert_eq!(system.overlay_radius, Some(px(5.))); + assert_eq!(system.switch_height, Some(px(18.))); + assert_eq!(system.switch_width, Some(px(36.))); + assert_eq!(system.switch_knob, Some(px(18.))); + assert_eq!(system.titlebar_height, Some(px(44.))); + assert!((system.root_font_size.unwrap() - px(13.33)).abs() < px(0.01)); + } + + #[test] + fn the_stated_font_beats_the_default() { + let text = "[General]\nfont=Noto Sans,10,-1,5,50,0,0,0,0,0\n"; + let system = KdeScheme::parse(text).into_system(Default::default(), None, None); + assert!((system.root_font_size.unwrap() - px(10.0 * 96.0 / 72.0)).abs() < px(0.01)); + + let text = "[General]\nfont=Adwaita Sans,-1,12,5,50,0,0,0,0,0\n"; + let system = KdeScheme::parse(text).into_system(Default::default(), None, None); + assert_eq!(system.root_font_size, Some(px(12.))); + } } diff --git a/crates/theme/src/source/linux.rs b/crates/theme/src/source/linux.rs index 5e7f9d5..c6acece 100644 --- a/crates/theme/src/source/linux.rs +++ b/crates/theme/src/source/linux.rs @@ -5,7 +5,7 @@ use std::path::{Path, PathBuf}; -use gpui::Hsla; +use gpui::{Hsla, Pixels}; use notify::{RecursiveMode, Watcher}; use super::ThemeSource; @@ -55,11 +55,12 @@ fn detect(home: &Path, portal_reachable: bool, gsettings_reachable: bool) -> Bac } } -/// What `gsettings` states for GNOME sessions without a portal. +/// What `gsettings` states for GNOME sessions. #[derive(Clone, Copy, Debug, Default, PartialEq)] struct GSettingsSnapshot { variant: Option, accent: Option, + root_font_size: Option, } fn gsettings_value(args: &[&str]) -> Option { @@ -97,23 +98,31 @@ fn read_gsettings() -> Option { "prefer-light" => Some(ThemeVariant::Light), _ => None, }); + let accent = gsettings_value(&["get", "org.gnome.desktop.interface", "accent-color"]) .and_then(|value| accent_rgb(value.trim().trim_matches('\''))); - if variant.is_none() && accent.is_none() { + let root_font_size = gsettings_value(&["get", "org.gnome.desktop.interface", "font-name"]) + .and_then(|value| font_root_px(value.trim().trim_matches('\''))); + + if variant.is_none() && accent.is_none() && root_font_size.is_none() { return None; } - Some(GSettingsSnapshot { variant, accent }) + Some(GSettingsSnapshot { + variant, + accent, + root_font_size, + }) +} + +fn font_root_px(name: &str) -> Option { + let points = name.split_whitespace().last()?.parse::().ok()?; + (points > 0.0).then(|| gpui::px(points * 96.0 / 72.0)) } pub(crate) fn platform_source() -> Box { let portal = portal::io::PortalClient::connect(); - - let gsettings = if portal.is_none() { - read_gsettings() - } else { - None - }; + let gsettings = read_gsettings(); let backend = home_dir() .map(|home| detect(&home, portal.is_some(), gsettings.is_some())) @@ -129,6 +138,36 @@ pub(crate) fn platform_source() -> Box { } impl LinuxSource { + fn gnome_snapshot( + &self, + variant: ThemeVariant, + accent: Option, + accessibility: crate::system::AccessibilityPreferences, + ) -> SystemTheme { + SystemTheme { + variant, + accent, + palette: PartialPalette::default(), + control_radius: Some(gpui::px(9.)), + overlay_radius: Some(gpui::px(15.)), + sm_radius: Some(gpui::px(6.)), + control_height: Some(gpui::px(34.)), + input_height: Some(gpui::px(34.)), + menu_item_height: Some(gpui::px(32.)), + switch_height: Some(gpui::px(26.)), + switch_width: Some(gpui::px(46.)), + switch_knob: Some(gpui::px(20.)), + titlebar_height: Some(gpui::px(47.)), + root_font_size: Some( + self.gsettings + .and_then(|gsettings| gsettings.root_font_size) + .unwrap_or(gpui::px(14.67)), + ), + accessibility, + source: SourceKind::Gnome, + } + } + fn portal_snapshot( &self, variant: ThemeVariant, @@ -136,31 +175,21 @@ impl LinuxSource { accessibility: crate::system::AccessibilityPreferences, ) -> SystemTheme { if let Some(appearance) = portal_appearance { - return SystemTheme { - variant: appearance.variant(variant), - accent: appearance.accent, - palette: PartialPalette::default(), - control_radius: None, - input_height: None, - titlebar_height: None, - root_font_size: None, + return self.gnome_snapshot( + appearance.variant(variant), + appearance.accent, accessibility, - source: SourceKind::Gnome, - }; + ); } + if let Some(gsettings) = self.gsettings { - return SystemTheme { - variant: gsettings.variant.unwrap_or(variant), - accent: gsettings.accent, - palette: PartialPalette::default(), - control_radius: None, - input_height: None, - titlebar_height: None, - root_font_size: None, + return self.gnome_snapshot( + gsettings.variant.unwrap_or(variant), + gsettings.accent, accessibility, - source: SourceKind::Gnome, - }; + ); } + SystemTheme::fallback(variant) } } @@ -289,4 +318,52 @@ mod tests { assert_eq!(detect(&home, true, false), Backend::Kde); std::fs::remove_dir_all(&home).expect("cleanup"); } + + #[test] + fn the_gsettings_font_name_states_the_root_font_size() { + assert_eq!( + font_root_px("Adwaita Sans 11"), + Some(gpui::px(11.0 * 96.0 / 72.0)) + ); + assert_eq!( + font_root_px("Noto Sans 10"), + Some(gpui::px(10.0 * 96.0 / 72.0)) + ); + assert_eq!( + font_root_px("Cantarell Bold 12.5"), + Some(gpui::px(12.5 * 96.0 / 72.0)) + ); + assert_eq!(font_root_px("Adwaita Sans"), None); + assert_eq!(font_root_px("Sans 0"), None); + } + + #[test] + fn the_gnome_snapshot_states_the_libadwaita_metrics() { + let source = LinuxSource { + backend: Backend::Portal, + portal: None, + gsettings: None, + file_watcher: None, + _signal: None, + }; + let system = source.gnome_snapshot( + ThemeVariant::Light, + None, + crate::system::AccessibilityPreferences::default(), + ); + + assert_eq!(system.source, SourceKind::Gnome); + assert_eq!(system.control_height, Some(gpui::px(34.))); + assert_eq!(system.input_height, Some(gpui::px(34.))); + assert_eq!(system.menu_item_height, Some(gpui::px(32.))); + assert_eq!(system.control_radius, Some(gpui::px(9.))); + assert_eq!(system.overlay_radius, Some(gpui::px(15.))); + assert_eq!(system.sm_radius, Some(gpui::px(6.))); + assert_eq!(system.switch_height, Some(gpui::px(26.))); + assert_eq!(system.switch_width, Some(gpui::px(46.))); + assert_eq!(system.switch_knob, Some(gpui::px(20.))); + assert_eq!(system.titlebar_height, Some(gpui::px(47.))); + // The default 11pt font when gsettings states none. + assert_eq!(system.root_font_size, Some(gpui::px(14.67))); + } } diff --git a/crates/theme/src/source/macos.rs b/crates/theme/src/source/macos.rs index 9b4caf1..30e3763 100644 --- a/crates/theme/src/source/macos.rs +++ b/crates/theme/src/source/macos.rs @@ -22,6 +22,16 @@ use crate::system::{AccessibilityPreferences, PartialPalette, SourceKind, System const CONTROL_RADIUS_PTS: f32 = 6.0; /// The rounded-bezel text field's intrinsic height at regular size. const INPUT_HEIGHT_PTS: f32 = 24.0; +/// AppKit's regular push button, measured via `NSButton::fittingSize`. +const CONTROL_HEIGHT_PTS: f32 = 24.0; +/// The standard `NSMenu` row height. +const MENU_ITEM_HEIGHT_PTS: f32 = 20.0; +/// `NSSwitch`, measured via `fittingSize`. +const SWITCH_TRACK_HEIGHT_PTS: f32 = 24.0; +const SWITCH_TRACK_WIDTH_PTS: f32 = 54.0; +const SWITCH_KNOB_PTS: f32 = 20.0; +/// The standard titled window's titlebar, measured via `contentLayoutRect`. +const TITLEBAR_HEIGHT_PTS: f32 = 32.0; /// The HIG's default body text on macOS is 13 pt (system font size 13 at 1x). const ROOT_FONT_SIZE_PTS: f32 = 13.0; @@ -112,8 +122,15 @@ impl ThemeSource for MacosSource { accent: Self::color_to_hsla(&NSColor::controlAccentColor()), palette, control_radius: Some(gpui::px(CONTROL_RADIUS_PTS)), + overlay_radius: None, + sm_radius: None, + control_height: Some(gpui::px(CONTROL_HEIGHT_PTS)), input_height: Some(gpui::px(INPUT_HEIGHT_PTS)), - titlebar_height: None, + menu_item_height: Some(gpui::px(MENU_ITEM_HEIGHT_PTS)), + switch_height: Some(gpui::px(SWITCH_TRACK_HEIGHT_PTS)), + switch_width: Some(gpui::px(SWITCH_TRACK_WIDTH_PTS)), + switch_knob: Some(gpui::px(SWITCH_KNOB_PTS)), + titlebar_height: Some(gpui::px(TITLEBAR_HEIGHT_PTS)), root_font_size: Some(gpui::px(ROOT_FONT_SIZE_PTS)), accessibility, source: SourceKind::Macos, diff --git a/crates/theme/src/source/omarchy.rs b/crates/theme/src/source/omarchy.rs index f411c44..2476a6e 100644 --- a/crates/theme/src/source/omarchy.rs +++ b/crates/theme/src/source/omarchy.rs @@ -60,8 +60,15 @@ impl OmarchyTheme { variant: self.variant, accent: self.accent, palette: self.palette, - control_radius: None, - input_height: None, + control_radius: Some(gpui::px(9.)), + overlay_radius: Some(gpui::px(15.)), + sm_radius: Some(gpui::px(6.)), + control_height: Some(gpui::px(34.)), + input_height: Some(gpui::px(34.)), + menu_item_height: Some(gpui::px(32.)), + switch_height: Some(gpui::px(26.)), + switch_width: Some(gpui::px(46.)), + switch_knob: Some(gpui::px(20.)), titlebar_height: None, root_font_size: None, accessibility, @@ -215,6 +222,23 @@ color15 = \"#635d6f\" assert_eq!(theme.variant, ThemeVariant::Light); } + #[test] + fn omarchy_shares_the_gnome_control_metrics() { + let theme = OmarchyTheme::parse(TOKYO_NIGHT).expect("parses"); + let system = theme.into_system(Default::default()); + + assert_eq!(system.control_height, Some(gpui::px(34.))); + assert_eq!(system.input_height, Some(gpui::px(34.))); + assert_eq!(system.menu_item_height, Some(gpui::px(32.))); + assert_eq!(system.control_radius, Some(gpui::px(9.))); + assert_eq!(system.overlay_radius, Some(gpui::px(15.))); + assert_eq!(system.switch_height, Some(gpui::px(26.))); + assert_eq!(system.switch_width, Some(gpui::px(46.))); + assert_eq!(system.switch_knob, Some(gpui::px(20.))); + assert_eq!(system.titlebar_height, None); + assert_eq!(system.root_font_size, None); + } + #[test] fn non_toml_text_is_rejected() { assert!(OmarchyTheme::parse("not a theme").is_none()); diff --git a/crates/theme/src/source/windows.rs b/crates/theme/src/source/windows.rs index 198d2b8..a2f66aa 100644 --- a/crates/theme/src/source/windows.rs +++ b/crates/theme/src/source/windows.rs @@ -14,8 +14,18 @@ use crate::system::{AccessibilityPreferences, PartialPalette, SourceKind, System /// WinUI 11's `ControlCornerRadius`, in pixels at 100% scaling. const CONTROL_RADIUS_PX: f32 = 4.0; +/// WinUI 11's `OverlayCornerRadius`, in pixels at 100% scaling. +const OVERLAY_RADIUS_PX: f32 = 8.0; /// WinUI 11's default text-box height, in pixels at 100% scaling. const INPUT_HEIGHT_PX: f32 = 32.0; +/// The default button height, in pixels at 100% scaling. +const CONTROL_HEIGHT_PX: f32 = 32.0; +/// The default menu row: item padding 4/5 + a 20px Body line + 2px item margins. +const MENU_ITEM_HEIGHT_PX: f32 = 33.0; +/// ToggleSwitch: a 40x20 track with a 12px resting knob. +const SWITCH_TRACK_HEIGHT_PX: f32 = 20.0; +const SWITCH_TRACK_WIDTH_PX: f32 = 40.0; +const SWITCH_KNOB_PX: f32 = 12.0; /// The window caption's height, in pixels at 100% scaling, as Win32 reports it. const TITLEBAR_HEIGHT_PX: f32 = 32.0; /// WinUI 11's default body text size, in pixels at 100% scaling. @@ -159,7 +169,14 @@ impl ThemeSource for WindowsSource { ..PartialPalette::default() }, control_radius: Some(gpui::px(CONTROL_RADIUS_PX)), + overlay_radius: Some(gpui::px(OVERLAY_RADIUS_PX)), + sm_radius: None, + control_height: Some(gpui::px(CONTROL_HEIGHT_PX)), input_height: Some(gpui::px(INPUT_HEIGHT_PX)), + menu_item_height: Some(gpui::px(MENU_ITEM_HEIGHT_PX)), + switch_height: Some(gpui::px(SWITCH_TRACK_HEIGHT_PX)), + switch_width: Some(gpui::px(SWITCH_TRACK_WIDTH_PX)), + switch_knob: Some(gpui::px(SWITCH_KNOB_PX)), titlebar_height: Some(gpui::px(TITLEBAR_HEIGHT_PX)), root_font_size: Some(gpui::px(ROOT_FONT_SIZE_PX * text_scale as f32)), accessibility: self.accessibility(), diff --git a/crates/theme/src/system.rs b/crates/theme/src/system.rs index 671e170..7be281b 100644 --- a/crates/theme/src/system.rs +++ b/crates/theme/src/system.rs @@ -1,6 +1,6 @@ //! The normalized snapshot a platform adapter produces. -use gpui::{Hsla, Pixels}; +use gpui::{Hsla, Pixels, px}; use crate::ThemeVariant; @@ -43,6 +43,17 @@ impl SourceKind { pub fn scrollbar_autohide(self) -> bool { matches!(self, Self::Macos | Self::Omarchy | Self::Gnome) } + + /// The base spacing unit: + /// + /// - GNOME lays out on a 6px grid + /// - Other platform on 4px grid + pub fn spacing_unit(self) -> Pixels { + match self { + Self::Gnome | Self::Omarchy => px(6.), + _ => px(4.), + } + } } /// A partial palette with optional colors. @@ -83,8 +94,22 @@ pub struct SystemTheme { pub palette: PartialPalette, /// The platform's stated control corner radius, if it has one. pub control_radius: Option, + /// The platform's stated menu/popover radius, if it has one. + pub overlay_radius: Option, + /// The platform's stated checkbox radius, if it has one. + pub sm_radius: Option, + /// The platform's stated control height, if it has one. + pub control_height: Option, /// The platform's stated text-field height, if it has one. pub input_height: Option, + /// The platform's stated menu row height, if it has one. + pub menu_item_height: Option, + /// The platform's stated switch track height, if it has one. + pub switch_height: Option, + /// The platform's stated switch track width, if it has one. + pub switch_width: Option, + /// The platform's stated switch knob size, if it has one. + pub switch_knob: Option, /// The platform's native titlebar height, if it states one. pub titlebar_height: Option, /// The platform's native root font size, if it has one. @@ -100,7 +125,14 @@ impl SystemTheme { accent: None, palette: PartialPalette::default(), control_radius: None, + overlay_radius: None, + sm_radius: None, + control_height: None, input_height: None, + menu_item_height: None, + switch_height: None, + switch_width: None, + switch_knob: None, titlebar_height: None, root_font_size: None, accessibility: AccessibilityPreferences::default(), diff --git a/docs/CHECKLIST.md b/docs/CHECKLIST.md index 3f134d2..907d183 100644 --- a/docs/CHECKLIST.md +++ b/docs/CHECKLIST.md @@ -9,18 +9,20 @@ is the first downstream consumer; its screens drive the order. ## Next +- [ ] **Platform metric adoption** — update components one at a time + through the review gate against the landed theme tokens. The stated + metrics and the radius ladder already flow through the size table, + so each stop covers border + focus chrome (`Theme::borders`), the + spacing unit (`Theme::spacing`), and the component's stated dims: + button (the API template) first, then input, select, menu, popover, + dialog, toast, tooltip, alert, combobox, command palette, switch; + per-platform targets in + [platform-metrics-audit.md](platform-metrics-audit.md). + ## Queue — to follow Gaps surfaced by the signed comparison; ordered by signed's usage. -- [ ] **Platform metric fidelity** — native-sizing pass over the theme and - components: stated `control_height` per platform (macOS 24 measured / - Windows 32 / GNOME ≈34 / KDE ≈33), Linux root font sizes (GNOME 14.67px, - KDE 13.33px), platform overlay/sm radii, per-source border + focus - policy (macOS/GNOME borderless fills, Windows/KDE 1px), titlebar - heights (GNOME 47), a theme spacing scale, switch/icon calibration; - ranked findings in - [platform-metrics-audit.md](platform-metrics-audit.md). - [ ] **Virtual list** — generic virtualized variable-height rows with a scroll handle, promoted from the editor views' pattern over gpui's `list`; signed's PR lists and commit diff drive it. @@ -55,6 +57,13 @@ Gaps surfaced by the signed comparison; ordered by signed's usage. - [x] Platform sources — macOS (NSColor), Windows 11 (UISettings), GNOME (portal), KDE (kdeglobals), Omarchy (colors.toml), neutral fallback; live watchers for each +- [x] Platform metric fidelity — stated control/input/menu-row/switch + metrics and the full radius ladder per platform, Linux root fonts + (GNOME gsettings `font-name`, KDE `kdeglobals font=`), per-source + border + focus chrome (`Theme::borders`), and a per-platform spacing + unit (`Theme::spacing`); findings in + [platform-metrics-audit.md](platform-metrics-audit.md). Components + adopt the tokens one by one (see Next) ### icons (`crates/icons`) diff --git a/docs/platform-metrics-audit.md b/docs/platform-metrics-audit.md index 232b102..5d15b7a 100644 --- a/docs/platform-metrics-audit.md +++ b/docs/platform-metrics-audit.md @@ -229,6 +229,10 @@ density is frozen at a macOS-ish tuning. ## Recommended changes, in priority order +Landed 2026-11 with two deviations: Omarchy is grouped with GNOME (it +styles its apps with adwaita), and macOS's titlebar is stated at the +measured 32pt rather than queried at runtime (28pt on older builds). + 1. **State `control_height` per platform** — add `control_height: Option` beside `input_height` on `SystemTheme`, reuse the ratio-scaling pattern: macOS 24 (measured), Windows 32, GNOME ≈34, KDE @@ -248,9 +252,11 @@ density is frozen at a macOS-ish tuning. queried rather than a drifted floor (32 on current builds, 28 older); Windows stays stated. 6. **Add a minimal spacing scale to `Theme`** driven by a per-platform base - unit (GNOME 6, KDE 4/6/8, others 4); route menu/popover paddings, menu - item min-height (GNOME 32), and the hardcoded `min_w_`/`max_w` values - through it. + unit (GNOME 6, KDE 4/6/8, others 4); route menu/popover paddings and menu + row min-heights through it — GNOME 32 (published), Windows ≈33 (padding + 4/5 + a 20px Body line + 2px item margins), KDE ≈29 (max(font, 20px + floors) + 1 + 2×4); the hardcoded `min_w_`/`max_w` surface values stay + put — they are platform-invariant. 7. **Calibrate switches and icons per platform** — macOS 54×24, Windows 40×20 with 12px knob, KDE 36×18; icon 16 on macOS/Windows. -- 2.54.0 From fd36ada35e6eb9cd7d8af944354636661203f224 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sat, 7 Nov 2026 15:16:25 +0700 Subject: [PATCH 03/14] audit button --- crates/core/src/button.rs | 49 ++++-- crates/core/src/spinner.rs | 10 +- crates/core/tests/button.rs | 23 ++- crates/theme/src/lib.rs | 2 +- docs/CHECKLIST.md | 46 ++++-- docs/platform-metrics-audit.md | 272 --------------------------------- examples/gallery/src/main.rs | 69 +++++++-- 7 files changed, 157 insertions(+), 314 deletions(-) delete mode 100644 docs/platform-metrics-audit.md diff --git a/crates/core/src/button.rs b/crates/core/src/button.rs index 9af3f81..b0b39e4 100644 --- a/crates/core/src/button.rs +++ b/crates/core/src/button.rs @@ -11,6 +11,7 @@ use gpui_native_utils::focus_ring; use std::rc::Rc; use crate::common::{Sizable, h_flex}; +use crate::spinner::Spinner; /// The single activation handler, shared by pointer, Enter, and Space. type ClickHandler = Box; @@ -20,10 +21,10 @@ type TooltipBuilder = Rc AnyView>; /// Which visual treatment a button uses. #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub enum ButtonVariant { - /// Accent-filled; the platform's default action. + /// The platform's default action. #[default] Primary, - /// Neutral and bordered; the everyday companion to `Primary`. + /// The everyday companion to `Primary`. Secondary, /// Unpainted until hovered, for toolbars and dense UI. Ghost, @@ -40,6 +41,7 @@ pub struct Button { variant: ButtonVariant, size: Size, disabled: bool, + loading: bool, tab_stop: bool, debug_selector: Option String>>, a11y_label: Option, @@ -60,6 +62,7 @@ impl Button { variant: ButtonVariant::default(), size: Size::default(), disabled: false, + loading: false, tab_stop: true, debug_selector: None, a11y_label: None, @@ -119,6 +122,12 @@ impl Button { self } + /// Shows the spinner in place of the icon and suppresses activation. + pub fn loading(mut self, loading: bool) -> Self { + self.loading = loading; + self + } + /// Overrides the accessibility label, which defaults to the visible label. pub fn accessibility_label(mut self, label: L) -> Self where @@ -149,11 +158,11 @@ impl RenderOnce for Button { let theme = cx.theme(); let colors = theme.colors; let ramps = theme.ramps; + let borders = theme.borders; let metrics = theme.sizes.resolve(self.size); let on_click = self.on_click; let a11y_label = self.a11y_label.or_else(|| self.label.clone()); - let icon_only = self.label.is_none() && self.icon.is_some(); let (background, foreground, hover, pressed) = match self.variant { ButtonVariant::Primary => ( @@ -183,35 +192,47 @@ impl RenderOnce for Button { }; h_flex() - .id(self.id) + .id(self.id.clone()) .when_some(self.debug_selector, |button, selector| { button.debug_selector(move || selector()) }) .role(Role::Button) .justify_center() - .when(icon_only, |button| button.size(metrics.control_height)) - .when(!icon_only, |button| { - button.h(metrics.control_height).px(metrics.padding_x) + .h(metrics.control_height) + .when(self.icon.is_some(), |button| { + button.px(metrics.dense_padding_x) }) + .when(self.icon.is_none(), |button| button.px(metrics.padding_x)) .line_height(relative(1.0)) - .gap_1p5() + .gap_2() .rounded(theme.radius.control) .text_size(metrics.text_size) - .when(self.variant == ButtonVariant::Secondary, |button| { - button.border_1().border_color(colors.border) - }) + .when( + borders.buttons && self.variant != ButtonVariant::Ghost, + |button| button.border_1().border_color(colors.border), + ) .when_some(background, |button, background| button.bg(background)) .text_color(foreground) .when_some(a11y_label, |button, label| button.aria_label(label)) - .when_some(self.icon, |button, role| { - button.child(icon(role).size(metrics.icon_size).color(foreground)) + .when(self.loading, |button| { + button.child( + Spinner::new((self.id.clone(), "spinner")) + .with_size(self.size) + .variant(crate::spinner::SpinnerVariant::Icon) + .color(foreground), + ) + }) + .when(!self.loading, |button| { + button.when_some(self.icon, |button, role| { + button.child(icon(role).size(metrics.icon_size).color(foreground)) + }) }) .when_some(self.label, |button, label| button.child(label)) .when(self.disabled, |button| button.opacity(0.5)) .when_some(self.tooltip, |button, tooltip| { button.tooltip(move |window, cx| tooltip(window, cx)) }) - .when(!self.disabled, |button| { + .when(!self.disabled && !self.loading, |button| { button .cursor_pointer() .focusable() diff --git a/crates/core/src/spinner.rs b/crates/core/src/spinner.rs index e9c130a..2a0bbee 100644 --- a/crates/core/src/spinner.rs +++ b/crates/core/src/spinner.rs @@ -41,6 +41,7 @@ pub struct Spinner { value: Option, variant: SpinnerVariant, size: Size, + color: Option, a11y_label: Option, } @@ -55,10 +56,17 @@ impl Spinner { value: None, variant: SpinnerVariant::default(), size: Size::default(), + color: None, a11y_label: None, } } + /// Overrides the arc color, for accent-filled surfaces. + pub fn color(mut self, color: Hsla) -> Self { + self.color = Some(color); + self + } + /// Sets the spinner's visual language. pub fn variant(mut self, variant: SpinnerVariant) -> Self { self.variant = variant; @@ -93,7 +101,7 @@ impl RenderOnce for Spinner { let side = theme.sizes.resolve(self.size).icon_size; let stroke = side * RING_STROKE; let track = theme.colors.border; - let accent = theme.ramps.primary.base; + let accent = self.color.unwrap_or(theme.ramps.primary.base); let value = self.value.map(|value| value.clamp(0.0, 1.0)); div() diff --git a/crates/core/tests/button.rs b/crates/core/tests/button.rs index df3c820..e8195c0 100644 --- a/crates/core/tests/button.rs +++ b/crates/core/tests/button.rs @@ -11,6 +11,7 @@ use gpui_native_theme::Theme; struct Harness { clicks: Rc>, disabled: bool, + loading: bool, } impl Render for Harness { @@ -20,14 +21,18 @@ impl Render for Harness { Button::new("test-button") .label("Save") .disabled(self.disabled) + .loading(self.loading) .on_click(move |_, _, _| *clicks.borrow_mut() += 1), ) } } -/// A maximized window whose button stretches across the top 28px, so +/// A maximized window whose button stretches across the top band, so /// `point(px(60.), px(14.))` always lands on it. -fn button_window(disabled: bool) -> (VisualTestContext, AnyWindowHandle, Rc>) { +fn button_window( + disabled: bool, + loading: bool, +) -> (VisualTestContext, AnyWindowHandle, Rc>) { let mut cx = TestAppContext::single(); cx.update(Theme::init); let clicks = Rc::new(RefCell::new(0)); @@ -35,6 +40,7 @@ fn button_window(disabled: bool) -> (VisualTestContext, AnyWindowHandle, Rc` beside `input_height` on `SystemTheme`, reuse the - ratio-scaling pattern: macOS 24 (measured), Windows 32, GNOME ≈34, KDE - ≈33. Fixes the smallness everywhere in one change. -2. **State `root_font_size` from Linux font settings** — GNOME - `font-name` (11pt ⇒ 14.67px), KDE `kdeglobals [General] font=` (10pt ⇒ - 13.33px); the parsing seams already exist for colors. -3. **Make the radius ladder platform-overridable** — stated overlay/sm - radius on `SystemTheme`: GNOME 15/6, Windows 8/4, KDE 5/4, macOS ≈10/4; - keep preset > system precedence. -4. **Per-source border and focus policy** — a theme-level token consumed by - components: secondary button and inputs drop borders on macOS/GNOME - (fill tints + 2px accent focus outline), keep 1px on Windows/KDE; - primary gains a border on Windows/KDE; menus/popovers shadow-only on - macOS/GNOME. -5. **State `titlebar_height` per platform** — GNOME 47, KDE ≈44; macOS - queried rather than a drifted floor (32 on current builds, 28 older); - Windows stays stated. -6. **Add a minimal spacing scale to `Theme`** driven by a per-platform base - unit (GNOME 6, KDE 4/6/8, others 4); route menu/popover paddings and menu - row min-heights through it — GNOME 32 (published), Windows ≈33 (padding - 4/5 + a 20px Body line + 2px item margins), KDE ≈29 (max(font, 20px - floors) + 1 + 2×4); the hardcoded `min_w_`/`max_w` surface values stay - put — they are platform-invariant. -7. **Calibrate switches and icons per platform** — macOS 54×24, Windows - 40×20 with 12px knob, KDE 36×18; icon 16 on macOS/Windows. - -## Source confidence notes - -Everything in the GNOME/Windows/KDE tables is quoted from the cited -stylesheets, sources, or Learn pages (published). KDE button/line-edit/title -heights are formulas from source constants (derived). Windows TextBox height -token and the focus-stroke pixel spec live in the OS `generic.xaml` and were -not retrievable — low confidence, commonly cited as 32. Apple publishes no -macOS control metrics; all macOS control numbers here are AppKit -measurements from 2026-11 on this machine and may shift across macOS -releases (titlebar already differs from older builds). diff --git a/examples/gallery/src/main.rs b/examples/gallery/src/main.rs index 0c592cb..422f550 100644 --- a/examples/gallery/src/main.rs +++ b/examples/gallery/src/main.rs @@ -879,24 +879,75 @@ impl Render for ButtonDemo { button = button.tooltip(Tooltip::text("Removes the selected item")); } button - })) + })), + )) + .child(demo_section( + "icon + label", + colors, + h_flex() + .flex_wrap() + .gap_2() + .children(variants.map(|(name, variant)| { + let view = view.clone(); + let mut button = + Button::new(SharedString::from(format!("button-demo-icon-{name}"))) + .label(button_label(variant)) + .icon(IconRole::Folder) + .variant(variant) + .on_click(move |_, _, cx| { + view.update(cx, |demo, cx| { + demo.clicks += 1; + cx.notify(); + }); + }); + if variant == ButtonVariant::Destructive { + button = button.tooltip(Tooltip::text("Removes the selected item")); + } + button + })), + )) + .child(demo_section( + "icon only", + colors, + h_flex() + .flex_wrap() + .gap_2() + .children(variants.map(|(name, variant)| { + let view = view.clone(); + Button::new(SharedString::from(format!("button-demo-icon-only-{name}"))) + .icon(IconRole::Folder) + .variant(variant) + .accessibility_label("Open the selected file") + .tooltip(Tooltip::text("Opens the selected file")) + .on_click(move |_, _, cx| { + view.update(cx, |demo, cx| { + demo.clicks += 1; + cx.notify(); + }); + }) + })), + )) + .child(demo_section( + "states", + colors, + h_flex() + .flex_wrap() + .gap_2() .child( Button::new("button-demo-disabled") .label("Disabled") .disabled(true), ) .child( - Button::new("button-demo-icon") - .label("Open") - .icon(IconRole::Folder) - .tooltip(Tooltip::text("Opens the selected file")), + Button::new("button-demo-loading") + .label("Saving") + .loading(true), ) .child( - Button::new("button-demo-icon-only") + Button::new("button-demo-loading-icon-only") .icon(IconRole::Folder) - .variant(ButtonVariant::Ghost) - .accessibility_label("Open the selected file") - .tooltip(Tooltip::text("Opens the selected file")), + .accessibility_label("Saving the selected file") + .loading(true), ), )) .child(demo_section( -- 2.54.0 From ffbb732f5fec06d2eabc92f2673604f8184257f1 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sat, 7 Nov 2026 17:00:36 +0700 Subject: [PATCH 04/14] audit input --- crates/core/src/button.rs | 31 +++- crates/core/src/input.rs | 269 +++++++++++++++++++---------- crates/core/tests/control_sizes.rs | 16 +- crates/core/tests/input.rs | 165 +++++++++++++++++- crates/theme/src/lib.rs | 12 +- crates/theme/src/resolve.rs | 22 +-- crates/utils/src/lib.rs | 4 +- docs/CHECKLIST.md | 27 ++- examples/gallery/src/main.rs | 35 +++- 9 files changed, 437 insertions(+), 144 deletions(-) diff --git a/crates/core/src/button.rs b/crates/core/src/button.rs index b0b39e4..619a0d2 100644 --- a/crates/core/src/button.rs +++ b/crates/core/src/button.rs @@ -3,10 +3,11 @@ use gpui::prelude::FluentBuilder as _; use gpui::{ AnyView, App, ClickEvent, ElementId, InteractiveElement as _, IntoElement, ParentElement as _, - RenderOnce, Role, SharedString, StatefulInteractiveElement as _, Styled as _, Window, relative, + RenderOnce, Role, SharedString, StatefulInteractiveElement as _, Styled as _, Window, hsla, + relative, }; use gpui_native_icons::{IconRole, icon}; -use gpui_native_theme::{ActiveTheme as _, Size}; +use gpui_native_theme::{ActiveTheme as _, BORDER_WIDTH, Size}; use gpui_native_utils::focus_ring; use std::rc::Rc; @@ -161,6 +162,7 @@ impl RenderOnce for Button { let borders = theme.borders; let metrics = theme.sizes.resolve(self.size); + let show_border = borders.buttons && self.variant != ButtonVariant::Ghost; let on_click = self.on_click; let a11y_label = self.a11y_label.or_else(|| self.label.clone()); @@ -191,6 +193,19 @@ impl RenderOnce for Button { ), }; + let (border, hover_border, pressed_border) = if show_border { + (colors.border, colors.border, colors.border) + } else { + match background { + Some(fill) => (fill, hover, pressed), + None => ( + hsla(0.0, 0.0, 0.0, 0.0), + hsla(0.0, 0.0, 0.0, 0.0), + hsla(0.0, 0.0, 0.0, 0.0), + ), + } + }; + h_flex() .id(self.id.clone()) .when_some(self.debug_selector, |button, selector| { @@ -198,7 +213,7 @@ impl RenderOnce for Button { }) .role(Role::Button) .justify_center() - .h(metrics.control_height) + .h(metrics.control_height + BORDER_WIDTH * 2.0) .when(self.icon.is_some(), |button| { button.px(metrics.dense_padding_x) }) @@ -207,10 +222,8 @@ impl RenderOnce for Button { .gap_2() .rounded(theme.radius.control) .text_size(metrics.text_size) - .when( - borders.buttons && self.variant != ButtonVariant::Ghost, - |button| button.border_1().border_color(colors.border), - ) + .border_1() + .border_color(border) .when_some(background, |button, background| button.bg(background)) .text_color(foreground) .when_some(a11y_label, |button, label| button.aria_label(label)) @@ -237,8 +250,8 @@ impl RenderOnce for Button { .cursor_pointer() .focusable() .tab_stop(self.tab_stop) - .hover(move |style| style.bg(hover)) - .active(move |style| style.bg(pressed)) + .hover(move |style| style.bg(hover).border_color(hover_border)) + .active(move |style| style.bg(pressed).border_color(pressed_border)) .focus_visible(|style| style.shadow(focus_ring(colors.ring))) .when_some(on_click, |button, on_click| { button.on_click(move |event, window, cx| on_click(event, window, cx)) diff --git a/crates/core/src/input.rs b/crates/core/src/input.rs index 3b4269f..2230852 100644 --- a/crates/core/src/input.rs +++ b/crates/core/src/input.rs @@ -7,15 +7,17 @@ use unicode_segmentation::UnicodeSegmentation; use gpui::prelude::FluentBuilder as _; use gpui::{ - App, AppContext as _, Bounds, ClipboardItem, ContentMask, Context, CursorStyle, DefiniteLength, - DispatchPhase, ElementId, Entity, EntityInputHandler, EventEmitter, Font, HitboxBehavior, - InteractiveElement as _, IntoElement, KeyBinding, KeyDownEvent, MouseButton, MouseDownEvent, - MouseMoveEvent, MouseUpEvent, ParentElement as _, Pixels, RenderOnce, Role, ScrollDelta, - ScrollEvent, SharedString, StatefulInteractiveElement as _, Styled as _, Subscription, Task, - TextAlign, TextRun, UTF16Selection, Window, WrappedLine, actions, canvas, div, fill, point, px, + AnyElement, App, AppContext as _, Bounds, ClipboardItem, ContentMask, Context, CursorStyle, + DefiniteLength, DispatchPhase, ElementId, Entity, EntityInputHandler, EventEmitter, Font, + HitboxBehavior, InteractiveElement as _, IntoElement, KeyBinding, KeyDownEvent, MouseButton, + MouseDownEvent, MouseMoveEvent, MouseUpEvent, ParentElement as _, Pixels, RenderOnce, Role, + ScrollDelta, ScrollEvent, SharedString, StatefulInteractiveElement as _, Styled as _, + Subscription, Task, TextAlign, TextRun, UTF16Selection, Window, WrappedLine, actions, canvas, + div, fill, point, px, }; use gpui_native_icons::IconRole; -use gpui_native_theme::{ActiveTheme as _, ColorTokens, Size}; +use gpui_native_theme::{ActiveTheme as _, BORDER_WIDTH, ColorTokens, Size}; +use gpui_native_utils::focus_ring; use crate::button::{Button, ButtonVariant}; use crate::common::{Sizable, h_flex}; @@ -1904,6 +1906,8 @@ pub struct Input { a11y_label: Option, /// Multiline field height in rows; ignored when the state is single-line. rows: usize, + prefix: Option, + suffix: Option, } impl Input { @@ -1920,6 +1924,8 @@ impl Input { disabled: false, a11y_label: None, rows: DEFAULT_ROWS, + prefix: None, + suffix: None, } } @@ -1952,6 +1958,24 @@ impl Input { self.a11y_label = Some(label.into()); self } + + /// Docks an element inside the field's leading edge. + pub fn prefix(mut self, prefix: E) -> Self + where + E: IntoElement, + { + self.prefix = Some(prefix.into_any_element()); + self + } + + /// Docks an element inside the field's trailing edge. + pub fn suffix(mut self, suffix: E) -> Self + where + E: IntoElement, + { + self.suffix = Some(suffix.into_any_element()); + self + } } impl Sizable for Input { @@ -1974,18 +1998,20 @@ impl RenderOnce for Input { let revealed = self.state.read(cx).is_revealed(); let multiline = self.state.read(cx).is_multiline(); - let right_padding = if is_password { - metrics.dense_padding_x + metrics.dense_icon_size - } else { - metrics.dense_padding_x - }; + let small = theme.sizes.resolve(Size::Small); + let docked_height = small.control_height + BORDER_WIDTH * 2.0; + let edge_inset = ((metrics.input_height - docked_height) / 2.0).max(Pixels::ZERO); - // The field's own breathing room; the text starts here from the top when multiline. let vertical_inset = metrics.dense_padding_x * 0.5; + let reveal_width = small.dense_padding_x * 2.0 + small.icon_size; let field_padding = FieldPadding { - left: metrics.dense_padding_x, - right: right_padding, + left: Pixels::ZERO, + right: if is_password { + (edge_inset + reveal_width - metrics.dense_padding_x).max(Pixels::ZERO) + } else { + Pixels::ZERO + }, top: vertical_inset, }; @@ -1993,9 +2019,10 @@ impl RenderOnce for Input { DefiniteLength::Fraction(ratio) => metrics.text_size * ratio, DefiniteLength::Absolute(length) => length.to_pixels(window.rem_size()), }); + let field_height = line_height.map_or(metrics.input_height, |line_height| { line_height * self.rows as f32 + vertical_inset * 2.0 - }); + }) + BORDER_WIDTH * 2.0; let fallback_label = self .a11y_label @@ -2003,8 +2030,20 @@ impl RenderOnce for Input { .or_else(|| (!self.placeholder.is_empty()).then_some(self.placeholder.clone())) .unwrap_or_else(|| SharedString::from(self.id.to_string())); + let edge_zone = |name: &'static str, element: AnyElement| { + div() + .debug_selector(|| name.to_string()) + .flex_none() + .h_full() + .flex() + .items_center() + .on_mouse_down(MouseButton::Left, |_, _, cx| cx.stop_propagation()) + .child(element) + }; + h_flex() .id(self.id.clone()) + .debug_selector(|| "input-field".to_string()) .key_context(INPUT_CONTEXT) .role(Role::TextInput) .aria_label(fallback_label) @@ -2012,117 +2051,159 @@ impl RenderOnce for Input { .overflow_hidden() .w_full() .h(field_height) - .px(metrics.dense_padding_x) + .pl(if self.prefix.is_some() { + edge_inset + } else { + metrics.dense_padding_x + }) + .pr(if self.suffix.is_some() { + edge_inset + } else { + metrics.dense_padding_x + }) + .gap_1p5() .rounded(radius.control) - .border_1() - .border_color(colors.border) .bg(colors.input) .text_size(metrics.text_size) .text_color(colors.foreground) - .when_some(line_height, |input, line_height| { - input.items_start().line_height(line_height) + .border_1() + .border_color(if theme.borders.fields { + colors.border + } else { + colors.input }) .when(self.disabled, |input| input.opacity(0.5)) .when(!self.disabled, |input| { input.track_focus(&focus_handle).tab_stop(true) }) - .when(focused, |input| input.border_color(colors.ring)) - .when(is_empty && !focused, |input| { - input.child( - div() - .text_color(colors.muted_foreground) - .when(multiline, |placeholder| placeholder.pt(vertical_inset)) - .child(self.placeholder.clone()), - ) + .when(focused && !theme.borders.focus_ring, |input| { + input.border_color(colors.ring) + }) + .when(focused && theme.borders.focus_ring, |input| { + input.shadow(focus_ring(colors.ring)) + }) + .when_some(self.prefix, |input, prefix| { + input.child(edge_zone("input-prefix", prefix)) }) .child( - canvas( - { - let state = self.state.clone(); - move |bounds, window, cx| { - // The hitbox backs the I-beam hover cursor. - let hitbox = window.insert_hitbox(bounds, HitboxBehavior::Normal); - let wrap_width = multiline.then(|| { - (bounds.size.width - field_padding.left - field_padding.right) - .max(Pixels::ZERO) - }); - let text = state.read(cx).shape_current_text(wrap_width, window); - (text, hitbox) - } - }, - { - let state = self.state.clone(); - let focus = focus_handle.clone(); - let disabled = self.disabled; + div() + .relative() + .flex_1() + .min_w_0() + .h_full() + .overflow_hidden() + .flex() + .flex_row() + .items_center() + .when_some(line_height, |region, line_height| { + region.items_start().line_height(line_height) + }) + .when(is_empty && !focused, |region| { + region.child( + div() + .text_color(colors.muted_foreground) + .when(multiline, |placeholder| placeholder.pt(vertical_inset)) + .child(self.placeholder.clone()), + ) + }) + .child( + canvas( + { + let state = self.state.clone(); + move |bounds, window, cx| { + // The hitbox backs the I-beam hover cursor. + let hitbox = + window.insert_hitbox(bounds, HitboxBehavior::Normal); + let wrap_width = multiline.then(|| { + (bounds.size.width + - field_padding.left + - field_padding.right) + .max(Pixels::ZERO) + }); + let text = + state.read(cx).shape_current_text(wrap_width, window); + (text, hitbox) + } + }, + { + let state = self.state.clone(); + let focus = focus_handle.clone(); + let disabled = self.disabled; - move |bounds, (text, hitbox), window, cx| { - if !disabled { - window.set_cursor_style(CursorStyle::IBeam, &hitbox); - } + move |bounds, (text, hitbox), window, cx| { + if !disabled { + window.set_cursor_style(CursorStyle::IBeam, &hitbox); + } - let dragging = state.update(cx, |state, cx| { - state.paint(text, bounds, field_padding, colors, window, cx) - }); + let dragging = state.update(cx, |state, cx| { + state.paint(text, bounds, field_padding, colors, window, cx) + }); - if disabled { - return; - } - - window.handle_input( - &focus, - gpui::ElementInputHandler::new(bounds, state.clone()), - cx, - ); - - window.on_mouse_event({ - let state = state.clone(); - move |event: &ScrollEvent, phase, window, cx| { - if phase != DispatchPhase::Bubble - || !bounds.contains(&event.position) - { + if disabled { return; } - let delta = match event.delta { - ScrollDelta::Pixels(delta) => delta, - ScrollDelta::Lines(lines) => point( - px(lines.x * SCROLL_LINES_HEIGHT), - px(lines.y * SCROLL_LINES_HEIGHT), - ), - }; + window.handle_input( + &focus, + gpui::ElementInputHandler::new(bounds, state.clone()), + cx, + ); - let consumed = state - .update(cx, |state, cx| state.scroll_by(delta, window, cx)); + window.on_mouse_event({ + let state = state.clone(); + move |event: &ScrollEvent, phase, window, cx| { + if phase != DispatchPhase::Bubble + || !bounds.contains(&event.position) + { + return; + } - if consumed { - cx.stop_propagation(); + let delta = match event.delta { + ScrollDelta::Pixels(delta) => delta, + ScrollDelta::Lines(lines) => point( + px(lines.x * SCROLL_LINES_HEIGHT), + px(lines.y * SCROLL_LINES_HEIGHT), + ), + }; + + let consumed = state.update(cx, |state, cx| { + state.scroll_by(delta, window, cx) + }); + + if consumed { + cx.stop_propagation(); + } + } + }); + + if dragging { + InputState::register_drag_listeners(&state, window); } } - }); - - if dragging { - InputState::register_drag_listeners(&state, window); - } - } - }, - ) - .absolute() - .inset_0(), + }, + ) + .absolute() + .inset_0(), + ), ) + .when_some(self.suffix, |input, suffix| { + input.child(edge_zone("input-suffix", suffix)) + }) .when(is_password && !self.disabled, |input| { input.child( div() .id((self.id.clone(), "input-reveal")) .debug_selector(|| "input-reveal".to_string()) .absolute() - .right(metrics.dense_padding_x / 2.0) - .top_0p5() - .bottom_0p5() + .right(edge_inset) + .top_0() + .bottom_0() .flex() .items_center() .on_mouse_down(MouseButton::Left, |_, _, cx| cx.stop_propagation()) .child( Button::new((self.id.clone(), "input-reveal-button")) + .debug_selector(|| "input-reveal-button".to_string()) .icon(if revealed { IconRole::EyeOff } else { diff --git a/crates/core/tests/control_sizes.rs b/crates/core/tests/control_sizes.rs index e83662b..3a06744 100644 --- a/crates/core/tests/control_sizes.rs +++ b/crates/core/tests/control_sizes.rs @@ -5,7 +5,7 @@ use gpui::{ use gpui_native_core::button::{Button, ButtonVariant}; use gpui_native_core::input::{Input, InputState}; use gpui_native_theme::ActiveTheme as _; -use gpui_native_theme::Theme; +use gpui_native_theme::{BORDER_WIDTH, Theme}; struct Harness { state: Entity, @@ -36,9 +36,10 @@ impl Render for Harness { } } -/// The medium input renders at the theme's stated field height and the -/// medium button at the stated control height; the two match wherever the -/// platform states equal values (macOS, Windows, GNOME). +/// The medium input and button render at the stated height plus the +/// hairline border sized outside it, so the two match wherever the platform +/// states equal values (macOS, Windows, GNOME) and stay equal to each other +/// on every platform. #[gpui::test] fn controls_render_at_the_stated_heights(cx: &mut TestAppContext) { cx.update(Theme::init); @@ -59,9 +60,10 @@ fn controls_render_at_the_stated_heights(cx: &mut TestAppContext) { .debug_bounds("input-wrapper") .expect("input rendered"); - // Both bounds are snapped to device pixels, so allow half a pixel. - let input_height = cx.update(|cx| cx.theme().sizes.medium.input_height); - let button_height = cx.update(|cx| cx.theme().sizes.medium.control_height); + // Both bounds are snapped to device pixels, so allow half a pixel. The + // stated height is the interior; the border sizes outside it. + let input_height = cx.update(|cx| cx.theme().sizes.medium.input_height) + BORDER_WIDTH * 2.0; + let button_height = cx.update(|cx| cx.theme().sizes.medium.control_height) + BORDER_WIDTH * 2.0; assert!((input.size.height - input_height).abs() <= gpui::px(0.5)); assert!((button.size.height - button_height).abs() <= gpui::px(0.5)); } diff --git a/crates/core/tests/input.rs b/crates/core/tests/input.rs index 1e9e625..7086659 100644 --- a/crates/core/tests/input.rs +++ b/crates/core/tests/input.rs @@ -3,12 +3,15 @@ use gpui::{ MouseUpEvent, ParentElement as _, Render, ScrollDelta, ScrollEvent, Styled as _, TestAppContext, VisualTestContext, Window, div, point, px, }; +use gpui_native_core::button::{Button, ButtonVariant}; +use gpui_native_core::common::Sizable as _; use gpui_native_core::input::{ INPUT_CONTEXT, Input, InputBindings, InputState, InputStateEvent, Submit, SubmitOn, bind_input_keys, }; -use gpui_native_theme::Theme; -use std::cell::RefCell; +use gpui_native_icons::IconRole; +use gpui_native_theme::{BORDER_WIDTH, Size, Theme}; +use std::cell::{Cell, RefCell}; use std::rc::Rc; use std::time::Duration; @@ -763,6 +766,36 @@ fn the_reveal_toggle_flips_masking() { assert!(!revealed, "the toggle should mask the field again"); } +/// The reveal pill sits with equal insets on its three free sides; the +/// small control metric must stay whole-pixel or snapping skews it. +#[test] +fn the_reveal_pill_insets_equally() { + let (cx, _state, mut visual) = reveal_window(); + + let field = visual.debug_bounds("input-field").expect("field missing"); + let pill = visual + .debug_bounds("input-reveal-button") + .expect("reveal pill missing"); + + cx.update(|cx| { + let theme = Theme::global(cx); + let small = theme.sizes.resolve(Size::Small); + let border = BORDER_WIDTH; + let docked = small.control_height + border * 2.0; + let inset = ((field.size.height - border * 2.0 - docked) / 2.0).max(px(0.)); + + let top_gap = pill.top() - (field.top() + border); + let bottom_gap = (field.bottom() - border) - pill.bottom(); + let right_gap = (field.right() - border) - pill.right(); + assert!( + (top_gap - inset).abs() < px(0.01) + && (bottom_gap - inset).abs() < px(0.01) + && (right_gap - inset).abs() < px(0.01), + "pill gaps top {top_gap:?} bottom {bottom_gap:?} right {right_gap:?} vs inset {inset:?}" + ); + }); +} + #[test] fn the_caret_can_rest_on_a_line_break() { let (cx, state, mut visual) = multiline_window("ab\ncdef"); @@ -906,3 +939,131 @@ fn vertical_scroll_keeps_the_caret_in_view() { .unwrap_or_default(); assert!(first_row_top < px(0.), "first row top: {first_row_top:?}"); } + +struct EdgeHarness { + state: Entity, + clicked: Rc>, +} + +impl Render for EdgeHarness { + fn render( + &mut self, + _window: &mut Window, + _cx: &mut gpui::Context, + ) -> impl gpui::IntoElement { + let clicked = self.clicked.clone(); + div().w(px(400.)).p_4().child( + Input::new("edge-input", self.state.clone()) + .placeholder("Search") + .prefix( + Button::new("edge-input-prefix-button") + .icon(IconRole::Search) + .variant(ButtonVariant::Ghost) + .small(), + ) + .suffix( + Button::new("edge-input-suffix-button") + .debug_selector(|| "edge-suffix-button".to_string()) + .icon(IconRole::Plus) + .variant(ButtonVariant::Ghost) + .small() + .on_click(move |_, _, _| clicked.set(true)), + ), + ) + } +} + +fn edge_input_window( + initial: &str, +) -> ( + TestAppContext, + Entity, + VisualTestContext, + Rc>, +) { + let clicked = Rc::new(Cell::new(false)); + let clicked_for_window = clicked.clone(); + let mut cx = TestAppContext::single(); + cx.update(|cx| { + Theme::init(cx); + bind_input_keys(cx, InputBindings::default()); + }); + let state = cx.update(|cx| InputState::new_with_text(initial, cx)); + let state_for_window = state.clone(); + let window = cx.add_window(move |_, _| EdgeHarness { + state: state_for_window, + clicked: clicked_for_window, + }); + let mut visual = VisualTestContext::from_window(window.into(), &cx); + visual.update(|window, cx| { + window.draw(cx).clear(cx); + }); + (cx, state, visual, clicked) +} + +/// Prefix and suffix dock flush inside the field, prefix leading, and a +/// small button in a zone centers with equal insets like the reveal pill. +#[test] +fn prefix_and_suffix_dock_inside_the_field() { + let (cx, _state, mut visual, _clicked) = edge_input_window("abc"); + + let field = visual.debug_bounds("input-field").expect("field missing"); + let prefix = visual + .debug_bounds("input-prefix") + .expect("prefix zone missing"); + let suffix = visual + .debug_bounds("input-suffix") + .expect("suffix zone missing"); + let pill = visual + .debug_bounds("edge-suffix-button") + .expect("suffix button missing"); + + assert!(prefix.left() >= field.left()); + assert!(prefix.right() <= suffix.left()); + assert!(suffix.right() <= field.right()); + + cx.update(|cx| { + let theme = Theme::global(cx); + let small = theme.sizes.resolve(Size::Small); + let border = BORDER_WIDTH; + let docked = small.control_height + border * 2.0; + let inset = ((field.size.height - border * 2.0 - docked) / 2.0).max(px(0.)); + + let top_gap = pill.top() - (field.top() + border); + let bottom_gap = (field.bottom() - border) - pill.bottom(); + let right_gap = (field.right() - border) - pill.right(); + assert!( + (top_gap - inset).abs() < px(0.01) + && (bottom_gap - inset).abs() < px(0.01) + && (right_gap - inset).abs() < px(0.01), + "suffix pill gaps top {top_gap:?} bottom {bottom_gap:?} right {right_gap:?} vs inset {inset:?}" + ); + }); +} + +/// A click on a docked control activates it without running the field's +/// caret handler underneath. +#[test] +fn a_suffix_click_activates_without_focusing_the_field() { + let (cx, state, mut visual, clicked) = edge_input_window("abc"); + + let bounds = visual + .debug_bounds("input-suffix") + .expect("suffix zone missing"); + click( + &mut visual, + point( + bounds.left() + bounds.size.width / 2.0, + bounds.top() + bounds.size.height / 2.0, + ), + ); + + assert!(clicked.get(), "the suffix button should activate"); + let focused = visual.update(|window, cx| state.read(cx).focus_handle().is_focused(window)); + assert!(!focused, "the suffix click must not focus the field"); + + // The zone must not swallow text interaction either. + focus(&mut visual, &state); + visual.simulate_keystrokes("x"); + assert_eq!(value(&cx, &state), "abcx"); +} diff --git a/crates/theme/src/lib.rs b/crates/theme/src/lib.rs index 4b068eb..63ab0e3 100644 --- a/crates/theme/src/lib.rs +++ b/crates/theme/src/lib.rs @@ -23,6 +23,10 @@ pub use system::{AccessibilityPreferences, PartialPalette, SourceKind, SystemThe use gpui::{App, Global, Hsla, Pixels, WindowAppearance, px}; use std::time::Duration; +/// The hairline width of the control border every button and field paints; +/// sized outside the stated metrics so control sizes match across platforms. +pub const BORDER_WIDTH: Pixels = px(1.0); + /// Light or dark, resolved from the system appearance. #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, serde::Deserialize)] #[serde(rename_all = "lowercase")] @@ -459,17 +463,17 @@ impl SizeTable { /// Sets the medium class's text-field height to the platform's stated value. pub fn set_input_height(&mut self, medium_input_height: Pixels) { let ratio = medium_input_height / self.medium.input_height; - self.small.input_height *= ratio; + self.small.input_height = (self.small.input_height * ratio).floor(); self.medium.input_height = medium_input_height; - self.large.input_height *= ratio; + self.large.input_height = (self.large.input_height * ratio).floor(); } /// Sets the medium class's control height to the platform's stated value. pub fn set_control_height(&mut self, medium_control_height: Pixels) { let ratio = medium_control_height / self.medium.control_height; - self.small.control_height *= ratio; + self.small.control_height = (self.small.control_height * ratio).floor(); self.medium.control_height = medium_control_height; - self.large.control_height *= ratio; + self.large.control_height = (self.large.control_height * ratio).floor(); } /// Sets the medium class's menu row height to the platform's stated value. diff --git a/crates/theme/src/resolve.rs b/crates/theme/src/resolve.rs index 7155736..389b2a7 100644 --- a/crates/theme/src/resolve.rs +++ b/crates/theme/src/resolve.rs @@ -449,17 +449,11 @@ mod tests { let mut stated = system(ThemeVariant::Light, PartialPalette::default(), None); stated.input_height = Some(px(24.)); let resolved = resolve(&stated, None, None); - - // The medium class takes the stated height verbatim, and the other - // classes keep their derived ratio to the medium class. assert_eq!(resolved.sizes.medium.input_height, px(24.)); - - // Derive what small would be without the statement, scale it by the - // stated-to-derived ratio, and compare. let derived = SizeTable::with_root_font_size(px(16.)); let scale = px(24.) / derived.medium.input_height; - let expected_small = derived.small.input_height * scale; - assert!((resolved.sizes.small.input_height - expected_small).abs() < px(0.01)); + let expected_small = (derived.small.input_height * scale).floor(); + assert_eq!(resolved.sizes.small.input_height, expected_small); } #[test] @@ -471,14 +465,10 @@ mod tests { let derived = SizeTable::with_root_font_size(px(16.)); let scale = px(32.) / derived.medium.control_height; - assert!( - (resolved.sizes.small.control_height - derived.small.control_height * scale).abs() - < px(0.01) - ); - assert!( - (resolved.sizes.large.control_height - derived.large.control_height * scale).abs() - < px(0.01) - ); + let expected_small = (derived.small.control_height * scale).floor(); + let expected_large = (derived.large.control_height * scale).floor(); + assert_eq!(resolved.sizes.small.control_height, expected_small); + assert_eq!(resolved.sizes.large.control_height, expected_large); } #[test] diff --git a/crates/utils/src/lib.rs b/crates/utils/src/lib.rs index 7297b2b..880ff50 100644 --- a/crates/utils/src/lib.rs +++ b/crates/utils/src/lib.rs @@ -35,9 +35,9 @@ pub fn focus_handle(id: &ElementId, window: &mut Window, cx: &mut App) -> FocusH .clone() } -/// Returns a focus ring shadow with the given color. +/// Returns a focus ring shadow with the given color at half ink. pub fn focus_ring(color: Hsla) -> Vec { - vec![BoxShadow::new(px(0.), px(0.), color).spread_radius(px(3.))] + vec![BoxShadow::new(px(0.), px(0.), color.alpha(0.5)).spread_radius(px(3.))] } /// The next selectable row in the direction, or `None` at the ends. diff --git a/docs/CHECKLIST.md b/docs/CHECKLIST.md index f751e23..bfa2743 100644 --- a/docs/CHECKLIST.md +++ b/docs/CHECKLIST.md @@ -9,17 +9,14 @@ is the first downstream consumer; its screens drive the order. ## Next -- [ ] **Input platform chrome** — field border behind - `Theme::borders.fields`; focus split between the ring shadow and - border recolor via `Theme::borders.focus_ring`. +- [ ] **Select platform chrome** — the input treatment on the trigger; the + popup border behind `Theme::borders.overlays`. ## Queue — to follow Platform metric adoption continues one component at a time; each stop goes through the review gate and a gallery pass. -- [ ] **Select platform chrome** — the input treatment on the trigger; the - popup border behind `Theme::borders.overlays`. - [ ] **Menu platform chrome** — overlay border gate; paddings, separators, and the row min-height through `Theme::spacing` and the stated menu row height. @@ -87,13 +84,25 @@ Gaps surfaced by the signed comparison; ordered by signed's usage. ### core (`crates/core`) -- [x] Button platform chrome — the filled variants' 1px border gated behind - `Theme::borders.buttons` (Windows / KDE keep it, macOS / GNOME paint - fills, ghost stays borderless); icon-bearing buttons pad with - `dense_padding_x` instead of `padding_x` +- [x] Button platform chrome — every variant always paints the hairline + border, sized outside the stated control height (`BORDER_WIDTH`); where + the platform paints no border (`Theme::borders.buttons`, ghost) the + border takes the fill's color and disappears, so sizes match across + platforms; icon-bearing buttons pad with `dense_padding_x` instead of + `padding_x` - [x] Button loading state — `.loading(true)` shows the spinner in place of the icon and suppresses activation; Spinner gained a `.color` override for accent-filled surfaces +- [x] Input platform chrome — the field always paints the hairline border, + sized outside the stated field height; where the platform paints no + border (`Theme::borders.fields`) it takes the field's fill; focus + recolors the border on Windows / KDE and paints the accent ring on + macOS / GNOME via `Theme::borders.focus_ring`; the shared `focus_ring` + band paints at half ink (every focusable control picks it up) +- [x] Input prefix / suffix — elements docked inside the field's edges, + typically small ghost buttons; a docked click activates the control + without reaching the field's caret handler, and the text region shrinks + to make room - [x] Button, Checkbox, Switch, Radio + RadioGroup, Toggle, Link, Slider, Progress, Avatar (image + seeded pixel-art fallback) - [x] Spinner — painted ring + rotating-icon variant, indeterminate and diff --git a/examples/gallery/src/main.rs b/examples/gallery/src/main.rs index 422f550..de91736 100644 --- a/examples/gallery/src/main.rs +++ b/examples/gallery/src/main.rs @@ -974,6 +974,7 @@ struct InputDemo { text: Entity, password: Entity, notes: Entity, + search: Entity, _subscriptions: Vec, } @@ -984,17 +985,20 @@ impl InputDemo { password.update(cx, |state, cx| state.set_password(true, cx)); let notes = InputState::new(cx); notes.update(cx, |state, cx| state.set_multiline(true, cx)); + let search = InputState::new(cx); // The panel is a cached view in the dock: the value readouts re-render // only when the inputs it reads are notified. let subscriptions = vec![ cx.observe(&text, |_, _, cx| cx.notify()), cx.observe(&password, |_, _, cx| cx.notify()), cx.observe(¬es, |_, _, cx| cx.notify()), + cx.observe(&search, |_, _, cx| cx.notify()), ]; Self { text, password, notes, + search, _subscriptions: subscriptions, } } @@ -1007,7 +1011,36 @@ impl Render for InputDemo { ScrollArea::new("input-demo").size_full().p_4().child( v_flex() .gap_3() - .child(Input::new("input-demo-text", self.text.clone()).placeholder("Type here…")) + .child( + Input::new("input-demo-search", self.search.clone()) + .placeholder("Search") + .prefix( + Button::new("input-demo-search-prefix") + .icon(IconRole::Search) + .variant(ButtonVariant::Ghost) + .small() + .accessibility_label("Search"), + ), + ) + .child( + Input::new("input-demo-text", self.text.clone()) + .placeholder("Type here…") + .suffix( + Button::new("input-demo-clear") + .icon(IconRole::Close) + .variant(ButtonVariant::Ghost) + .small() + .accessibility_label("Clear") + .on_click({ + let text = self.text.clone(); + move |_, window, cx| { + text.update(cx, |state, cx| { + state.set_value("", window, cx) + }); + } + }), + ), + ) .child( Input::new("input-demo-password", self.password.clone()) .placeholder("Password"), -- 2.54.0 From a859704185614d6d1ff6824504d6b848bb4132c3 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sat, 7 Nov 2026 19:30:04 +0700 Subject: [PATCH 05/14] audit select --- crates/core/src/menu.rs | 70 +++++++++++------------- crates/core/src/popover.rs | 66 ++++++++-------------- crates/core/src/select.rs | 32 +++++++++-- crates/core/tests/popover.rs | 103 +++++++++++++++++++++++------------ crates/core/tests/select.rs | 84 +++++++++++++++++++++++++++- crates/theme/src/lib.rs | 16 +++--- crates/theme/src/resolve.rs | 15 ++++- docs/CHECKLIST.md | 14 +++-- 8 files changed, 262 insertions(+), 138 deletions(-) diff --git a/crates/core/src/menu.rs b/crates/core/src/menu.rs index 48fe83e..43fd3ff 100644 --- a/crates/core/src/menu.rs +++ b/crates/core/src/menu.rs @@ -127,6 +127,7 @@ impl MenuItem { struct MenuChrome { surface: gpui::Hsla, border: gpui::Hsla, + panel_border: gpui::Hsla, foreground: gpui::Hsla, muted_foreground: gpui::Hsla, primary: gpui::Hsla, @@ -242,6 +243,11 @@ impl RenderOnce for Menu { MenuChrome { surface: theme.colors.surface, border: theme.colors.border, + panel_border: if theme.borders.overlays { + theme.colors.border + } else { + theme.colors.surface + }, foreground: theme.colors.foreground, muted_foreground: theme.colors.muted_foreground, primary: theme.ramps.primary.base, @@ -259,7 +265,6 @@ impl RenderOnce for Menu { }; let trigger_bounds: Rc>>> = Rc::new(RefCell::new(None)); - let panel_bounds: Rc>>> = Rc::new(RefCell::new(None)); let selection = window.use_keyed_state("menu-selection", cx, |_, _| 0usize); let was_open = window.use_keyed_state("menu-was-open", cx, |_, _| self.open); @@ -363,6 +368,7 @@ impl RenderOnce for Menu { let panel = v_flex() .id("menu-panel") + .occlude() .track_focus(&focus) .role(Role::Menu) .aria_label(label) @@ -373,9 +379,25 @@ impl RenderOnce for Menu { .text_color(chrome.foreground) .rounded(chrome.radius_overlay) .border_1() - .border_color(chrome.border) + .border_color(chrome.panel_border) .shadow(chrome.shadow.clone()) .overflow_hidden() + .on_mouse_down_out({ + let on_change = on_change.clone(); + let trigger_bounds = trigger_bounds.clone(); + + move |event, window, cx| { + if trigger_bounds + .borrow() + .is_some_and(|bounds| bounds.contains(&event.position)) + { + return; + } + if let Some(on_change) = &on_change { + on_change(false, window, cx); + } + } + }) .on_key_down({ let items = items.clone(); let on_change = on_change.clone(); @@ -392,44 +414,14 @@ impl RenderOnce for Menu { .children(rows), ) .child( - canvas( - { - let cell = panel_bounds.clone(); - move |bounds, _window, _cx| *cell.borrow_mut() = Some(bounds) - }, - { - let on_change = on_change.clone(); - move |_, _, window, cx| { - if !focus.contains_focused(window, cx) { - window.focus(&focus, cx); - } - - window.on_mouse_event({ - let on_change = on_change.clone(); - move |event: &MouseDownEvent, phase, window, cx| { - if phase != DispatchPhase::Bubble { - return; - } - - let on_panel = panel_bounds - .borrow() - .is_some_and(|bounds| bounds.contains(&event.position)); - let on_trigger = trigger_bounds - .borrow() - .is_some_and(|bounds| bounds.contains(&event.position)); - if on_panel || on_trigger { - return; - } - - if let Some(on_change) = &on_change { - on_change(false, window, cx); - } - cx.stop_propagation(); - } - }); + canvas(|_, _, _| {}, { + let focus = focus.clone(); + move |_, _, window, cx| { + if !focus.contains_focused(window, cx) { + window.focus(&focus, cx); } - }, - ) + } + }) .absolute() .inset_0(), ); diff --git a/crates/core/src/popover.rs b/crates/core/src/popover.rs index 9e5b95c..97d2be1 100644 --- a/crates/core/src/popover.rs +++ b/crates/core/src/popover.rs @@ -102,7 +102,6 @@ impl RenderOnce for Popover { }; let trigger_bounds: BoundsCell = Rc::new(RefCell::new(None)); - let panel_bounds: BoundsCell = Rc::new(RefCell::new(None)); let fallback_label = self .a11y_label @@ -166,6 +165,7 @@ impl RenderOnce for Popover { let panel = v_flex() .id("popover-panel") + .occlude() .track_focus(&focus) .role(Role::Dialog) .aria_label(label) @@ -178,6 +178,21 @@ impl RenderOnce for Popover { .border_color(colors.border) .shadow(shadows) .overflow_hidden() + .on_mouse_down_out({ + let on_change = on_change.clone(); + let trigger_bounds = trigger_bounds.clone(); + move |event, window, cx| { + if trigger_bounds + .borrow() + .is_some_and(|bounds| bounds.contains(&event.position)) + { + return; + } + if let Some(on_change) = &on_change { + on_change(false, window, cx); + } + } + }) .on_key_down({ let on_change = on_change.clone(); move |event: &KeyDownEvent, window, cx| { @@ -191,49 +206,14 @@ impl RenderOnce for Popover { }) .when_some(content, |panel, content| panel.child(content)) .child( - canvas( - { - let cell = panel_bounds.clone(); - move |bounds, _window, _cx| *cell.borrow_mut() = Some(bounds) - }, - { - let on_change = on_change.clone(); - let focus = focus; - move |_, _, window, cx| { - if !focus.contains_focused(window, cx) { - window.focus(&focus, cx); - } - window.on_mouse_event({ - let on_change = on_change.clone(); - move |event: &MouseDownEvent, phase, window, cx| { - if phase != DispatchPhase::Bubble - || event.button != MouseButton::Left - { - return; - } - - let on_panel = panel_bounds - .borrow() - .is_some_and(|bounds| bounds.contains(&event.position)); - - let on_trigger = trigger_bounds - .borrow() - .is_some_and(|bounds| bounds.contains(&event.position)); - - if on_panel || on_trigger { - return; - } - - if let Some(on_change) = &on_change { - on_change(false, window, cx); - } - - cx.stop_propagation(); - } - }); + canvas(|_, _, _| {}, { + let focus = focus; + move |_, _, window, cx| { + if !focus.contains_focused(window, cx) { + window.focus(&focus, cx); } - }, - ) + } + }) .absolute() .inset_0(), ); diff --git a/crates/core/src/select.rs b/crates/core/src/select.rs index 132bb35..2acea6c 100644 --- a/crates/core/src/select.rs +++ b/crates/core/src/select.rs @@ -6,7 +6,8 @@ use gpui::{ SharedString, StatefulInteractiveElement as _, StyleRefinement, Styled as _, Window, }; use gpui_native_icons::{IconRole, icon}; -use gpui_native_theme::{ActiveTheme as _, Size}; +use gpui_native_theme::{ActiveTheme as _, BORDER_WIDTH, Size}; +use gpui_native_utils::focus_ring; use crate::common::{Sizable, StyledExt, h_flex}; use crate::menu::{Menu, MenuItem}; @@ -169,6 +170,8 @@ where let muted_foreground = theme.colors.muted_foreground; let radius = theme.radius.control; let icon_size = metrics.dense_icon_size; + let fields_bordered = theme.borders.fields; + let focus_ring_on = theme.borders.focus_ring; let label = self .a11y_label @@ -180,7 +183,22 @@ where .use_keyed_state("select-open", cx, |_, _| false) .clone() }); + let is_open = *open.read(cx); + let open_for_items = open.clone(); + + let (rest_border, hover_border) = if fields_bordered { + ( + if is_open && !focus_ring_on { + ring + } else { + border + }, + border, + ) + } else { + (input, muted) + }; let selected_index = self.selected.as_ref().and_then(|value| { self.options @@ -194,6 +212,7 @@ where let trigger_id = self.id.clone(); let trigger_label = label.clone(); let trigger_style = self.style; + let trigger = move |_window: &mut Window, _cx: &mut App| { let text = trigger_selected .as_ref() @@ -207,17 +226,18 @@ where h_flex() .id((trigger_id.clone(), "select-trigger")) + .debug_selector(|| "select-trigger".to_string()) .role(Role::ComboBox) .aria_label(trigger_label.clone()) .cursor_pointer() .justify_between() .gap_2() .w_full() - .h(metrics.input_height) + .h(metrics.input_height + BORDER_WIDTH * 2.0) .px(metrics.dense_padding_x) .rounded(radius) .border_1() - .border_color(if is_open { ring } else { border }) + .border_color(rest_border) .bg(input) .text_size(metrics.text_size) .text_color(if trigger_selected.is_some() { @@ -225,7 +245,10 @@ where } else { muted_foreground }) - .hover(|style| style.bg(muted)) + .when(is_open && focus_ring_on, |trigger| { + trigger.shadow(focus_ring(ring)) + }) + .hover(|style| style.bg(muted).border_color(hover_border)) .child(text) .child( icon(IconRole::ChevronDown) @@ -236,7 +259,6 @@ where .into_any_element() }; - let open_for_items = open.clone(); let items = move |_window: &mut Window, _cx: &mut App| { self.options .iter() diff --git a/crates/core/tests/popover.rs b/crates/core/tests/popover.rs index ec91f26..adaf33a 100644 --- a/crates/core/tests/popover.rs +++ b/crates/core/tests/popover.rs @@ -1,8 +1,9 @@ use std::{cell::RefCell, rc::Rc}; use gpui::{ - Context, IntoElement, Modifiers, MouseButton, ParentElement as _, Render, Styled as _, - TestAppContext, VisualTestContext, Window, WindowHandle, div, point, px, rgba, + Context, InteractiveElement as _, IntoElement, Modifiers, MouseButton, ParentElement as _, + Render, StatefulInteractiveElement as _, Styled as _, TestAppContext, VisualTestContext, + Window, WindowHandle, div, point, px, rgba, }; use gpui_native_core::popover::Popover; use gpui_native_theme::Theme; @@ -10,37 +11,53 @@ use gpui_native_theme::Theme; struct Harness { open: bool, changes: Rc>>, + target_clicks: Rc>, } impl Render for Harness { fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { let changes = self.changes.clone(); + let target_clicks = self.target_clicks.clone(); let view = cx.entity(); - div().size_full().child( - Popover::new("test-popover") - .open(self.open) - .on_change(move |next, _, cx| { - changes.borrow_mut().push(next); - view.update(cx, |harness, cx| { - harness.open = next; - cx.notify(); - }); - }) - .trigger(|_, _| { + div() + .size_full() + .child( + div().absolute().left(px(400.)).top(px(300.)).child( div() - .w(px(100.)) - .h(px(32.)) + .id("popover-target") + .size(px(40.)) .bg(rgba(0x000000ff)) - .into_any_element() - }) - .content(|_, _| { - div() - .w(px(200.)) - .h(px(80.)) - .child("Popover content") - .into_any_element() - }), - ) + .on_click(move |_, _, cx| { + *target_clicks.borrow_mut() += 1; + cx.stop_propagation(); + }), + ), + ) + .child( + Popover::new("test-popover") + .open(self.open) + .on_change(move |next, _, cx| { + changes.borrow_mut().push(next); + view.update(cx, |harness, cx| { + harness.open = next; + cx.notify(); + }); + }) + .trigger(|_, _| { + div() + .w(px(100.)) + .h(px(32.)) + .bg(rgba(0x000000ff)) + .into_any_element() + }) + .content(|_, _| { + div() + .w(px(200.)) + .h(px(80.)) + .child("Popover content") + .into_any_element() + }), + ) } } @@ -48,27 +65,31 @@ impl Render for Harness { /// hangs below it around `(0..200, 36..116)`, so clicks at `(10, 16)` land /// on the trigger, `(100, 70)` on the panel, and anything past `(300, 300)` /// is outside both. -fn popover_window( - open: bool, -) -> ( +type PopoverWindow = ( TestAppContext, VisualTestContext, WindowHandle, Rc>>, -) { + Rc>, +); + +fn popover_window(open: bool) -> PopoverWindow { let mut cx = TestAppContext::single(); cx.update(Theme::init); let changes = Rc::new(RefCell::new(Vec::new())); let harness_changes = changes.clone(); + let target_clicks = Rc::new(RefCell::new(0)); + let harness_clicks = target_clicks.clone(); let window = cx.add_window(move |_, _| Harness { open, changes: harness_changes, + target_clicks: harness_clicks, }); let mut visual = VisualTestContext::from_window(window.into(), &cx); visual.update(|window, cx| { window.draw(cx).clear(cx); }); - (cx, visual, window, changes) + (cx, visual, window, changes, target_clicks) } fn draw(visual: &mut VisualTestContext) { @@ -84,7 +105,7 @@ fn click(visual: &mut VisualTestContext, position: gpui::Point) { #[test] fn clicking_outside_dismisses_and_reports_closed() { - let (_cx, mut visual, _window, changes) = popover_window(true); + let (_cx, mut visual, _window, changes, _clicks) = popover_window(true); click(&mut visual, point(px(600.), px(600.))); assert_eq!(*changes.borrow(), vec![false]); @@ -98,7 +119,7 @@ fn clicking_outside_dismisses_and_reports_closed() { #[test] fn clicking_the_trigger_toggles_and_clicking_the_panel_does_nothing() { - let (_cx, mut visual, _window, changes) = popover_window(true); + let (_cx, mut visual, _window, changes, _clicks) = popover_window(true); // On the open panel's trigger: toggles closed. click(&mut visual, point(px(10.), px(16.))); @@ -111,7 +132,7 @@ fn clicking_the_trigger_toggles_and_clicking_the_panel_does_nothing() { #[test] fn clicking_the_trigger_opens_a_closed_popover() { - let (_cx, mut visual, _window, changes) = popover_window(false); + let (_cx, mut visual, _window, changes, _clicks) = popover_window(false); click(&mut visual, point(px(10.), px(16.))); assert_eq!(*changes.borrow(), vec![true]); @@ -119,7 +140,7 @@ fn clicking_the_trigger_opens_a_closed_popover() { #[test] fn escape_dismisses_while_the_panel_holds_focus() { - let (_cx, mut visual, _window, changes) = popover_window(true); + let (_cx, mut visual, _window, changes, _clicks) = popover_window(true); draw(&mut visual); // The panel claims focus on open, so escape reaches its key listener. @@ -134,7 +155,7 @@ fn escape_dismisses_while_the_panel_holds_focus() { #[test] fn the_controlled_loop_reopens_from_the_app_side() { - let (mut cx, mut visual, window, changes) = popover_window(true); + let (mut cx, mut visual, window, changes, _clicks) = popover_window(true); // Dismiss once. click(&mut visual, point(px(600.), px(600.))); @@ -155,3 +176,15 @@ fn the_controlled_loop_reopens_from_the_app_side() { visual.simulate_keystrokes("escape"); assert_eq!(*changes.borrow(), vec![false, false]); } + +/// Clicking another control while the popover is open dismisses it and still +/// acts on that control: dismissing does not claim the event. +#[test] +fn clicking_another_control_dismisses_and_reaches_it() { + let (_cx, mut visual, _window, changes, clicks) = popover_window(true); + + click(&mut visual, point(px(420.), px(320.))); + + assert_eq!(*changes.borrow(), vec![false]); + assert_eq!(*clicks.borrow(), 1); +} diff --git a/crates/core/tests/select.rs b/crates/core/tests/select.rs index 5f7f77b..8926deb 100644 --- a/crates/core/tests/select.rs +++ b/crates/core/tests/select.rs @@ -1,11 +1,12 @@ use std::{cell::RefCell, rc::Rc}; use gpui::{ - Context, IntoElement, MouseButton, ParentElement as _, Render, Styled as _, TestAppContext, - VisualTestContext, Window, div, point, px, + Context, Entity, IntoElement, MouseButton, ParentElement as _, Render, Styled as _, + TestAppContext, VisualTestContext, Window, div, point, px, }; +use gpui_native_core::input::{Input, InputState}; use gpui_native_core::select::Select; -use gpui_native_theme::Theme; +use gpui_native_theme::{ActiveTheme as _, BORDER_WIDTH, Theme}; #[derive(Clone, Copy, Debug, PartialEq, Eq)] enum Strategy { @@ -200,3 +201,80 @@ fn two_selects_keep_their_popups_independent() { assert_eq!(*first_changes.borrow(), vec![]); assert_eq!(*second_changes.borrow(), vec![Some(Strategy::Curated)]); } + +#[test] +fn a_click_outside_closes_the_popup() { + let (_cx, mut visual, changes) = select_window(); + + click(&mut visual, point(px(100.), px(31.))); + draw(&mut visual); + click(&mut visual, point(px(100.), px(300.))); + draw(&mut visual); + + // The popup held keyboard focus while open; a keystroke after the outside + // click answers nothing if the popup really closed. + visual.simulate_keystrokes("enter"); + draw(&mut visual); + assert_eq!(*changes.borrow(), Vec::>::new()); +} + +/// Clicking another control while the popup is open dismisses it and still +/// acts on the control: dismissing does not claim the event. +#[test] +fn a_click_on_another_control_dismisses_and_reaches_it() { + let (_cx, mut visual, first_changes, second_changes) = pair_window(); + + click(&mut visual, point(px(100.), px(31.))); + draw(&mut visual); + + // The second select's trigger, below the first — at x=300, past the first + // popup's 182px-wide panel: this click must close the first popup AND open + // the second. + click(&mut visual, point(px(300.), px(69.))); + draw(&mut visual); + visual.simulate_keystrokes("enter"); + draw(&mut visual); + + assert_eq!(*first_changes.borrow(), Vec::>::new()); + assert_eq!(*second_changes.borrow(), vec![Some(Strategy::Curated)]); +} + +struct HeightsHarness { + state: Entity, +} + +impl Render for HeightsHarness { + fn render(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { + div() + .flex() + .flex_col() + .gap_4() + .child(Select::new("heights").options(STRATEGIES)) + .child(Input::new("heights-field", self.state.clone())) + } +} + +/// The trigger renders at the same height as the input field: the stated +/// metric plus the always-painted hairline border, whatever the platform. +#[test] +fn the_trigger_matches_the_input_height() { + let mut cx = TestAppContext::single(); + cx.update(Theme::init); + let state = cx.update(InputState::new); + let state_for_window = state.clone(); + let window = cx.add_window(move |_, _| HeightsHarness { + state: state_for_window, + }); + let mut visual = VisualTestContext::from_window(window.into(), &cx); + draw(&mut visual); + + let trigger = visual + .debug_bounds("select-trigger") + .expect("trigger rendered"); + let field = visual.debug_bounds("input-field").expect("input rendered"); + + // Snapped bounds: allow half a pixel. + assert!((trigger.size.height - field.size.height).abs() <= px(0.5)); + let expected = cx.update(|cx| cx.theme().sizes.medium.input_height) + BORDER_WIDTH * 2.0; + assert!((trigger.size.height - expected).abs() <= px(0.5)); +} diff --git a/crates/theme/src/lib.rs b/crates/theme/src/lib.rs index 63ab0e3..26f8eb9 100644 --- a/crates/theme/src/lib.rs +++ b/crates/theme/src/lib.rs @@ -101,18 +101,18 @@ impl Default for Radius { pub struct Borders { /// Filled actions keep a border (Windows, KDE). pub buttons: bool, - /// Text fields and selects keep a border (Windows, KDE; the macOS - /// hairline bezel). + /// Text fields and selects keep a border. pub fields: bool, - /// Floating layers keep a border (Windows, KDE). + /// Floating layers keep a border. pub overlays: bool, - /// Keyboard focus paints an accent ring around the control (macOS, - /// GNOME); otherwise focus re-colors the border (Windows, KDE). + /// Keyboard focus paints an accent ring around the control. pub focus_ring: bool, } impl Borders { - pub(crate) fn for_platform(source: SourceKind) -> Self { + pub(crate) fn for_platform(source: SourceKind, variant: ThemeVariant) -> Self { + let overlays = variant == ThemeVariant::Dark; + match source { SourceKind::Windows | SourceKind::Kde => Self { buttons: true, @@ -123,13 +123,13 @@ impl Borders { SourceKind::Macos => Self { buttons: false, fields: true, - overlays: false, + overlays, focus_ring: true, }, SourceKind::Omarchy | SourceKind::Gnome => Self { buttons: false, fields: false, - overlays: false, + overlays, focus_ring: true, }, SourceKind::Fallback => Self { diff --git a/crates/theme/src/resolve.rs b/crates/theme/src/resolve.rs index 389b2a7..035d5b2 100644 --- a/crates/theme/src/resolve.rs +++ b/crates/theme/src/resolve.rs @@ -275,7 +275,7 @@ pub(crate) fn resolve( }, colors, radius, - borders: Borders::for_platform(system.source), + borders: Borders::for_platform(system.source, variant), spacing: Spacing { unit: system.source.spacing_unit(), }, @@ -535,6 +535,19 @@ mod tests { assert_eq!(unit, px(4.)); } + #[test] + fn dark_mode_separates_overlays_by_border() { + for source in [SourceKind::Macos, SourceKind::Gnome, SourceKind::Omarchy] { + let mut stated = system(ThemeVariant::Dark, PartialPalette::default(), None); + stated.source = source; + let resolved = resolve(&stated, None, None); + assert!( + resolved.borders.overlays, + "{source:?} frames floating layers in dark" + ); + } + } + #[test] fn overlay_is_translucent_and_overridable() { let light = resolve( diff --git a/docs/CHECKLIST.md b/docs/CHECKLIST.md index bfa2743..3ef5043 100644 --- a/docs/CHECKLIST.md +++ b/docs/CHECKLIST.md @@ -9,9 +9,6 @@ is the first downstream consumer; its screens drive the order. ## Next -- [ ] **Select platform chrome** — the input treatment on the trigger; the - popup border behind `Theme::borders.overlays`. - ## Queue — to follow Platform metric adoption continues one component at a time; each stop goes @@ -74,7 +71,9 @@ Gaps surfaced by the signed comparison; ordered by signed's usage. metrics and the full radius ladder per platform, Linux root fonts (GNOME gsettings `font-name`, KDE `kdeglobals font=`), per-source border + focus chrome (`Theme::borders`), and a per-platform spacing - unit (`Theme::spacing`). Components adopt the tokens one by one + unit (`Theme::spacing`); overlay borders engage in dark mode, where + the elevation shadow barely reads and floating layers separate by + border instead. Components adopt the tokens one by one (see Next and Queue) ### icons (`crates/icons`) @@ -103,6 +102,13 @@ Gaps surfaced by the signed comparison; ordered by signed's usage. typically small ghost buttons; a docked click activates the control without reaching the field's caret handler, and the text region shrinks to make room +- [x] Select platform chrome — the trigger takes the input treatment: the + hairline border always painted, sized outside the stated field height + and taking the fill's color where the platform paints none; open + recolors the border on Windows / KDE and paints the accent ring on + macOS / GNOME, so the trigger matches a focused field; the menu + popup's border sits behind `Theme::borders.overlays`, disappearing + into the surface where the platform paints none - [x] Button, Checkbox, Switch, Radio + RadioGroup, Toggle, Link, Slider, Progress, Avatar (image + seeded pixel-art fallback) - [x] Spinner — painted ring + rotating-icon variant, indeterminate and -- 2.54.0 From 66961a1026dcdeee14304fc82de1d765227546d2 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sat, 7 Nov 2026 19:42:10 +0700 Subject: [PATCH 06/14] audit menu --- crates/core/src/menu.rs | 22 ++++++++++----- crates/core/tests/menu.rs | 22 +++++++++++++++ crates/core/tests/select_pointer.rs | 42 +++++++++++++++++++++-------- docs/CHECKLIST.md | 8 +++--- 4 files changed, 74 insertions(+), 20 deletions(-) diff --git a/crates/core/src/menu.rs b/crates/core/src/menu.rs index 43fd3ff..1cae710 100644 --- a/crates/core/src/menu.rs +++ b/crates/core/src/menu.rs @@ -7,7 +7,7 @@ use gpui::{ Entity, InteractiveElement as _, IntoElement, KeyDownEvent, MouseButton, MouseDownEvent, ParentElement as _, Pixels, Point, RenderOnce, Role, ScrollHandle, SharedString, StatefulInteractiveElement as _, Styled as _, Window, anchored, canvas, deferred, div, - ease_out_quint, px, + ease_out_quint, px, relative, }; use gpui_native_icons::{IconRole, icon as icon_glyph}; use gpui_native_theme::ActiveTheme as _; @@ -138,6 +138,9 @@ struct MenuChrome { radius_sm: Pixels, icon_size: Pixels, dense_icon_size: Pixels, + menu_item_height: Pixels, + spacing_unit: Pixels, + text_size: Pixels, shadow: Vec, } @@ -258,6 +261,9 @@ impl RenderOnce for Menu { radius_sm: theme.radius.sm, icon_size: theme.sizes.medium.icon_size, dense_icon_size: theme.sizes.medium.dense_icon_size, + menu_item_height: theme.sizes.medium.menu_item_height, + spacing_unit: theme.spacing.unit, + text_size: theme.sizes.medium.text_size, shadow: theme.elevations.popover.shadows(), }, theme.motion.overlay, @@ -374,7 +380,7 @@ impl RenderOnce for Menu { .aria_label(label) .mt_1() .min_w_56() - .p_1() + .p(chrome.spacing_unit) .bg(chrome.surface) .text_color(chrome.foreground) .rounded(chrome.radius_overlay) @@ -454,8 +460,9 @@ fn render_item( match item.kind { MenuItemKind::Separator => div() .h(px(1.)) - .mx_2() - .my_1() + .debug_selector(move || format!("menu-separator-{index}")) + .mx(chrome.spacing_unit * 2.0) + .my(chrome.spacing_unit) .bg(chrome.border) .into_any_element(), MenuItemKind::Action => { @@ -468,11 +475,14 @@ fn render_item( div() .id(("menu-item", index)) + .debug_selector(move || format!("menu-item-{index}")) .flex() .items_center() .gap_2() - .px_2() - .py_1() + .min_h(chrome.menu_item_height) + .px(chrome.spacing_unit * 2.0) + .text_size(chrome.text_size) + .line_height(relative(1.0)) .rounded(chrome.radius_sm) .map(|row| { if disabled { diff --git a/crates/core/tests/menu.rs b/crates/core/tests/menu.rs index 6021dc0..dcf94c0 100644 --- a/crates/core/tests/menu.rs +++ b/crates/core/tests/menu.rs @@ -112,6 +112,28 @@ fn click(visual: &mut VisualTestContext, position: gpui::Point) { visual.simulate_mouse_up(position, MouseButton::Left, Modifiers::default()); } +/// Every action row renders at the platform's stated menu row height, +/// whatever its content — the metric is the whole row, not a floor under +/// padding. Item 1 of the harness menu is a separator, so it is skipped. +#[test] +fn rows_render_at_the_stated_menu_row_height() { + let (cx, mut visual, _window, _changes, _activated) = menu_window(true); + + let stated = cx.update(|cx| Theme::global(cx).sizes.medium.menu_item_height); + for index in [0, 2, 3, 4] { + let selector: &'static str = Box::leak(format!("menu-item-{index}").into_boxed_str()); + let row = visual + .debug_bounds(selector) + .unwrap_or_else(|| panic!("row {index} should have been laid out")); + // Painted bounds are snapped to device pixels; allow half a pixel. + assert!( + (row.size.height - stated).abs() <= px(0.5), + "row {index} is {:?} tall, stated menu row height is {stated:?}", + row.size.height + ); + } +} + #[test] fn arrow_navigation_skips_separators_and_disabled_items() { let (_cx, mut visual, _window, changes, activated) = menu_window(true); diff --git a/crates/core/tests/select_pointer.rs b/crates/core/tests/select_pointer.rs index c24c4d9..f0dd044 100644 --- a/crates/core/tests/select_pointer.rs +++ b/crates/core/tests/select_pointer.rs @@ -67,25 +67,45 @@ fn click(visual: &mut VisualTestContext, position: gpui::Point) { visual.simulate_mouse_up(position, MouseButton::Left, gpui::Modifiers::default()); } -/// The trigger band is `16..46`; the panel hangs 4 px below it with a 4 px -/// inner padding, so the three rows ride `~58..90`, `~90..122`, `~122..154`. -/// Every option must answer a pointer press — a popup that clips its last -/// item never does. +/// The three option rows must answer a pointer press wherever they were laid +/// out — a popup that clips its last item never does. Row positions come from +/// the rendered bounds, so the stated menu row metric can change freely. #[test] fn every_option_row_answers_the_pointer() { let (_cx, mut visual, changes) = level_window(); - let expected = [ - (px(74.), Level::Low), - (px(106.), Level::Mid), - (px(138.), Level::High), - ]; - for (y, value) in expected { + // Open once to measure where the rows were actually drawn. + click(&mut visual, point(px(100.), px(31.))); + visual.update(|window, cx| { + window.draw(cx).clear(cx); + }); + let centers: Vec> = (0..3) + .map(|index| { + let selector: &'static str = Box::leak(format!("menu-item-{index}").into_boxed_str()); + let row = visual + .debug_bounds(selector) + .unwrap_or_else(|| panic!("row {index} should have been laid out")); + point( + row.left() + row.size.width / 2.0, + row.top() + row.size.height / 2.0, + ) + }) + .collect(); + + // Close again; each iteration below opens the popup fresh. + click(&mut visual, point(px(100.), px(31.))); + visual.update(|window, cx| { + window.draw(cx).clear(cx); + }); + + let expected = [(Level::Low, 0), (Level::Mid, 1), (Level::High, 2)]; + for (value, index) in expected { + // Choosing the previous row closed the popup; reopen it. click(&mut visual, point(px(100.), px(31.))); visual.update(|window, cx| { window.draw(cx).clear(cx); }); - click(&mut visual, point(px(100.), y)); + click(&mut visual, centers[index]); visual.update(|window, cx| { window.draw(cx).clear(cx); }); diff --git a/docs/CHECKLIST.md b/docs/CHECKLIST.md index 3ef5043..9fa77e1 100644 --- a/docs/CHECKLIST.md +++ b/docs/CHECKLIST.md @@ -14,9 +14,6 @@ is the first downstream consumer; its screens drive the order. Platform metric adoption continues one component at a time; each stop goes through the review gate and a gallery pass. -- [ ] **Menu platform chrome** — overlay border gate; paddings, separators, - and the row min-height through `Theme::spacing` and the stated menu - row height. - [ ] **Popover platform chrome** — overlay border gate; the anchor gap through `Theme::spacing`. - [ ] **Dialog platform chrome** — overlay border gate. @@ -109,6 +106,11 @@ Gaps surfaced by the signed comparison; ordered by signed's usage. macOS / GNOME, so the trigger matches a focused field; the menu popup's border sits behind `Theme::borders.overlays`, disappearing into the surface where the platform paints none +- [x] Menu platform chrome — the popup border behind `Theme::borders.overlays` + (and dark mode); the panel padding and separator margins come from + `Theme::spacing`, and every action row renders at the stated + `menu_item_height` with content centered, so the stated metric is the + whole row; rows carry debug selectors for headless geometry tests - [x] Button, Checkbox, Switch, Radio + RadioGroup, Toggle, Link, Slider, Progress, Avatar (image + seeded pixel-art fallback) - [x] Spinner — painted ring + rotating-icon variant, indeterminate and -- 2.54.0 From 2b3e3b4e99942e941f37a1d51718da1beb841a93 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sat, 7 Nov 2026 20:07:09 +0700 Subject: [PATCH 07/14] audit popover and dialog --- crates/core/src/dialog.rs | 6 +++++- crates/core/src/menu.rs | 7 ++++--- crates/core/src/popover.rs | 28 +++++++++++++++++++--------- crates/core/tests/popover.rs | 24 ++++++++++++++++++++++++ crates/theme/src/lib.rs | 11 +++++++++-- crates/theme/src/resolve.rs | 6 ++++-- docs/CHECKLIST.md | 16 ++++++++++------ 7 files changed, 75 insertions(+), 23 deletions(-) diff --git a/crates/core/src/dialog.rs b/crates/core/src/dialog.rs index f24d23b..3856ebc 100644 --- a/crates/core/src/dialog.rs +++ b/crates/core/src/dialog.rs @@ -130,7 +130,11 @@ impl RenderOnce for Dialog { let theme = cx.theme(); ( theme.colors.overlay, - theme.colors.border, + if theme.borders.overlays { + theme.colors.border + } else { + theme.colors.surface + }, theme.colors.foreground, theme.colors.muted_foreground, theme.colors.surface, diff --git a/crates/core/src/menu.rs b/crates/core/src/menu.rs index 1cae710..f6aea86 100644 --- a/crates/core/src/menu.rs +++ b/crates/core/src/menu.rs @@ -7,7 +7,7 @@ use gpui::{ Entity, InteractiveElement as _, IntoElement, KeyDownEvent, MouseButton, MouseDownEvent, ParentElement as _, Pixels, Point, RenderOnce, Role, ScrollHandle, SharedString, StatefulInteractiveElement as _, Styled as _, Window, anchored, canvas, deferred, div, - ease_out_quint, px, relative, + ease_out_quint, point, px, relative, }; use gpui_native_icons::{IconRole, icon as icon_glyph}; use gpui_native_theme::ActiveTheme as _; @@ -374,11 +374,11 @@ impl RenderOnce for Menu { let panel = v_flex() .id("menu-panel") + .debug_selector(|| "menu-panel".to_string()) .occlude() .track_focus(&focus) .role(Role::Menu) .aria_label(label) - .mt_1() .min_w_56() .p(chrome.spacing_unit) .bg(chrome.surface) @@ -435,7 +435,8 @@ impl RenderOnce for Menu { let anchored = match self.open_at { Some(position) => anchored().position(position), None => anchored(), - }; + } + .offset(point(px(0.), chrome.spacing_unit)); wrapper = wrapper.child(deferred(anchored.child(panel.with_animation( "menu-enter", diff --git a/crates/core/src/popover.rs b/crates/core/src/popover.rs index 97d2be1..bc226f8 100644 --- a/crates/core/src/popover.rs +++ b/crates/core/src/popover.rs @@ -6,7 +6,7 @@ use gpui::{ Animation, AnimationExt as _, AnyElement, App, Bounds, DispatchPhase, ElementId, InteractiveElement as _, IntoElement, KeyDownEvent, MouseButton, MouseDownEvent, ParentElement as _, Pixels, RenderOnce, Role, SharedString, StatefulInteractiveElement as _, - Styled as _, Window, anchored, canvas, deferred, div, ease_out_quint, + Styled as _, Window, anchored, canvas, deferred, div, ease_out_quint, point, px, }; use gpui_native_theme::ActiveTheme as _; @@ -91,13 +91,19 @@ impl Popover { impl RenderOnce for Popover { fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { - let (colors, radius, shadows, enter_duration) = { + let (colors, radius, shadows, enter_duration, panel_border, anchor_gap) = { let theme = cx.theme(); ( theme.colors, theme.radius, theme.elevations.popover.shadows(), theme.motion.overlay, + if theme.borders.overlays { + theme.colors.border + } else { + theme.colors.surface + }, + theme.spacing.unit, ) }; @@ -165,17 +171,17 @@ impl RenderOnce for Popover { let panel = v_flex() .id("popover-panel") + .debug_selector(|| "popover-panel".to_string()) .occlude() .track_focus(&focus) .role(Role::Dialog) .aria_label(label) - .mt_1() .min_w_32() .bg(colors.surface) .text_color(colors.surface_foreground) .rounded(radius.overlay) .border_1() - .border_color(colors.border) + .border_color(panel_border) .shadow(shadows) .overflow_hidden() .on_mouse_down_out({ @@ -218,11 +224,15 @@ impl RenderOnce for Popover { .inset_0(), ); - wrapper = wrapper.child(deferred(anchored().child(panel.with_animation( - "popover-enter", - Animation::new(enter_duration).with_easing(ease_out_quint()), - |panel, delta| panel.opacity(delta), - )))); + wrapper = wrapper.child(deferred( + anchored() + .offset(point(px(0.), anchor_gap)) + .child(panel.with_animation( + "popover-enter", + Animation::new(enter_duration).with_easing(ease_out_quint()), + |panel, delta| panel.opacity(delta), + )), + )); } wrapper diff --git a/crates/core/tests/popover.rs b/crates/core/tests/popover.rs index adaf33a..c51d024 100644 --- a/crates/core/tests/popover.rs +++ b/crates/core/tests/popover.rs @@ -45,6 +45,7 @@ impl Render for Harness { }) .trigger(|_, _| { div() + .debug_selector(|| "popover-trigger".to_string()) .w(px(100.)) .h(px(32.)) .bg(rgba(0x000000ff)) @@ -188,3 +189,26 @@ fn clicking_another_control_dismisses_and_reaches_it() { assert_eq!(*changes.borrow(), vec![false]); assert_eq!(*clicks.borrow(), 1); } + +/// The panel hangs one spacing unit below the trigger: the anchor gap is +/// theme-owned, and rides the anchored element so the window-edge clamp +/// stays intact. +#[test] +fn the_panel_hangs_one_spacing_unit_below_the_trigger() { + let (cx, mut visual, _window, _changes, _clicks) = popover_window(true); + + let trigger = visual + .debug_bounds("popover-trigger") + .expect("trigger rendered"); + let panel = visual + .debug_bounds("popover-panel") + .expect("panel rendered"); + + let expected = cx.update(|cx| Theme::global(cx).spacing.unit); + let gap = panel.top() - trigger.bottom(); + // Painted bounds are snapped to device pixels; allow half a pixel. + assert!( + (gap - expected).abs() <= px(0.5), + "the anchor gap is {gap:?}, expected {expected:?}" + ); +} diff --git a/crates/theme/src/lib.rs b/crates/theme/src/lib.rs index 26f8eb9..8681693 100644 --- a/crates/theme/src/lib.rs +++ b/crates/theme/src/lib.rs @@ -189,6 +189,13 @@ pub struct Elevation { } impl Elevation { + /// No shadow: dark floating layers separate by border. + fn none() -> Self { + Self { + layers: [Shadow::none(), Shadow::none()], + } + } + fn single(key: Shadow) -> Self { Self { layers: [key, Shadow::none()], @@ -232,8 +239,8 @@ impl Elevations { pub(crate) fn for_platform(source: SourceKind, variant: ThemeVariant) -> Self { if variant == ThemeVariant::Dark { return Self { - popover: Elevation::single(Shadow::black(0., 4., 24., 0.3)), - dialog: Elevation::single(Shadow::black(0., 8., 40., 0.35)), + popover: Elevation::none(), + dialog: Elevation::none(), control: Elevation::single(Shadow::black(0., 1., 2., 0.25)), }; } diff --git a/crates/theme/src/resolve.rs b/crates/theme/src/resolve.rs index 035d5b2..7811e16 100644 --- a/crates/theme/src/resolve.rs +++ b/crates/theme/src/resolve.rs @@ -545,6 +545,8 @@ mod tests { resolved.borders.overlays, "{source:?} frames floating layers in dark" ); + assert!(resolved.elevations.popover.shadows().is_empty()); + assert!(resolved.elevations.dialog.shadows().is_empty()); } } @@ -578,7 +580,7 @@ mod tests { #[test] fn elevations_and_motion_resolve_with_defaults() { let resolved = resolve( - &system(ThemeVariant::Dark, PartialPalette::default(), None), + &system(ThemeVariant::Light, PartialPalette::default(), None), None, None, ); @@ -590,7 +592,7 @@ mod tests { ); assert!(resolved.motion.overlay > std::time::Duration::ZERO); let shadows = resolved.elevations.popover.shadows(); - assert_eq!(shadows.len(), 1); + assert_eq!(shadows.len(), 2); assert!(!shadows[0].inset); } diff --git a/docs/CHECKLIST.md b/docs/CHECKLIST.md index 9fa77e1..f980d0a 100644 --- a/docs/CHECKLIST.md +++ b/docs/CHECKLIST.md @@ -14,9 +14,6 @@ is the first downstream consumer; its screens drive the order. Platform metric adoption continues one component at a time; each stop goes through the review gate and a gallery pass. -- [ ] **Popover platform chrome** — overlay border gate; the anchor gap - through `Theme::spacing`. -- [ ] **Dialog platform chrome** — overlay border gate. - [ ] **Toast platform chrome** — overlay border gate. - [ ] **Tooltip platform chrome** — overlay border gate. - [ ] **Alert platform chrome** — overlay border gate. @@ -69,9 +66,9 @@ Gaps surfaced by the signed comparison; ordered by signed's usage. (GNOME gsettings `font-name`, KDE `kdeglobals font=`), per-source border + focus chrome (`Theme::borders`), and a per-platform spacing unit (`Theme::spacing`); overlay borders engage in dark mode, where - the elevation shadow barely reads and floating layers separate by - border instead. Components adopt the tokens one by one - (see Next and Queue) + floating layers separate by border instead of shadow — dark elevation + shadows are dropped entirely, since they only muddied the dark scrim. + Components adopt the tokens one by one (see Next and Queue) ### icons (`crates/icons`) @@ -111,6 +108,13 @@ Gaps surfaced by the signed comparison; ordered by signed's usage. `Theme::spacing`, and every action row renders at the stated `menu_item_height` with content centered, so the stated metric is the whole row; rows carry debug selectors for headless geometry tests +- [x] Popover platform chrome — the panel border behind + `Theme::borders.overlays` (and dark mode); the anchor gap is the + spacing unit and rides `anchored().offset`, not a margin, so the + window-edge clamp stays intact; the panel carries a debug selector +- [x] Dialog platform chrome — the panel border behind + `Theme::borders.overlays` (and dark mode), taking the surface color + where the platform paints none - [x] Button, Checkbox, Switch, Radio + RadioGroup, Toggle, Link, Slider, Progress, Avatar (image + seeded pixel-art fallback) - [x] Spinner — painted ring + rotating-icon variant, indeterminate and -- 2.54.0 From 65cae95851421674b23a02f790dd3f627aedced9 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sat, 7 Nov 2026 20:25:14 +0700 Subject: [PATCH 08/14] audit toast --- crates/core/src/toast.rs | 45 ++++++++++++++++++---------------------- docs/CHECKLIST.md | 5 ++++- 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/crates/core/src/toast.rs b/crates/core/src/toast.rs index ff0595c..c0e8045 100644 --- a/crates/core/src/toast.rs +++ b/crates/core/src/toast.rs @@ -12,7 +12,8 @@ use gpui::{ use gpui_native_icons::{IconRole, icon}; use gpui_native_theme::ActiveTheme as _; -use crate::common::DismissHandler; +use crate::button::{Button, ButtonVariant}; +use crate::common::{DismissHandler, Sizable as _}; /// The action handler of a [`Toast`]. pub type ActionHandler = Rc; @@ -256,12 +257,13 @@ impl RenderOnce for ToastsView { fn pill(entry: &ToastEntry, toasts: &Entity, cx: &App) -> gpui::Stateful { let theme = cx.theme(); let surface = theme.colors.surface; - let border = theme.colors.border; + let border = if theme.borders.overlays { + theme.colors.border + } else { + surface + }; let foreground = theme.colors.foreground; let muted = theme.colors.muted; - let primary = theme.ramps.primary.base; - let primary_hover = theme.ramps.primary.hover; - let primary_foreground = theme.colors.primary_foreground; let radius = theme.radius.full; let shadows = theme.elevations.popover.shadows(); let icon_size = theme.sizes.small.icon_size; @@ -304,25 +306,18 @@ fn pill(entry: &ToastEntry, toasts: &Entity, cx: &App) -> gpui::Stateful .when_some(toast.action.clone(), |pill, action| { let toasts = toasts.clone(); pill.child( - div() - .id(("toast-action", id)) - .flex_none() - .ml_1p5() - .cursor_pointer() - .px_2() - .py_0p5() - .rounded(radius) - .bg(primary) - .hover(|style| style.bg(primary_hover)) - .text_size(text_size) - .text_color(primary_foreground) - .child(action.label.clone()) - .on_click(move |_, _window, cx| { - (action.handler.clone())(cx); - toasts.update(cx, |toasts, cx| { - toasts.dismiss(id, cx); - }); - }), + div().flex_none().ml_2p5().child( + Button::new(("toast-action", id)) + .label(action.label.clone()) + .variant(ButtonVariant::Primary) + .small() + .on_click(move |_, _window, cx| { + (action.handler.clone())(cx); + toasts.update(cx, |toasts, cx| { + toasts.dismiss(id, cx); + }); + }), + ), ) }) .child( @@ -331,7 +326,7 @@ fn pill(entry: &ToastEntry, toasts: &Entity, cx: &App) -> gpui::Stateful .flex_none() .ml_4() .cursor_pointer() - .p_0p5() + .p_1() .rounded(theme.radius.sm) .hover(|style| style.bg(muted)) .child( diff --git a/docs/CHECKLIST.md b/docs/CHECKLIST.md index f980d0a..22a0dd7 100644 --- a/docs/CHECKLIST.md +++ b/docs/CHECKLIST.md @@ -14,7 +14,6 @@ is the first downstream consumer; its screens drive the order. Platform metric adoption continues one component at a time; each stop goes through the review gate and a gallery pass. -- [ ] **Toast platform chrome** — overlay border gate. - [ ] **Tooltip platform chrome** — overlay border gate. - [ ] **Alert platform chrome** — overlay border gate. - [ ] **Combobox platform chrome** — popup border behind @@ -115,6 +114,10 @@ Gaps surfaced by the signed comparison; ordered by signed's usage. - [x] Dialog platform chrome — the panel border behind `Theme::borders.overlays` (and dark mode), taking the surface color where the platform paints none +- [x] Toast platform chrome — the pill border behind + `Theme::borders.overlays` (and dark mode), taking the surface color + where the platform paints none; the pill rides the popover elevation, + so dark mode's no-shadow rule applies to it too - [x] Button, Checkbox, Switch, Radio + RadioGroup, Toggle, Link, Slider, Progress, Avatar (image + seeded pixel-art fallback) - [x] Spinner — painted ring + rotating-icon variant, indeterminate and -- 2.54.0 From eb2273f87edb8ab50dd628fbd3c5a7707db813f8 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sun, 8 Nov 2026 09:02:49 +0700 Subject: [PATCH 09/14] audit tooltip --- crates/core/src/button.rs | 11 +++- crates/core/src/tooltip.rs | 116 +++++++++++++++++++++++++++++----- crates/core/tests/tooltip.rs | 119 ++++++++++++++++++++++++++++++++--- crates/utils/src/lib.rs | 41 +++++++++++- docs/CHECKLIST.md | 11 +++- examples/gallery/src/main.rs | 28 ++++++--- 6 files changed, 289 insertions(+), 37 deletions(-) diff --git a/crates/core/src/button.rs b/crates/core/src/button.rs index 619a0d2..524ceef 100644 --- a/crates/core/src/button.rs +++ b/crates/core/src/button.rs @@ -3,12 +3,12 @@ use gpui::prelude::FluentBuilder as _; use gpui::{ AnyView, App, ClickEvent, ElementId, InteractiveElement as _, IntoElement, ParentElement as _, - RenderOnce, Role, SharedString, StatefulInteractiveElement as _, Styled as _, Window, hsla, - relative, + RenderOnce, Role, SharedString, StatefulInteractiveElement as _, Styled as _, Window, canvas, + hsla, relative, }; use gpui_native_icons::{IconRole, icon}; use gpui_native_theme::{ActiveTheme as _, BORDER_WIDTH, Size}; -use gpui_native_utils::focus_ring; +use gpui_native_utils::{focus_ring, record_anchor_bounds}; use std::rc::Rc; use crate::common::{Sizable, h_flex}; @@ -241,6 +241,11 @@ impl RenderOnce for Button { }) }) .when_some(self.label, |button, label| button.child(label)) + .child( + canvas(record_anchor_bounds(self.id.clone()), |_, _, _, _| {}) + .absolute() + .inset_0(), + ) .when(self.disabled, |button| button.opacity(0.5)) .when_some(self.tooltip, |button, tooltip| { button.tooltip(move |window, cx| tooltip(window, cx)) diff --git a/crates/core/src/tooltip.rs b/crates/core/src/tooltip.rs index 321081f..31c1e2b 100644 --- a/crates/core/src/tooltip.rs +++ b/crates/core/src/tooltip.rs @@ -1,13 +1,17 @@ use gpui::AppContext as _; use gpui::{ - AnyView, App, Context, Entity, IntoElement, ParentElement as _, Render, SharedString, - Styled as _, Window, div, px, + AnyView, App, Bounds, Context, ElementId, Entity, InteractiveElement as _, IntoElement, + ParentElement as _, Pixels, Render, SharedString, Styled as _, Window, canvas, div, point, px, }; -use gpui_native_theme::ActiveTheme as _; +use gpui_native_theme::{ActiveTheme as _, BORDER_WIDTH}; +use gpui_native_utils::{anchor_bounds, record_anchor_bounds}; + +const PANEL_BOUNDS_ID: &str = "tooltip-panel-bounds"; /// A small text panel gpui shows next to a hovered element. pub struct Tooltip { text: SharedString, + anchor: Option>, } impl Tooltip { @@ -16,7 +20,28 @@ impl Tooltip { where L: Into, { - cx.new(|_| Self { text: text.into() }) + cx.new(|_| Self { + text: text.into(), + anchor: None, + }) + } + + /// Sets the anchor element ID for the tooltip. + pub fn anchored(text: L, anchor_id: I) -> impl Fn(&mut Window, &mut App) -> AnyView + where + L: Into, + I: Into, + { + let text = text.into(); + let anchor_id = anchor_id.into(); + move |_window, cx| { + let anchor = anchor_bounds(&anchor_id, cx); + cx.new(|_| Self { + text: text.clone(), + anchor, + }) + .into() + } } /// The `.tooltip(...)` closure for a plain text tooltip. @@ -30,20 +55,79 @@ impl Tooltip { } impl Render for Tooltip { - fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { + // Copied out: the mutable reads below need `cx` free of this borrow. let theme = cx.theme(); let text_size = theme.sizes.small.text_size; + let spacing_unit = theme.spacing.unit; + let surface = theme.colors.surface; + let surface_foreground = theme.colors.surface_foreground; + let radius_sm = theme.radius.sm; + let shadows = theme.elevations.popover.shadows(); - div() - .px_2() - .py_1() - .max_w(px(360.)) - .bg(theme.colors.surface) - .border_1() - .border_color(theme.colors.border) - .rounded(theme.radius.sm) - .text_size(text_size) - .text_color(theme.colors.surface_foreground) - .child(self.text.clone()) + let border = if theme.borders.overlays { + theme.colors.border + } else { + theme.colors.surface + }; + + let build_panel = || { + div() + .debug_selector(|| "tooltip-panel".to_string()) + .px_2() + .py_1() + .max_w(px(360.)) + .bg(surface) + .border_1() + .border_color(border) + .rounded(radius_sm) + .shadow(shadows.clone()) + .text_size(text_size) + .text_color(surface_foreground) + .child(self.text.clone()) + }; + + let (wrapper, panel) = match self.anchor { + Some(parent) => { + let origin = window.mouse_position() + point(px(1.), px(1.)); + let viewport_height = window.viewport_size().height; + + let height = anchor_bounds(&ElementId::from(PANEL_BOUNDS_ID), cx) + .map(|bounds| bounds.size.height) + .unwrap_or_else(|| { + text_size * 1.5 + window.rem_size() * 0.5 + BORDER_WIDTH * 2.0 + }); + + let below = viewport_height - spacing_unit - parent.bottom(); + let above = parent.top() - spacing_unit; + let hang_below = below >= height || below >= above; + + let panel = build_panel() + .ml(parent.left() - origin.x) + .mt(if hang_below { + parent.bottom() + spacing_unit - origin.y + } else { + parent.top() - spacing_unit - height - origin.y + }) + .child( + canvas( + record_anchor_bounds(ElementId::from(PANEL_BOUNDS_ID)), + |_, _, _, _| {}, + ) + .absolute() + .inset_0(), + ); + (div().debug_selector(|| "tooltip-anchor".to_string()), panel) + } + None => { + let wrapper = div() + .debug_selector(|| "tooltip-anchor".to_string()) + .pl(spacing_unit * 2.0) + .pt(spacing_unit * 2.0); + (wrapper, build_panel()) + } + }; + + wrapper.child(panel) } } diff --git a/crates/core/tests/tooltip.rs b/crates/core/tests/tooltip.rs index 036647b..5cd943d 100644 --- a/crates/core/tests/tooltip.rs +++ b/crates/core/tests/tooltip.rs @@ -1,17 +1,18 @@ -use std::cell::RefCell; -use std::rc::Rc; +use std::{cell::RefCell, rc::Rc}; use gpui::{ Context, InteractiveElement as _, IntoElement, Modifiers, MouseMoveEvent, ParentElement as _, Render, StatefulInteractiveElement as _, Styled as _, TestAppContext, VisualTestContext, - Window, div, point, px, + Window, WindowHandle, canvas, div, point, px, }; use gpui_native_core::button::Button; use gpui_native_core::tooltip::Tooltip; use gpui_native_theme::Theme; +use gpui_native_utils::record_anchor_bounds; /// Hover target at the top-left with a tooltip whose builder records every -/// invocation. +/// invocation. The tooltip anchors to the target itself, so it hangs below +/// the target's bottom edge instead of at the cursor. struct Harness { calls: Rc>>, } @@ -21,20 +22,26 @@ impl Render for Harness { let calls = self.calls.clone(); let target = div() .id("hover-target") + .debug_selector(|| "hover-target".to_string()) .w(px(100.)) .h(px(40.)) .bg(gpui::black()) - .tooltip(move |_, cx| { + .child( + canvas(record_anchor_bounds("hover-target".into()), |_, _, _, _| {}) + .absolute() + .inset_0(), + ) + .tooltip(move |window, cx| { calls.borrow_mut().push("target"); - Tooltip::new("Target tooltip", cx).into() + Tooltip::anchored("Target tooltip", "hover-target")(window, cx) }); let button_calls = self.calls.clone(); let button = Button::new("hover-button") .label("Hover me") - .tooltip(move |_, cx| { + .tooltip(move |window, cx| { button_calls.borrow_mut().push("button"); - Tooltip::new("Button tooltip", cx).into() + Tooltip::anchored("Button tooltip", "hover-button")(window, cx) }); div() @@ -80,6 +87,40 @@ fn draw(visual: &mut VisualTestContext) { }); } +struct LowHarness; + +impl Render for LowHarness { + fn render(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { + // The target is pinned to the window's bottom edge: the space below it + // is nil, so the tooltip must flip above the parent. + div().size_full().flex().flex_col().justify_end().child( + div() + .id("low-target") + .debug_selector(|| "low-target".to_string()) + .w(px(100.)) + .h(px(40.)) + .bg(gpui::black()) + .child( + canvas(record_anchor_bounds("low-target".into()), |_, _, _, _| {}) + .absolute() + .inset_0(), + ) + .tooltip(Tooltip::anchored("Low tooltip", "low-target")), + ) + } +} + +fn low_tooltip_window() -> (TestAppContext, VisualTestContext, WindowHandle) { + let mut cx = TestAppContext::single(); + cx.update(Theme::init); + let window = cx.add_window(move |_, _| LowHarness); + let mut visual = VisualTestContext::from_window(window.into(), &cx); + visual.update(|window, cx| { + window.draw(cx).clear(cx); + }); + (cx, visual, window) +} + #[test] fn hovering_the_target_deploys_the_tooltip_after_the_delay() { let (cx, mut visual, calls) = tooltip_window(); @@ -118,3 +159,65 @@ fn the_button_passthrough_deploys_the_tooltip() { draw(&mut visual); assert_eq!(*calls.borrow(), vec!["button"]); } + +/// An anchored tooltip hangs below the hovered parent's bottom edge with the +/// spacing gap: it never covers the parent, wherever the cursor is inside it. +#[test] +fn the_anchored_panel_hangs_below_the_parent_without_overlapping() { + let (cx, mut visual, _calls) = tooltip_window(); + hover(&mut visual, point(px(50.), px(20.))); + cx.executor().advance_clock(SHOW_DELAY); + visual.run_until_parked(); + draw(&mut visual); + + let target = visual + .debug_bounds("hover-target") + .expect("target laid out"); + let panel = visual + .debug_bounds("tooltip-panel") + .expect("panel rendered"); + + let unit = cx.update(|cx| Theme::global(cx).spacing.unit); + assert!( + (panel.top() - target.bottom() - unit).abs() <= px(0.5), + "the panel hangs {gap:?} below the target", + gap = panel.top() - target.bottom() + ); + assert!((panel.left() - target.left()).abs() <= px(0.5)); + assert!( + panel.top() >= target.bottom(), + "the panel must never overlap the hovered parent" + ); +} + +/// A parent pinned to the window's bottom edge leaves no room below: the +/// panel flips above the parent's top edge instead. +#[test] +fn an_anchored_tooltip_flips_above_a_low_parent() { + let (mut cx, mut visual, window) = low_tooltip_window(); + + let window_height = window + .update(&mut cx, |_, window, _cx| window.viewport_size().height) + .expect("window alive"); + hover(&mut visual, point(px(50.), window_height - px(20.))); + cx.executor().advance_clock(SHOW_DELAY); + visual.run_until_parked(); + draw(&mut visual); + + let target = visual.debug_bounds("low-target").expect("target laid out"); + let panel = visual + .debug_bounds("tooltip-panel") + .expect("panel rendered"); + + // The flip hangs the panel above the parent's top edge. + assert!( + panel.bottom() <= target.top(), + "the panel must not cover the parent: panel bottom {:?}, parent top {:?}", + panel.bottom(), + target.top() + ); + assert!( + panel.top() >= px(0.), + "the flipped panel must stay inside the window" + ); +} diff --git a/crates/utils/src/lib.rs b/crates/utils/src/lib.rs index 880ff50..fd350ea 100644 --- a/crates/utils/src/lib.rs +++ b/crates/utils/src/lib.rs @@ -1,9 +1,10 @@ use gpui::prelude::FluentBuilder as _; use gpui::{ - App, BoxShadow, Div, ElementId, FocusHandle, Hsla, IntoElement, Refineable as _, - StatefulInteractiveElement, StyleRefinement, Styled, Window, div, px, + App, Bounds, BoxShadow, Div, ElementId, FocusHandle, Global, Hsla, IntoElement, Pixels, + Refineable as _, StatefulInteractiveElement, StyleRefinement, Styled, Window, div, px, }; use gpui_native_theme::ActiveTheme as _; +use std::collections::HashMap; /// A row whose children center on the cross axis. pub fn h_flex() -> Div { @@ -35,6 +36,42 @@ pub fn focus_handle(id: &ElementId, window: &mut Window, cx: &mut App) -> FocusH .clone() } +/// The painted bounds of elements that tooltips anchor to, keyed by element id. +#[derive(Clone, Default)] +struct AnchorBoundsRegistry(HashMap>); + +impl Global for AnchorBoundsRegistry {} + +impl AnchorBoundsRegistry { + fn record(&mut self, id: ElementId, bounds: Bounds) { + self.0.insert(id, bounds); + } + + fn get(&self, id: &ElementId) -> Option> { + self.0.get(id).copied() + } +} + +/// The last bounds an element painted at. +pub fn anchor_bounds(id: &ElementId, cx: &mut App) -> Option> { + cx.try_global::() + .and_then(|registry| registry.get(id)) +} + +/// A canvas prepaint closure recording the element's painted bounds. +pub fn record_anchor_bounds( + id: ElementId, +) -> impl Fn(Bounds, &mut Window, &mut App) + 'static { + move |bounds, _window, cx| { + let mut registry = cx + .try_global::() + .cloned() + .unwrap_or_default(); + registry.record(id.clone(), bounds); + cx.set_global(registry); + } +} + /// Returns a focus ring shadow with the given color at half ink. pub fn focus_ring(color: Hsla) -> Vec { vec![BoxShadow::new(px(0.), px(0.), color.alpha(0.5)).spread_radius(px(3.))] diff --git a/docs/CHECKLIST.md b/docs/CHECKLIST.md index 22a0dd7..41ce6e9 100644 --- a/docs/CHECKLIST.md +++ b/docs/CHECKLIST.md @@ -14,7 +14,6 @@ is the first downstream consumer; its screens drive the order. Platform metric adoption continues one component at a time; each stop goes through the review gate and a gallery pass. -- [ ] **Tooltip platform chrome** — overlay border gate. - [ ] **Alert platform chrome** — overlay border gate. - [ ] **Combobox platform chrome** — popup border behind `Theme::borders.overlays`; the trigger follows the input stop's field @@ -118,6 +117,16 @@ Gaps surfaced by the signed comparison; ordered by signed's usage. `Theme::borders.overlays` (and dark mode), taking the surface color where the platform paints none; the pill rides the popover elevation, so dark mode's no-shadow rule applies to it too +- [x] Tooltip platform chrome — the panel border behind + `Theme::borders.overlays` (and dark mode), taking the surface color + where the platform paints none, and riding the popover elevation for + the light-mode shadow; anchored tooltips hang below the + hovered parent (`Tooltip::anchored(text, id)`) so they never cover it — + the element records its painted bounds with `record_anchor_bounds` + (`Button` does), the panel hangs below that with the spacing gap, and + it flips above the parent when it would overflow the window bottom + (the panel records its own height after the first frame); unanchored + tooltips fall back to the cursor-offset wrapper - [x] Button, Checkbox, Switch, Radio + RadioGroup, Toggle, Link, Slider, Progress, Avatar (image + seeded pixel-art fallback) - [x] Spinner — painted ring + rotating-icon variant, indeterminate and diff --git a/examples/gallery/src/main.rs b/examples/gallery/src/main.rs index de91736..8b0418a 100644 --- a/examples/gallery/src/main.rs +++ b/examples/gallery/src/main.rs @@ -876,7 +876,9 @@ impl Render for ButtonDemo { }); }); if variant == ButtonVariant::Destructive { - button = button.tooltip(Tooltip::text("Removes the selected item")); + let id = SharedString::from(format!("button-demo-{name}")); + button = button + .tooltip(Tooltip::anchored("Removes the selected item", id)); } button })), @@ -901,7 +903,9 @@ impl Render for ButtonDemo { }); }); if variant == ButtonVariant::Destructive { - button = button.tooltip(Tooltip::text("Removes the selected item")); + let id = SharedString::from(format!("button-demo-icon-{name}")); + button = button + .tooltip(Tooltip::anchored("Removes the selected item", id)); } button })), @@ -914,11 +918,12 @@ impl Render for ButtonDemo { .gap_2() .children(variants.map(|(name, variant)| { let view = view.clone(); - Button::new(SharedString::from(format!("button-demo-icon-only-{name}"))) + let id = SharedString::from(format!("button-demo-icon-only-{name}")); + Button::new(id.clone()) .icon(IconRole::Folder) .variant(variant) .accessibility_label("Open the selected file") - .tooltip(Tooltip::text("Opens the selected file")) + .tooltip(Tooltip::anchored("Opens the selected file", id)) .on_click(move |_, _, cx| { view.update(cx, |demo, cx| { demo.clicks += 1; @@ -2800,20 +2805,29 @@ impl Render for TooltipDemo { Button::new("tooltip-demo-destructive") .label("Delete") .variant(ButtonVariant::Destructive) - .tooltip(Tooltip::text("Removes the selected item")), + .tooltip(Tooltip::anchored( + "Removes the selected item", + "tooltip-demo-destructive", + )), ) .child( Button::new("tooltip-demo-icon") .label("Open") .icon(IconRole::Folder) - .tooltip(Tooltip::text("Opens the selected file")), + .tooltip(Tooltip::anchored( + "Opens the selected file", + "tooltip-demo-icon", + )), ) .child( Button::new("tooltip-demo-icon-only") .icon(IconRole::Folder) .variant(ButtonVariant::Ghost) .accessibility_label("Open the selected file") - .tooltip(Tooltip::text("Opens the selected file")), + .tooltip(Tooltip::anchored( + "Opens the selected file", + "tooltip-demo-icon-only", + )), ), )) } -- 2.54.0 From 21d32736a86a4835e6abec37471a267d08dc4aa8 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sun, 8 Nov 2026 09:07:23 +0700 Subject: [PATCH 10/14] audit alert --- crates/core/src/alert.rs | 10 ++++++++-- docs/CHECKLIST.md | 5 ++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/crates/core/src/alert.rs b/crates/core/src/alert.rs index 8b2d19b..57eb08c 100644 --- a/crates/core/src/alert.rs +++ b/crates/core/src/alert.rs @@ -126,7 +126,13 @@ impl RenderOnce for Alert { let muted_foreground = theme.colors.muted_foreground; let muted = theme.colors.muted; let surface = theme.colors.surface; - let border = theme.colors.border; + + let border = if theme.borders.overlays { + theme.colors.border + } else { + surface + }; + let shadows = theme.elevations.popover.shadows(); let metrics = theme.sizes.resolve(Size::Medium); let icon_size = metrics.icon_size; @@ -190,7 +196,7 @@ impl RenderOnce for Alert { .id((self.id, "alert-close")) .flex_none() .cursor_pointer() - .p_0p5() + .p_1() .rounded(theme.radius.sm) .hover(|style| style.bg(muted)) .child( diff --git a/docs/CHECKLIST.md b/docs/CHECKLIST.md index 41ce6e9..1192583 100644 --- a/docs/CHECKLIST.md +++ b/docs/CHECKLIST.md @@ -14,7 +14,6 @@ is the first downstream consumer; its screens drive the order. Platform metric adoption continues one component at a time; each stop goes through the review gate and a gallery pass. -- [ ] **Alert platform chrome** — overlay border gate. - [ ] **Combobox platform chrome** — popup border behind `Theme::borders.overlays`; the trigger follows the input stop's field treatment. @@ -127,6 +126,10 @@ Gaps surfaced by the signed comparison; ordered by signed's usage. it flips above the parent when it would overflow the window bottom (the panel records its own height after the first frame); unanchored tooltips fall back to the cursor-offset wrapper +- [x] Alert platform chrome — the banner border behind + `Theme::borders.overlays` (and dark mode), taking the surface color + where the platform paints none; it rides the popover elevation, so + dark mode's no-shadow rule applies to it too - [x] Button, Checkbox, Switch, Radio + RadioGroup, Toggle, Link, Slider, Progress, Avatar (image + seeded pixel-art fallback) - [x] Spinner — painted ring + rotating-icon variant, indeterminate and -- 2.54.0 From abb56567e4355f07999e3617db3cc985545fba77 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sun, 8 Nov 2026 11:00:39 +0700 Subject: [PATCH 11/14] audit combobox --- crates/core/src/combobox.rs | 73 ++++++- crates/core/src/input.rs | 362 +++++++++++++------------------ crates/core/tests/combobox.rs | 29 ++- crates/core/tests/field_bench.rs | 29 ++- crates/core/tests/input.rs | 51 +++-- docs/CHECKLIST.md | 23 +- examples/gallery/src/main.rs | 5 +- 7 files changed, 315 insertions(+), 257 deletions(-) diff --git a/crates/core/src/combobox.rs b/crates/core/src/combobox.rs index bbf50d6..2e19081 100644 --- a/crates/core/src/combobox.rs +++ b/crates/core/src/combobox.rs @@ -12,13 +12,13 @@ use gpui::prelude::FluentBuilder as _; use gpui::{ App, Bounds, Context, ElementId, Entity, EventEmitter, InteractiveElement as _, IntoElement, KeyBinding, MouseButton, MouseDownEvent, ParentElement as _, Pixels, Render, Role, - ScrollHandle, SharedString, StatefulInteractiveElement as _, Styled as _, Subscription, Task, - Window, actions, canvas, deferred, div, px, + ScrollHandle, SharedString, StatefulInteractiveElement as _, StyleRefinement, Styled, + Subscription, Task, Window, actions, anchored, canvas, deferred, div, point, px, }; use gpui_native_icons::{IconRole, icon}; -use gpui_native_theme::{ActiveTheme as _, Size}; +use gpui_native_theme::{ActiveTheme as _, BORDER_WIDTH, Size}; -use crate::common::{Sizable, h_flex, v_flex}; +use crate::common::{Sizable, StyledExt as _, h_flex, v_flex}; use crate::input::{Input, InputState, InputStateEvent}; use crate::scroll_area::ScrollArea; @@ -81,6 +81,7 @@ where filter: Option>, unmatched: UnmatchedText, size: Size, + style: StyleRefinement, } impl Combobox @@ -103,6 +104,7 @@ where filter: None, unmatched: UnmatchedText::default(), size: Size::default(), + style: StyleRefinement::default(), } } @@ -165,6 +167,16 @@ where } } +/// Style refinements set the combobox's width. +impl Styled for Combobox +where + T: Clone + PartialEq + 'static, +{ + fn style(&mut self) -> &mut StyleRefinement { + &mut self.style + } +} + /// Emitted when the combobox's value changes. pub struct ComboboxChanged; @@ -189,6 +201,8 @@ where filter: ComboboxFilter, unmatched: UnmatchedText, size: Size, + /// Width refinements from the builder, applied to the wrapper each render. + style: StyleRefinement, /// Handler reports in flight; held so they die with the state. tasks: Vec>, /// Keeps the query-event subscription tied to the state's lifetime. @@ -225,6 +239,7 @@ where let subscription = cx.subscribe_in(&query, window, |state, _, event, window, cx| match event { + InputStateEvent::Focused => state.query_focused(window, cx), InputStateEvent::TextChanged => state.text_changed(window, cx), InputStateEvent::Blurred => state.blurred(window, cx), _ => {} @@ -248,6 +263,7 @@ where }), unmatched: combobox.unmatched, size: combobox.size, + style: combobox.style, tasks: Vec::new(), _subscription: subscription, } @@ -364,6 +380,16 @@ where cx.notify(); } + /// Gaining focus. + fn query_focused(&mut self, _window: &mut Window, cx: &mut Context) { + if self.selected.is_some() { + return; + } + self.refilter(""); + self.highlight = (!self.visible.is_empty()).then_some(0usize); + cx.notify(); + } + /// Losing focus. [`UnmatchedText`] decides what the text becomes. fn blurred(&mut self, window: &mut Window, cx: &mut Context) { self.highlight = None; @@ -447,11 +473,25 @@ where T: Clone + PartialEq + 'static, { fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { - let (metrics, colors, radius) = { + let (metrics, colors, radius, panel_border, spacing_unit, shadows) = { let theme = cx.theme(); let metrics = theme.sizes.resolve(self.size); - (metrics, theme.colors, theme.radius) + + let panel_border = if theme.borders.overlays { + theme.colors.border + } else { + theme.colors.surface + }; + ( + metrics, + theme.colors, + theme.radius, + panel_border, + theme.spacing.unit, + theme.elevations.popover.shadows(), + ) }; + let icon_size = metrics.dense_icon_size; let id = self.id.clone(); let state = cx.entity(); @@ -490,7 +530,9 @@ where .id(id.clone()) .key_context(COMBOBOX_CONTEXT) .relative() - .w_full() + .w(window.rem_size() * 12.0) + .min_w(window.rem_size() * 6.0) + .refine_style(&self.style) .role(Role::ComboBox) .aria_expanded(open) .aria_label(self.name.clone()) @@ -532,13 +574,14 @@ where .child( Input::new((id.clone(), "query"), self.query.clone()) .placeholder(self.placeholder.clone()) + .placeholder_when_focused(true) .with_size(self.size), ) .child( h_flex() .absolute() .right(metrics.dense_padding_x) - .top((metrics.input_height - icon_size) / 2.) + .top((metrics.input_height + BORDER_WIDTH * 2.0 - icon_size) / 2.) .child(chevron), ); @@ -626,6 +669,10 @@ where }; let panel_bounds: Rc>>> = Rc::new(RefCell::new(None)); + let popup_width = wrapper_bounds + .borrow() + .map(|bounds| bounds.size.width) + .unwrap_or_else(|| window.rem_size() * 12.0); let panel = v_flex() .id((id.clone(), "panel")) @@ -633,14 +680,14 @@ where let name = format!("{id}-panel"); move || name }) - .mt_1() - .w_full() + .w(popup_width) .p_1() .bg(colors.surface) .text_color(colors.foreground) .rounded(radius.overlay) .border_1() - .border_color(colors.border) + .border_color(panel_border) + .shadow(shadows) .overflow_hidden() .child(rows) .child( @@ -684,7 +731,9 @@ where .inset_0(), ); - wrapper = wrapper.child(deferred(panel)); + wrapper = wrapper.child(deferred( + anchored().offset(point(px(0.), spacing_unit)).child(panel), + )); } wrapper diff --git a/crates/core/src/input.rs b/crates/core/src/input.rs index 2230852..b9c7015 100644 --- a/crates/core/src/input.rs +++ b/crates/core/src/input.rs @@ -7,13 +7,13 @@ use unicode_segmentation::UnicodeSegmentation; use gpui::prelude::FluentBuilder as _; use gpui::{ - AnyElement, App, AppContext as _, Bounds, ClipboardItem, ContentMask, Context, CursorStyle, - DefiniteLength, DispatchPhase, ElementId, Entity, EntityInputHandler, EventEmitter, Font, - HitboxBehavior, InteractiveElement as _, IntoElement, KeyBinding, KeyDownEvent, MouseButton, - MouseDownEvent, MouseMoveEvent, MouseUpEvent, ParentElement as _, Pixels, RenderOnce, Role, - ScrollDelta, ScrollEvent, SharedString, StatefulInteractiveElement as _, Styled as _, - Subscription, Task, TextAlign, TextRun, UTF16Selection, Window, WrappedLine, actions, canvas, - div, fill, point, px, + Animation, AnimationExt as _, AnyElement, App, AppContext as _, Bounds, ClipboardItem, + ContentMask, Context, CursorStyle, DefiniteLength, DispatchPhase, ElementId, Entity, + EntityInputHandler, EventEmitter, Font, HitboxBehavior, InteractiveElement as _, IntoElement, + KeyBinding, KeyDownEvent, MouseButton, MouseDownEvent, MouseMoveEvent, MouseUpEvent, + ParentElement as _, Pixels, RenderOnce, Role, ScrollDelta, ScrollEvent, SharedString, + StatefulInteractiveElement as _, Styled as _, TextAlign, TextRun, UTF16Selection, Window, + WrappedLine, actions, canvas, div, fill, point, px, }; use gpui_native_icons::IconRole; use gpui_native_theme::{ActiveTheme as _, BORDER_WIDTH, ColorTokens, Size}; @@ -478,97 +478,6 @@ impl HistoryEntry { } } -/// Caret blink controller. -struct CaretBlink { - interval: Duration, - generation: usize, - visible: bool, - active: bool, - paused: bool, - /// Pending blink timers; held so they die with the controller. - tasks: Vec>, -} - -impl CaretBlink { - fn new(_cx: &mut Context) -> Self { - Self { - interval: CARET_BLINK_INTERVAL, - generation: 0, - visible: true, - active: false, - paused: false, - tasks: Vec::new(), - } - } - - fn visible(&self) -> bool { - self.visible - } - - /// Starts blinking; the caret shows first and hides after one interval. - fn enable(&mut self, cx: &mut Context) { - if self.active { - return; - } - self.active = true; - self.visible = true; - self.paused = false; - self.schedule(cx); - } - - /// Stops blinking. The caret stays hidden; paint gates on focus anyway. - fn disable(&mut self, cx: &mut Context) { - self.active = false; - self.visible = false; - self.paused = false; - cx.notify(); - } - - /// Holds the caret visible and restarts the hide timer. - fn pause(&mut self, cx: &mut Context) { - if !self.active { - return; - } - if !self.visible { - self.visible = true; - cx.notify(); - } - self.paused = true; - self.generation += 1; - self.schedule_pause(cx); - } - - fn tick(&mut self, cx: &mut Context) { - if !self.active || self.paused { - return; - } - self.visible = !self.visible; - cx.notify(); - self.schedule(cx); - } - - fn schedule(&mut self, cx: &mut Context) { - self.generation += 1; - self.schedule_pause(cx); - } - - fn schedule_pause(&mut self, cx: &mut Context) { - let generation = self.generation; - let interval = self.interval; - let task = cx.spawn(async move |this, cx| { - cx.background_executor().timer(interval).await; - this.update(cx, |this, cx| { - if this.generation == generation { - this.paused = false; - this.tick(cx); - } - }) - .ok(); - }); - self.tasks.push(task); - } -} - /// The application-owned state of a text field. pub struct InputState { text: String, @@ -608,20 +517,18 @@ pub struct InputState { scroll_offset: gpui::Point, /// Set by every caret-affecting change. scroll_to_caret_pending: bool, - /// Drives the focused caret's blink. - caret_blink: Entity, - /// The shaped line and element bounds, recorded during each paint, - /// and shared with hit-testing and the IME geometry conversions. + /// Bumped on every edit. + caret_epoch: usize, + /// When the current caret epoch started. + caret_epoch_started: Option, + /// The shaped line and element bounds, recorded during each paint. layout: Rc>>, - /// Keeps the field repainting on blink ticks. Dropping it unsubscribes. - _caret_blink_subscription: Subscription, } impl EventEmitter for InputState {} impl InputState { fn new_inner(cx: &mut Context) -> Self { - let caret_blink = cx.new(CaretBlink::new); Self { text: String::new(), cursor: 0, @@ -651,9 +558,9 @@ impl InputState { goal_x: None, scroll_offset: point(Pixels::ZERO, Pixels::ZERO), scroll_to_caret_pending: true, - caret_blink: caret_blink.clone(), + caret_epoch: 0, + caret_epoch_started: Some(Instant::now()), layout: Rc::new(RefCell::new(None)), - _caret_blink_subscription: cx.observe(&caret_blink, |_, _, cx| cx.notify()), } } @@ -663,9 +570,6 @@ impl InputState { } /// Creates the state entity with initial text, caret at the end. - /// - /// Newlines become spaces; flip the field to multiline with [`set_multiline`] - /// before `set_value` to seed lines. pub fn new_with_text(text: impl Into, cx: &mut App) -> Entity { let text = sanitize(text.into(), false); cx.new(|cx| { @@ -683,10 +587,6 @@ impl InputState { } /// Replaces the whole value and collapses the selection to the end. - /// - /// Newlines become spaces unless the field is multiline. Emits - /// [`InputStateEvent::TextChanged`] when the value changed; setting the - /// current value is a no-op. pub fn set_value(&mut self, value: V, window: &mut Window, cx: &mut Context) where V: Into, @@ -868,7 +768,8 @@ impl InputState { cx.notify(); self.scroll_to_caret_pending = true; self.goal_x = None; - self.caret_blink.update(cx, |blink, cx| blink.pause(cx)); + self.caret_epoch += 1; + self.caret_epoch_started = Some(Instant::now()); window.notify_text_input_changed(&self.focus_handle); } @@ -901,10 +802,11 @@ impl InputState { } self.focused = focused; + // A fresh focus starts a fresh blink epoch, so the caret shows solid + // for one interval before blinking resumes. if focused { - self.caret_blink.update(cx, |blink, cx| blink.enable(cx)); - } else { - self.caret_blink.update(cx, |blink, cx| blink.disable(cx)); + self.caret_epoch += 1; + self.caret_epoch_started = Some(Instant::now()); } cx.emit(if focused { @@ -915,8 +817,22 @@ impl InputState { } /// Whether the caret should paint. - pub fn caret_visible(&mut self, focused: bool, cx: &mut Context) -> bool { - focused && !self.read_only && self.caret_blink.read(cx).visible() + pub fn caret_visible(&mut self, focused: bool) -> bool { + let elapsed = self + .caret_epoch_started + .map(|started| started.elapsed()) + .unwrap_or_default(); + self.caret_blink_visible_at(focused, elapsed) + } + + /// The visibility math at a given time since the epoch started. + pub fn caret_blink_visible_at(&self, focused: bool, elapsed: Duration) -> bool { + if !focused || self.read_only { + return false; + } + let period = CARET_BLINK_INTERVAL * 2; + (elapsed.as_millis() as u64 % period.as_millis() as u64) + < CARET_BLINK_INTERVAL.as_millis() as u64 } /// Replaces a byte range with `text` and collapses the caret to its end. @@ -1317,7 +1233,7 @@ impl InputState { } // A caret promises that typing lands there; in read-only it does not. - if focused && self.caret_visible(focused, cx) && self.selection().is_empty() { + if focused && self.caret_visible(focused) && self.selection().is_empty() { self.paint_caret(origin, font_size, colors.foreground, window); } @@ -1903,8 +1819,10 @@ pub struct Input { placeholder: SharedString, size: Size, disabled: bool, + /// The placeholder stays visible while focused. + placeholder_when_focused: bool, a11y_label: Option, - /// Multiline field height in rows; ignored when the state is single-line. + /// Multiline field height in row, ignored when the state is single-line. rows: usize, prefix: Option, suffix: Option, @@ -1922,6 +1840,7 @@ impl Input { placeholder: SharedString::default(), size: Size::default(), disabled: false, + placeholder_when_focused: false, a11y_label: None, rows: DEFAULT_ROWS, prefix: None, @@ -1944,6 +1863,12 @@ impl Input { self } + /// Shows the placeholder while the field is focused. + pub fn placeholder_when_focused(mut self, when_focused: bool) -> Self { + self.placeholder_when_focused = when_focused; + self + } + /// Renders the field inert, unfocusable, and dimmed; disabled always wins. pub fn disabled(mut self, disabled: bool) -> Self { self.disabled = disabled; @@ -1991,12 +1916,13 @@ impl RenderOnce for Input { let radius = theme.radius; let metrics = theme.sizes.resolve(self.size); let focus_handle = self.state.read(cx).focus_handle.clone(); - let focused = focus_handle.is_focused(window); + let is_empty = self.state.read(cx).text.is_empty(); let is_password = self.state.read(cx).is_password(); let revealed = self.state.read(cx).is_revealed(); let multiline = self.state.read(cx).is_multiline(); + let recording = focused && !self.disabled && !self.state.read(cx).is_read_only(); let small = theme.sizes.resolve(Size::Small); let docked_height = small.control_height + BORDER_WIDTH * 2.0; @@ -2041,6 +1967,97 @@ impl RenderOnce for Input { .child(element) }; + let canvas_element = { + let canvas = canvas( + { + let state = self.state.clone(); + move |bounds, window, cx| { + // The hitbox backs the I-beam hover cursor. + let hitbox = window.insert_hitbox(bounds, HitboxBehavior::Normal); + let wrap_width = multiline.then(|| { + (bounds.size.width - field_padding.left - field_padding.right) + .max(Pixels::ZERO) + }); + let text = state.read(cx).shape_current_text(wrap_width, window); + (text, hitbox) + } + }, + { + let state = self.state.clone(); + let focus = focus_handle.clone(); + let disabled = self.disabled; + + move |bounds, (text, hitbox), window, cx| { + if !disabled { + window.set_cursor_style(CursorStyle::IBeam, &hitbox); + } + + let dragging = state.update(cx, |state, cx| { + state.paint(text, bounds, field_padding, colors, window, cx) + }); + + if disabled { + return; + } + + window.handle_input( + &focus, + gpui::ElementInputHandler::new(bounds, state.clone()), + cx, + ); + + window.on_mouse_event({ + let state = state.clone(); + move |event: &ScrollEvent, phase, window, cx| { + if phase != DispatchPhase::Bubble + || !bounds.contains(&event.position) + { + return; + } + + let delta = match event.delta { + ScrollDelta::Pixels(delta) => delta, + ScrollDelta::Lines(lines) => point( + px(lines.x * SCROLL_LINES_HEIGHT), + px(lines.y * SCROLL_LINES_HEIGHT), + ), + }; + + let consumed = state + .update(cx, |state, cx| state.scroll_by(delta, window, cx)); + + if consumed { + cx.stop_propagation(); + } + } + }); + + if dragging { + InputState::register_drag_listeners(&state, window); + } + } + }, + ) + .absolute() + .inset_0(); + if recording { + let epoch = self.state.read(cx).caret_epoch; + let animation_id = ElementId::from(SharedString::from(format!( + "{self_id}-caret-blink-{epoch}", + self_id = self.id, + ))); + canvas + .with_animation( + animation_id, + Animation::new(CARET_BLINK_INTERVAL * 2).repeat(), + |canvas, _| canvas, + ) + .into_any_element() + } else { + canvas.into_any_element() + } + }; + h_flex() .id(self.id.clone()) .debug_selector(|| "input-field".to_string()) @@ -2098,93 +2115,18 @@ impl RenderOnce for Input { .when_some(line_height, |region, line_height| { region.items_start().line_height(line_height) }) - .when(is_empty && !focused, |region| { - region.child( - div() - .text_color(colors.muted_foreground) - .when(multiline, |placeholder| placeholder.pt(vertical_inset)) - .child(self.placeholder.clone()), - ) - }) - .child( - canvas( - { - let state = self.state.clone(); - move |bounds, window, cx| { - // The hitbox backs the I-beam hover cursor. - let hitbox = - window.insert_hitbox(bounds, HitboxBehavior::Normal); - let wrap_width = multiline.then(|| { - (bounds.size.width - - field_padding.left - - field_padding.right) - .max(Pixels::ZERO) - }); - let text = - state.read(cx).shape_current_text(wrap_width, window); - (text, hitbox) - } - }, - { - let state = self.state.clone(); - let focus = focus_handle.clone(); - let disabled = self.disabled; - - move |bounds, (text, hitbox), window, cx| { - if !disabled { - window.set_cursor_style(CursorStyle::IBeam, &hitbox); - } - - let dragging = state.update(cx, |state, cx| { - state.paint(text, bounds, field_padding, colors, window, cx) - }); - - if disabled { - return; - } - - window.handle_input( - &focus, - gpui::ElementInputHandler::new(bounds, state.clone()), - cx, - ); - - window.on_mouse_event({ - let state = state.clone(); - move |event: &ScrollEvent, phase, window, cx| { - if phase != DispatchPhase::Bubble - || !bounds.contains(&event.position) - { - return; - } - - let delta = match event.delta { - ScrollDelta::Pixels(delta) => delta, - ScrollDelta::Lines(lines) => point( - px(lines.x * SCROLL_LINES_HEIGHT), - px(lines.y * SCROLL_LINES_HEIGHT), - ), - }; - - let consumed = state.update(cx, |state, cx| { - state.scroll_by(delta, window, cx) - }); - - if consumed { - cx.stop_propagation(); - } - } - }); - - if dragging { - InputState::register_drag_listeners(&state, window); - } - } - }, - ) - .absolute() - .inset_0(), - ), + .when( + is_empty && (!focused || self.placeholder_when_focused), + |region| { + region.child( + div() + .text_color(colors.muted_foreground) + .when(multiline, |placeholder| placeholder.pt(vertical_inset)) + .child(self.placeholder.clone()), + ) + }, + ) + .child(canvas_element), ) .when_some(self.suffix, |input, suffix| { input.child(edge_zone("input-suffix", suffix)) diff --git a/crates/core/tests/combobox.rs b/crates/core/tests/combobox.rs index bea30c9..5518d8f 100644 --- a/crates/core/tests/combobox.rs +++ b/crates/core/tests/combobox.rs @@ -2,8 +2,8 @@ use std::cell::RefCell; use std::rc::Rc; use gpui::{ - AppContext as _, Context, Entity, IntoElement, ParentElement as _, Render, SharedString, - Styled as _, TestAppContext, VisualTestContext, Window, div, + AppContext as _, Context, Entity, IntoElement, MouseButton, ParentElement as _, Render, + SharedString, Styled as _, TestAppContext, VisualTestContext, Window, div, point, px, }; use gpui_native_core::combobox::{Combobox, ComboboxOption, ComboboxState}; use gpui_native_core::init; @@ -99,6 +99,31 @@ fn draw(visual: &mut VisualTestContext) { }); } +fn click(visual: &mut VisualTestContext, position: gpui::Point) { + visual.simulate_mouse_down(position, MouseButton::Left, gpui::Modifiers::default()); + visual.simulate_mouse_up(position, MouseButton::Left, gpui::Modifiers::default()); +} + +/// Clicking into a valueless combobox field opens the options right away: +/// no keystroke is needed to see the choices. +#[test] +fn clicking_the_empty_field_opens_the_popup() { + let mut host = combo_host(false, false); + + // Inside the field, which sits at the top of the padded host. + click(&mut host.visual, point(px(100.), px(27.))); + draw(&mut host.visual); + + assert!( + host.state.read_with(&host.cx, |state, _| state.is_open()), + "clicking the empty field should show the options" + ); + assert!( + host.visual.debug_bounds("test-combo-panel").is_some(), + "the options panel should be rendered" + ); +} + fn focus(host: &mut ComboHost) { let state = host.state.clone(); host.visual.update(|window, cx| { diff --git a/crates/core/tests/field_bench.rs b/crates/core/tests/field_bench.rs index b3a99c5..fd50560 100644 --- a/crates/core/tests/field_bench.rs +++ b/crates/core/tests/field_bench.rs @@ -38,8 +38,11 @@ impl Render for FieldHost { } } -/// A field page in a real window, with the url input focused. -fn field_host() -> ( +/// A field page in a real window; the url input is focused unless +/// `focus` is false, which leaves the page truly idle. +fn field_host( + focus: bool, +) -> ( TestAppContext, Entity, Rc>, @@ -64,8 +67,10 @@ fn field_host() -> ( }); let mut visual = VisualTestContext::from_window(window.into(), &cx); visual.update(|window, cx| { - let focus = url.read(cx).focus_handle(); - window.focus(&focus, cx); + if focus { + let focus = url.read(cx).focus_handle(); + window.focus(&focus, cx); + } window.draw(cx).clear(cx); }); (cx, url, renders, visual) @@ -94,17 +99,27 @@ fn type_and_time(visual: &mut VisualTestContext, text: &str) -> std::time::Durat #[test] fn an_idle_field_page_schedules_no_frames() { - let (_cx, _url, _renders, mut visual) = field_host(); + let (_cx, _url, _renders, mut visual) = field_host(false); let frames = visual.update(|window, cx| window.simulate_next_frame(cx)); assert_eq!( frames, 0, - "an idle field page must not request animation frames" + "an idle unfocused field page must not request animation frames" + ); +} + +#[test] +fn a_focused_field_page_schedules_frames_for_its_blink() { + let (_cx, _url, _renders, mut visual) = field_host(true); + let frames = visual.update(|window, cx| window.simulate_next_frame(cx)); + assert!( + frames > 0, + "the focused caret's blink needs frames to redraw at the blink cadence" ); } #[test] fn field_typing_frame_cost() { - let (_cx, _url, _renders, mut visual) = field_host(); + let (_cx, _url, _renders, mut visual) = field_host(true); let elapsed = type_and_time(&mut visual, "dsdsadsadadadada"); assert!( elapsed.as_millis() < 1000, diff --git a/crates/core/tests/input.rs b/crates/core/tests/input.rs index 7086659..ad7ba73 100644 --- a/crates/core/tests/input.rs +++ b/crates/core/tests/input.rs @@ -269,42 +269,45 @@ fn read_only_refuses_edits_but_keeps_selection_and_copy() { assert_eq!(clipboard, "lo"); // The caret is suppressed entirely while read-only. - let caret = cx.update(|cx| state.update(cx, |state, cx| state.caret_visible(true, cx))); + let caret = cx.update(|cx| state.update(cx, |state, _| state.caret_visible(true))); assert!(!caret); } #[test] -fn caret_blinks_while_focused_and_holds_solid_while_typing() { +fn the_caret_visibility_is_pure_time_math_against_the_epoch() { let (cx, state, mut visual) = input_window("", false); focus(&mut visual, &state); - visual.update(|window, cx| { - window.draw(cx).clear(cx); - }); - visual.run_until_parked(); + let caret = || cx.update(|cx| state.update(cx, |state, _| state.caret_visible(true))); - let caret = || cx.update(|cx| state.update(cx, |state, cx| state.caret_visible(true, cx))); - - // The caret shows on focus, hides after one interval, and reappears - // after another. - assert!(caret()); - cx.executor().advance_clock(Duration::from_millis(500)); - visual.run_until_parked(); - assert!(!caret()); - cx.executor().advance_clock(Duration::from_millis(500)); - visual.run_until_parked(); + // Fresh focus starts a new epoch: the caret shows solid — the elapsed + // time since the epoch began is microseconds, far under one interval. assert!(caret()); - // Typing holds the caret solid and restarts the cycle. + // Typing restarts the epoch, so the caret stays solid through edits. visual.simulate_keystrokes("a"); assert!(caret()); - cx.executor().advance_clock(Duration::from_millis(500)); - visual.run_until_parked(); - assert!(!caret()); - // An unfocused field reports no caret. - let caret_unfocused = - cx.update(|cx| state.update(cx, |state, cx| state.caret_visible(false, cx))); - assert!(!caret_unfocused); + // The blink cadence itself: hidden in the second half of the cycle. + let hidden = cx.update(|cx| { + state.update(cx, |state, _| { + state.caret_blink_visible_at(true, Duration::from_millis(700)) + }) + }); + assert!(!hidden); + let visible = cx.update(|cx| { + state.update(cx, |state, _| { + state.caret_blink_visible_at(true, Duration::from_millis(300)) + }) + }); + assert!(visible); + + // An unfocused field reports no caret, at any point in the cycle. + let unfocused = cx.update(|cx| { + state.update(cx, |state, _| { + state.caret_blink_visible_at(false, Duration::from_millis(100)) + }) + }); + assert!(!unfocused); } #[test] diff --git a/docs/CHECKLIST.md b/docs/CHECKLIST.md index 1192583..baec1a0 100644 --- a/docs/CHECKLIST.md +++ b/docs/CHECKLIST.md @@ -130,12 +130,33 @@ Gaps surfaced by the signed comparison; ordered by signed's usage. `Theme::borders.overlays` (and dark mode), taking the surface color where the platform paints none; it rides the popover elevation, so dark mode's no-shadow rule applies to it too +- [x] Combobox platform chrome — the popup border behind + `Theme::borders.overlays` (and dark mode), taking the surface color + where the platform paints none, and riding the popover elevation for + the light-mode shadow; the trigger is a plain `Input`, so it + already carries the full field treatment, and the chevron recenters on + the field's full height, border included; the popup mounts on + `deferred(anchored().offset(...))` like the Popover — out of the + wrapper's layout, one spacing gap below, window-clamped — and always + matches the combobox's width; the combobox itself is no longer full + width: it takes a compact default and style refinements on the builder + (`.w(...)`) set the width; the empty field keeps its placeholder while + focused (`Input::placeholder_when_focused`) and clicking into it opens + the options right away, with no keystroke needed +- [x] Combobox platform chrome — the popup border behind + `Theme::borders.overlays` (and dark mode), taking the surface color + where the platform paints none; the trigger is a plain `Input`, so it + already carries the full field treatment, and the chevron recenters on + the field's full height, border included - [x] Button, Checkbox, Switch, Radio + RadioGroup, Toggle, Link, Slider, Progress, Avatar (image + seeded pixel-art fallback) - [x] Spinner — painted ring + rotating-icon variant, indeterminate and determinate; replaces `Tab::busy`'s placeholder - [x] Input — single-line, password with reveal, multiline, undo/redo, actions - + key context, caret blink + + key context, caret blink (time-math driven: the focused field mounts a + repeating blink animation whose frames redraw the caret, the caret is + solid for one interval after each edit, and an idle unfocused page + schedules no frames) - [x] ScrollArea, Popover, Menu, Tooltip, Dialog / AlertDialog, Toast, Alert + alerts stack - [x] Select, Segmented, ContextMenu, EmptyState, Badge diff --git a/examples/gallery/src/main.rs b/examples/gallery/src/main.rs index 8b0418a..6744019 100644 --- a/examples/gallery/src/main.rs +++ b/examples/gallery/src/main.rs @@ -1257,7 +1257,10 @@ impl ComboboxDemo { ) .selected(SharedString::from("main")) .placeholder("Switch branch…") - .keep_unmatched_text(), + .keep_unmatched_text() + // Widths are style refinements on the builder; the popup + // always matches whatever the combobox ends up being. + .w(px(280.)), window, cx, ) -- 2.54.0 From 4b7fce8edc6ff40240291ed784afb4d3788bc735 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sun, 8 Nov 2026 12:48:07 +0700 Subject: [PATCH 12/14] audit command palette --- crates/core/src/command_palette.rs | 25 ++++++++++++++++++++++--- crates/theme/src/lib.rs | 2 -- docs/CHECKLIST.md | 14 +++++--------- examples/gallery/src/main.rs | 6 ++++-- 4 files changed, 31 insertions(+), 16 deletions(-) diff --git a/crates/core/src/command_palette.rs b/crates/core/src/command_palette.rs index 2f6ea24..1fc6568 100644 --- a/crates/core/src/command_palette.rs +++ b/crates/core/src/command_palette.rs @@ -10,6 +10,7 @@ use gpui::{ ScrollHandle, SharedString, StatefulInteractiveElement as _, Styled as _, Subscription, Window, actions, canvas, div, px, relative, }; +use gpui_native_icons::{IconRole, icon}; use gpui_native_theme::{ActiveTheme as _, Size}; use crate::common::{h_flex, v_flex}; @@ -48,18 +49,27 @@ pub struct Command { pub label: SharedString, /// Gray text on the row's right edge, e.g. the keybinding. pub hint: Option, + /// Leading glyph shown before the label. + pub icon: Option, } impl Command { - /// Creates a command without a hint. + /// Creates a command without a hint or icon. pub fn new(id: impl Into, label: impl Into) -> Self { Self { id: id.into(), label: label.into(), hint: None, + icon: None, } } + /// Shows an icon before the label. + pub fn icon(mut self, role: IconRole) -> Self { + self.icon = Some(role); + self + } + /// Sets the row's right-edge hint. pub fn hint(mut self, hint: impl Into) -> Self { self.hint = Some(hint.into()); @@ -295,15 +305,21 @@ impl Render for CommandPaletteState { return div().into_any_element(); } - let (colors, radius, metrics, shadows) = { + let (colors, radius, metrics, panel_border, shadows) = { let theme = cx.theme(); ( theme.colors, theme.radius, theme.sizes.resolve(Size::default()), + if theme.borders.overlays { + theme.colors.border + } else { + theme.colors.surface + }, theme.elevations.dialog.shadows(), ) }; + let id = self.id.clone(); let state = cx.entity(); @@ -345,6 +361,9 @@ impl Render for CommandPaletteState { state.update(cx, |state, cx| state.choose_row(row_ix, window, cx)); } }) + .when_some(command.icon, |row, role| { + row.child(icon(role).size(metrics.icon_size).color(colors.foreground)) + }) .child( div() .flex_1() @@ -407,7 +426,7 @@ impl Render for CommandPaletteState { .text_color(colors.foreground) .rounded(radius.overlay) .border_1() - .border_color(colors.border) + .border_color(panel_border) .shadow(shadows) .overflow_hidden() .on_mouse_down(MouseButton::Left, |_, _, cx| cx.stop_propagation()) diff --git a/crates/theme/src/lib.rs b/crates/theme/src/lib.rs index 8681693..ce7b09a 100644 --- a/crates/theme/src/lib.rs +++ b/crates/theme/src/lib.rs @@ -419,8 +419,6 @@ impl SizeTable { }; let metrics = |f: &ClassFactors| { - // Standard controls carry 16px glyphs on every platform, fixed - // against the root font the way the platforms fix them. let icon_size = if f.floor_icon { (rem_size * f.icon).max(px(16.)) } else { diff --git a/docs/CHECKLIST.md b/docs/CHECKLIST.md index baec1a0..758587e 100644 --- a/docs/CHECKLIST.md +++ b/docs/CHECKLIST.md @@ -14,10 +14,6 @@ is the first downstream consumer; its screens drive the order. Platform metric adoption continues one component at a time; each stop goes through the review gate and a gallery pass. -- [ ] **Combobox platform chrome** — popup border behind - `Theme::borders.overlays`; the trigger follows the input stop's field - treatment. -- [ ] **Command palette platform chrome** — overlay border gate. - [ ] **Switch platform chrome** — stated track and knob dims (`switch_height` / `switch_width` / `switch_knob`, inset and travel derived); borderless knob. @@ -143,11 +139,10 @@ Gaps surfaced by the signed comparison; ordered by signed's usage. (`.w(...)`) set the width; the empty field keeps its placeholder while focused (`Input::placeholder_when_focused`) and clicking into it opens the options right away, with no keystroke needed -- [x] Combobox platform chrome — the popup border behind +- [x] Command palette platform chrome — the panel border behind `Theme::borders.overlays` (and dark mode), taking the surface color - where the platform paints none; the trigger is a plain `Input`, so it - already carries the full field treatment, and the chevron recenters on - the field's full height, border included + where the platform paints none; it rides the dialog elevation, so dark + mode's no-shadow rule applies to it too - [x] Button, Checkbox, Switch, Radio + RadioGroup, Toggle, Link, Slider, Progress, Avatar (image + seeded pixel-art fallback) - [x] Spinner — painted ring + rotating-icon variant, indeterminate and @@ -166,7 +161,8 @@ Gaps surfaced by the signed comparison; ordered by signed's usage. screen-reader label - [x] Combobox — type-to-filter picker with a caller-owned matcher, create / keep / revert blur modes -- [x] Command palette — filtered actions; the matcher stays caller-owned +- [x] Command palette — filtered actions; the matcher stays caller-owned, + and commands may carry a leading icon - [x] Scrollbar — overlay thumbs for gpui's `list` and `uniform_list`, platform autohide with a `visibility` override - [x] Clipboard button — copies a value, confirms with a brief check mark diff --git a/examples/gallery/src/main.rs b/examples/gallery/src/main.rs index 6744019..186c2e3 100644 --- a/examples/gallery/src/main.rs +++ b/examples/gallery/src/main.rs @@ -205,8 +205,10 @@ fn gallery_commands() -> Vec { Command::new("open-settings", "Open Settings").hint("Cmd+,"), Command::new("toggle-theme", "Toggle Theme").hint("Shift+Cmd+T"), Command::new("save-all", "Save All").hint("Cmd+Alt+S"), - Command::new("close-tab", "Close Tab").hint("Cmd+W"), - Command::new("reload-window", "Reload Window"), + Command::new("close-tab", "Close Tab") + .hint("Cmd+W") + .icon(IconRole::Close), + Command::new("reload-window", "Reload Window").icon(IconRole::Loader), ] } -- 2.54.0 From db5133b8071af7622a505635f8a6ba4d316ac4bc Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sun, 8 Nov 2026 12:53:40 +0700 Subject: [PATCH 13/14] audit switch --- crates/core/src/switch.rs | 15 +++++++++------ crates/core/tests/switch.rs | 24 ++++++++++++++++++++++++ docs/CHECKLIST.md | 9 +++++---- 3 files changed, 38 insertions(+), 10 deletions(-) diff --git a/crates/core/src/switch.rs b/crates/core/src/switch.rs index 611b28a..23f1d27 100644 --- a/crates/core/src/switch.rs +++ b/crates/core/src/switch.rs @@ -96,16 +96,17 @@ impl RenderOnce for Switch { let ramps = theme.ramps; let metrics = theme.sizes.resolve(self.size); - let track_height = metrics.control_height; - let track_width = metrics.control_height * 1.65; + let track_height = metrics.switch_height; + let track_width = metrics.switch_width; - let knob = metrics.indicator_size; - let inset = metrics.plate_inset; + let knob = metrics.switch_knob; + let inset = (track_height - knob) / 2.; let travel = track_width - knob - inset * 2.; let next = !self.checked; let on_change = self.on_change; let a11y_label = self.a11y_label.or_else(|| self.label.clone()); + let id = self.id.clone(); let anim_config = SpringConfig::new(500., 35., 1.); let anim = SpringAnimation::new(anim_config).to(if self.checked { travel } else { px(0.) }); @@ -132,6 +133,10 @@ impl RenderOnce for Switch { div() .flex_none() .relative() + .debug_selector({ + let name = format!("{id}-track"); + move || name + }) .w(track_width) .h(track_height) .rounded(theme.radius.full) @@ -149,8 +154,6 @@ impl RenderOnce for Switch { .size(knob) .rounded(theme.radius.full) .bg(colors.primary_foreground) - .border_1() - .border_color(colors.border) .with_spring("switch-thumb", anim, move |thumb, offset| { thumb.left(inset + offset) }), diff --git a/crates/core/tests/switch.rs b/crates/core/tests/switch.rs index 5882110..ffedc18 100644 --- a/crates/core/tests/switch.rs +++ b/crates/core/tests/switch.rs @@ -83,6 +83,30 @@ fn disabled_switch_does_not_report_changes() { assert!(changes.borrow().is_empty()); } +/// The track renders at the platform's stated switch dims, not the generic +/// control height — the stated metric is the whole chrome. +#[test] +fn track_renders_at_the_stated_switch_dims() { + let (mut cx, _window, _changes) = switch_window(false, false); + let (width, height) = cx.update(|_window, cx| { + let metrics = Theme::global(cx).sizes.medium; + (metrics.switch_width, metrics.switch_height) + }); + let track = cx + .debug_bounds("test-switch-track") + .expect("the switch track should have been laid out"); + assert!( + (track.size.width - width).abs() <= px(0.5), + "track is {:?} wide, stated switch width is {width:?}", + track.size.width + ); + assert!( + (track.size.height - height).abs() <= px(0.5), + "track is {:?} tall, stated switch height is {height:?}", + track.size.height + ); +} + #[test] fn enter_and_space_report_the_next_value_while_focused() { let (mut cx, window, changes) = switch_window(false, false); diff --git a/docs/CHECKLIST.md b/docs/CHECKLIST.md index 758587e..97ac0f9 100644 --- a/docs/CHECKLIST.md +++ b/docs/CHECKLIST.md @@ -14,10 +14,6 @@ is the first downstream consumer; its screens drive the order. Platform metric adoption continues one component at a time; each stop goes through the review gate and a gallery pass. -- [ ] **Switch platform chrome** — stated track and knob dims - (`switch_height` / `switch_width` / `switch_knob`, inset and travel - derived); borderless knob. - Gaps surfaced by the signed comparison; ordered by signed's usage. - [ ] **Virtual list** — generic virtualized variable-height rows with a @@ -143,6 +139,11 @@ Gaps surfaced by the signed comparison; ordered by signed's usage. `Theme::borders.overlays` (and dark mode), taking the surface color where the platform paints none; it rides the dialog elevation, so dark mode's no-shadow rule applies to it too +- [x] Switch platform chrome — the track and knob take the stated + `switch_height` / `switch_width` / `switch_knob` (the knob's inset + derived to center it in the track, its travel from the remaining + width); the knob is borderless, and the track carries a debug selector + for headless geometry tests - [x] Button, Checkbox, Switch, Radio + RadioGroup, Toggle, Link, Slider, Progress, Avatar (image + seeded pixel-art fallback) - [x] Spinner — painted ring + rotating-icon variant, indeterminate and -- 2.54.0 From b07b1d6c2e7e0cb6cafadf3acbc58be96f8e813f Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sun, 8 Nov 2026 13:26:21 +0700 Subject: [PATCH 14/14] clean up --- crates/core/src/avatar.rs | 6 --- crates/core/tests/alert.rs | 43 -------------------- crates/core/tests/dialog.rs | 8 ---- crates/core/tests/field_bench.rs | 63 ++++-------------------------- crates/core/tests/input.rs | 38 ------------------ crates/core/tests/menu.rs | 17 -------- crates/core/tests/progress.rs | 36 ----------------- crates/core/tests/scroll_area.rs | 7 ---- crates/core/tests/segmented.rs | 25 ------------ crates/core/tests/status_dot.rs | 13 ------ crates/core/tests/tooltip.rs | 10 ----- crates/data/tests/tabs.rs | 10 ----- crates/data/tests/tree.rs | 14 ------- crates/editor/tests/diff_view.rs | 6 --- crates/editor/tests/markdown.rs | 7 ---- crates/icons/src/freedesktop.rs | 8 ---- crates/icons/src/lib.rs | 17 -------- crates/icons/src/segoe.rs | 5 --- crates/theme/src/resolve.rs | 29 -------------- crates/theme/src/source/omarchy.rs | 5 --- crates/theme/src/source/portal.rs | 10 ----- 21 files changed, 7 insertions(+), 370 deletions(-) delete mode 100644 crates/core/tests/progress.rs diff --git a/crates/core/src/avatar.rs b/crates/core/src/avatar.rs index 2f3b553..af87432 100644 --- a/crates/core/src/avatar.rs +++ b/crates/core/src/avatar.rs @@ -315,10 +315,4 @@ mod tests { assert!(filled >= MIN_FILLED * 2, "seed {seed} filled {filled}"); } } - - #[test] - fn fnv1a_is_stable_across_calls() { - assert_eq!(fnv1a(b"reya"), fnv1a(b"reya")); - assert_ne!(fnv1a(b"reya"), fnv1a(b"signed")); - } } diff --git a/crates/core/tests/alert.rs b/crates/core/tests/alert.rs index fda0148..9a6a0cb 100644 --- a/crates/core/tests/alert.rs +++ b/crates/core/tests/alert.rs @@ -54,32 +54,6 @@ fn alert_window( ) } -/// A banner with only a message paints, so the title stays optional. -#[test] -fn a_message_only_alert_renders() { - let mut cx = TestAppContext::single(); - cx.update(Theme::init); - let window = cx.add_window(|_, _| MessageOnlyHarness); - let window = window.into(); - let mut cx = VisualTestContext::from_window(window, &cx); - cx.update(|window, cx| { - window.draw(cx).clear(cx); - }); - assert!(cx.debug_bounds("message-only-alert").is_some()); -} - -struct MessageOnlyHarness; - -impl Render for MessageOnlyHarness { - fn render(&mut self, _window: &mut Window, _cx: &mut gpui::Context) -> impl IntoElement { - div().size_full().p_4().child( - div() - .debug_selector(|| "message-only-alert".to_string()) - .child(Alert::new("message-only").message("Only line.")), - ) - } -} - /// Clicking the close button is how the user acts on the alert; the banner /// reports the intent instead of hiding itself. #[test] @@ -94,20 +68,3 @@ fn closing_the_alert_reports_dismissal() { assert!(*dismissed.borrow(), "the close button reported dismissal"); } - -/// Every severity paints a banner. -#[test] -fn every_severity_renders_a_banner() { - for severity in [ - AlertSeverity::Info, - AlertSeverity::Success, - AlertSeverity::Warning, - AlertSeverity::Danger, - ] { - let (mut cx, _window, _dismissed) = alert_window(severity); - assert!( - cx.debug_bounds("test-alert").is_some(), - "{severity:?} alert rendered" - ); - } -} diff --git a/crates/core/tests/dialog.rs b/crates/core/tests/dialog.rs index 787d55d..06a5b09 100644 --- a/crates/core/tests/dialog.rs +++ b/crates/core/tests/dialog.rs @@ -158,11 +158,3 @@ fn clicking_the_panel_does_not_dismiss() { click(&mut visual, point(px(960.), px(540.))); assert!(changes.borrow().is_empty()); } - -#[test] -fn a_closed_dialog_is_inert() { - let (_cx, mut visual, changes) = dialog_window(false, false); - visual.simulate_keystrokes("escape"); - click(&mut visual, point(px(100.), px(100.))); - assert!(changes.borrow().is_empty()); -} diff --git a/crates/core/tests/field_bench.rs b/crates/core/tests/field_bench.rs index fd50560..dd299a5 100644 --- a/crates/core/tests/field_bench.rs +++ b/crates/core/tests/field_bench.rs @@ -1,7 +1,3 @@ -use std::cell::RefCell; -use std::rc::Rc; -use std::time::Instant; - use gpui::{ AppContext as _, Context, Entity, IntoElement, ParentElement as _, Render, SharedString, Styled as _, TestAppContext, VisualTestContext, Window, div, @@ -13,12 +9,10 @@ use gpui_native_core::input::{Input, InputState}; struct FieldHost { url: Entity, - renders: Rc>, } impl Render for FieldHost { fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { - *self.renders.borrow_mut() += 1; let url_error = self .url .read(cx) @@ -40,30 +34,18 @@ impl Render for FieldHost { /// A field page in a real window; the url input is focused unless /// `focus` is false, which leaves the page truly idle. -fn field_host( - focus: bool, -) -> ( - TestAppContext, - Entity, - Rc>, - VisualTestContext, -) { +fn field_host(focus: bool) -> (TestAppContext, Entity, VisualTestContext) { let cx = TestAppContext::single(); cx.update(init); - let (url, renders, window) = cx.update(|cx| { - let renders = Rc::new(RefCell::new(0)); + let (url, window) = cx.update(|cx| { let url = InputState::new(cx); let url_for_host = url.clone(); - let renders_for_host = renders.clone(); let window = cx .open_window(gpui::WindowOptions::default(), |_, cx| { - cx.new(|_| FieldHost { - url: url_for_host, - renders: renders_for_host, - }) + cx.new(|_| FieldHost { url: url_for_host }) }) .unwrap(); - (url, renders, window) + (url, window) }); let mut visual = VisualTestContext::from_window(window.into(), &cx); visual.update(|window, cx| { @@ -73,33 +55,12 @@ fn field_host( } window.draw(cx).clear(cx); }); - (cx, url, renders, visual) -} - -fn draw(visual: &mut VisualTestContext) { - visual.update(|window, cx| { - window.draw(cx).clear(cx); - }); -} - -fn type_and_time(visual: &mut VisualTestContext, text: &str) -> std::time::Duration { - let started = Instant::now(); - for letter in text.chars() { - visual.simulate_keystrokes(&letter.to_string()); - draw(visual); - } - let elapsed = started.elapsed(); - println!( - "{} keystrokes: {elapsed:?} ({:.2} ms per keystroke)", - text.chars().count(), - elapsed.as_secs_f64() / text.chars().count() as f64 * 1000. - ); - elapsed + (cx, url, visual) } #[test] fn an_idle_field_page_schedules_no_frames() { - let (_cx, _url, _renders, mut visual) = field_host(false); + let (_cx, _url, mut visual) = field_host(false); let frames = visual.update(|window, cx| window.simulate_next_frame(cx)); assert_eq!( frames, 0, @@ -109,20 +70,10 @@ fn an_idle_field_page_schedules_no_frames() { #[test] fn a_focused_field_page_schedules_frames_for_its_blink() { - let (_cx, _url, _renders, mut visual) = field_host(true); + let (_cx, _url, mut visual) = field_host(true); let frames = visual.update(|window, cx| window.simulate_next_frame(cx)); assert!( frames > 0, "the focused caret's blink needs frames to redraw at the blink cadence" ); } - -#[test] -fn field_typing_frame_cost() { - let (_cx, _url, _renders, mut visual) = field_host(true); - let elapsed = type_and_time(&mut visual, "dsdsadsadadadada"); - assert!( - elapsed.as_millis() < 1000, - "typing 16 characters took {elapsed:?}" - ); -} diff --git a/crates/core/tests/input.rs b/crates/core/tests/input.rs index ad7ba73..95a279c 100644 --- a/crates/core/tests/input.rs +++ b/crates/core/tests/input.rs @@ -465,15 +465,6 @@ fn disabled_input_ignores_typing_and_clicks() { assert_eq!(value(&cx, &state), ""); } -#[test] -fn set_value_replaces_the_buffer() { - let (cx, state, mut visual) = input_window("", false); - visual.update(|window, cx| { - state.update(cx, |state, cx| state.set_value("preset", window, cx)); - }); - assert_eq!(value(&cx, &state), "preset"); -} - /// Subscribes to `state`'s events, recording each variant into a sink that /// lives for the rest of the test. The returned `Subscription` must stay /// bound for the same lifetime — dropping it unsubscribes. @@ -542,26 +533,6 @@ fn set_value_silent_updates_without_emitting() { assert_eq!(count(&sink, &InputStateEvent::TextChanged), 0); } -#[test] -fn focus_and_blur_emit_events() { - let (mut cx, state, mut visual) = input_window("", false); - let (sink, _subscription) = record_events(&mut cx, &state); - - focus(&mut visual, &state); - visual.update(|window, cx| { - window.draw(cx).clear(cx); - }); - visual.run_until_parked(); - assert_eq!(count(&sink, &InputStateEvent::Focused), 1); - - visual.update(|window, cx| window.blur(cx)); - visual.update(|window, cx| { - window.draw(cx).clear(cx); - }); - visual.run_until_parked(); - assert_eq!(count(&sink, &InputStateEvent::Blurred), 1); -} - #[test] fn pasted_newlines_become_spaces() { let (cx, state, mut visual) = input_window("", false); @@ -574,15 +545,6 @@ fn pasted_newlines_become_spaces() { assert_eq!(value(&cx, &state), "a bc"); } -#[test] -fn set_value_sanitizes_newlines() { - let (cx, state, mut visual) = input_window("", false); - visual.update(|window, cx| { - state.update(cx, |state, cx| state.set_value("a\nb", window, cx)); - }); - assert_eq!(value(&cx, &state), "a b"); -} - #[test] fn enter_emits_submitted() { let (mut cx, state, mut visual) = input_window("draft", false); diff --git a/crates/core/tests/menu.rs b/crates/core/tests/menu.rs index dcf94c0..8782117 100644 --- a/crates/core/tests/menu.rs +++ b/crates/core/tests/menu.rs @@ -185,14 +185,6 @@ fn clicking_outside_dismisses_and_swallows_the_click() { assert_eq!(*changes.borrow(), vec![false]); } -#[test] -fn clicking_the_trigger_toggles() { - let (_cx, mut visual, _window, changes, _activated) = menu_window(false); - - click(&mut visual, point(px(10.), px(16.))); - assert_eq!(*changes.borrow(), vec![true]); -} - #[test] fn clicking_an_item_activates_and_closes() { let (_cx, mut visual, _window, changes, activated) = menu_window(true); @@ -202,12 +194,3 @@ fn clicking_an_item_activates_and_closes() { assert_eq!(*activated.borrow(), vec!["cut"]); assert_eq!(*changes.borrow(), vec![false]); } - -#[test] -fn enter_activates_the_selected_item_and_closes() { - let (_cx, mut visual, _window, changes, activated) = menu_window(true); - - visual.simulate_keystrokes("enter"); - assert_eq!(*activated.borrow(), vec!["cut"]); - assert_eq!(*changes.borrow(), vec![false]); -} diff --git a/crates/core/tests/progress.rs b/crates/core/tests/progress.rs deleted file mode 100644 index 37f5904..0000000 --- a/crates/core/tests/progress.rs +++ /dev/null @@ -1,36 +0,0 @@ -use gpui::{ParentElement as _, Render, Styled as _, TestAppContext, Window, div}; -use gpui_native_core::progress::Progress; -use gpui_native_theme::Theme; - -struct Harness; - -impl Render for Harness { - fn render( - &mut self, - _window: &mut Window, - _cx: &mut gpui::Context, - ) -> impl gpui::IntoElement { - div() - .size_full() - .flex() - .flex_col() - .gap_2() - .child(Progress::new("test-progress", 0.6).accessibility_label("Upload")) - .child( - Progress::new("test-progress-indeterminate", 0.0) - .indeterminate(true) - .accessibility_label("Working"), - ) - } -} - -#[gpui::test] -fn determinate_and_indeterminate_bars_draw_headlessly(cx: &mut TestAppContext) { - cx.update(Theme::init); - let (view, visual) = cx.add_window_view(|_, _| Harness); - let _ = view; - visual.run_until_parked(); - visual.update(|window, cx| { - window.draw(cx).clear(cx); - }); -} diff --git a/crates/core/tests/scroll_area.rs b/crates/core/tests/scroll_area.rs index af5476b..a27533b 100644 --- a/crates/core/tests/scroll_area.rs +++ b/crates/core/tests/scroll_area.rs @@ -88,13 +88,6 @@ fn the_content_starts_unscrolled_and_horizontal_axes_scroll_x() { assert_eq!(handle.offset().x, px(0.)); } -#[test] -fn the_thumb_paints_when_the_content_overflows() { - let (_cx, _visual, handle) = scroll_area_window(30); - // 30 rows × 20px in a 200px viewport: the thumb has something to report. - assert!(handle.max_offset().y > px(0.)); -} - /// The gallery's exact shell: a window-filling root, a window-filling scroll /// area over padded flex-column content that towers past the viewport. struct GalleryShell { diff --git a/crates/core/tests/segmented.rs b/crates/core/tests/segmented.rs index d78721b..f6c5c64 100644 --- a/crates/core/tests/segmented.rs +++ b/crates/core/tests/segmented.rs @@ -121,28 +121,3 @@ fn arrows_move_the_selection_when_the_strip_has_focus() { vec![Filter::Popular, Filter::Recent, Filter::All, Filter::Recent] ); } - -/// The selected plate exists after a choice and settles on the chosen -/// segment's slot; without it the active background disappears entirely. -#[test] -fn the_plate_settles_on_the_chosen_segment() { - let (_cx, mut visual, changes) = filter_window(); - - click(&mut visual, point(px(70.), px(27.))); - assert_eq!(*changes.borrow(), vec![Filter::Popular]); - - // Let the spring settle in real time (it steps on Instant::now()), then - // check the plate landed in Popular's slot (~58..124). - std::thread::sleep(std::time::Duration::from_millis(600)); - visual.update(|window, cx| { - window.draw(cx).clear(cx); - }); - let plate = visual - .debug_bounds("segmented-plate") - .expect("plate missing"); - assert!( - plate.left() >= px(44.) && plate.left() <= px(94.), - "plate settled at x {:?}", - plate.origin - ); -} diff --git a/crates/core/tests/status_dot.rs b/crates/core/tests/status_dot.rs index 9ed3f61..cc7d160 100644 --- a/crates/core/tests/status_dot.rs +++ b/crates/core/tests/status_dot.rs @@ -45,16 +45,3 @@ fn the_dot_is_half_the_icon_size() { dot.size ); } - -#[test] -fn the_dot_keeps_a_square_footprint() { - let (cx, window) = dot_window(); - let mut visual = VisualTestContext::from_window(window, &cx); - let dot = visual.debug_bounds("test-dot").expect("dot missing"); - - assert!( - (dot.size.width - dot.size.height).abs() <= gpui::px(0.5), - "the dot must stay circular: {:?}", - dot.size - ); -} diff --git a/crates/core/tests/tooltip.rs b/crates/core/tests/tooltip.rs index 5cd943d..681bcf6 100644 --- a/crates/core/tests/tooltip.rs +++ b/crates/core/tests/tooltip.rs @@ -150,16 +150,6 @@ fn hovering_elsewhere_deploys_nothing() { assert!(calls.borrow().is_empty()); } -#[test] -fn the_button_passthrough_deploys_the_tooltip() { - let (cx, mut visual, calls) = tooltip_window(); - hover(&mut visual, point(px(50.), px(100.))); - cx.executor().advance_clock(SHOW_DELAY); - visual.run_until_parked(); - draw(&mut visual); - assert_eq!(*calls.borrow(), vec!["button"]); -} - /// An anchored tooltip hangs below the hovered parent's bottom edge with the /// spacing gap: it never covers the parent, wherever the cursor is inside it. #[test] diff --git a/crates/data/tests/tabs.rs b/crates/data/tests/tabs.rs index e76971c..2d8d555 100644 --- a/crates/data/tests/tabs.rs +++ b/crates/data/tests/tabs.rs @@ -100,13 +100,3 @@ fn arrow_keys_skip_disabled_tabs_and_never_wrap() { visual.simulate_keystrokes("end"); // End assert_eq!(*changes.borrow(), vec![1, 3, 1, 0, 3]); } - -#[test] -fn clicking_a_different_tab_reports_once() { - let (_cx, mut visual, changes) = tabs_window(); - - click(&mut visual, point(px(20.), px(12.))); // One: no change, just focus - click(&mut visual, point(px(62.), px(12.))); // Two - click(&mut visual, point(px(62.), px(12.))); // Two again: no change - assert_eq!(*changes.borrow(), vec![1]); -} diff --git a/crates/data/tests/tree.rs b/crates/data/tests/tree.rs index 463b612..a1c8a0e 100644 --- a/crates/data/tests/tree.rs +++ b/crates/data/tests/tree.rs @@ -175,17 +175,3 @@ fn keyboard_navigation_skips_disabled_rows_and_never_wraps() { ] ); } - -#[test] -fn disclosure_keys_skip_disabled_and_unselected_rows() { - let (_cx, mut visual, _window, changes, disclosures) = tree_window(); - - // Focus through the first row, then park the selection on the leaf. - click(&mut visual, point(px(50.), px(16.))); - visual.simulate_keystrokes("down"); // Child (leaf), focused tree - visual.simulate_keystrokes("left"); // leaf: no disclosure - visual.simulate_keystrokes("right"); // leaf: no disclosure - draw(&mut visual); - assert!(disclosures.borrow().is_empty()); - assert_eq!(*changes.borrow(), vec![Some(0), Some(1)]); -} diff --git a/crates/editor/tests/diff_view.rs b/crates/editor/tests/diff_view.rs index c6878d0..700d0b1 100644 --- a/crates/editor/tests/diff_view.rs +++ b/crates/editor/tests/diff_view.rs @@ -226,12 +226,6 @@ fn rows_render_virtualized_and_the_view_jumps_to_the_first_change() { ); } -#[test] -fn identical_versions_produce_no_rows() { - let rows = diff_rows(OLD, OLD, Language::Rust); - assert!(rows.is_empty(), "identical versions have no diff to show"); -} - #[test] fn set_versions_re_diffs() { let mut cx = TestAppContext::single(); diff --git a/crates/editor/tests/markdown.rs b/crates/editor/tests/markdown.rs index 5a6d9f3..2b55e9d 100644 --- a/crates/editor/tests/markdown.rs +++ b/crates/editor/tests/markdown.rs @@ -241,13 +241,6 @@ fn nested_code_and_tables_carry_indent_and_quote() { )); } -#[test] -fn task_markers_carry_their_checked_state() { - let blocks = parse_body("- [ ] todo\n- [x] done\n"); - assert_eq!(item_of(&blocks, 0).0, ListMarker::Task(false)); - assert_eq!(item_of(&blocks, 1).0, ListMarker::Task(true)); -} - #[test] fn quotes_mark_their_paragraphs_and_lists() { let blocks = parse_body("> quoted\n>\n> - listed\n\nplain"); diff --git a/crates/icons/src/freedesktop.rs b/crates/icons/src/freedesktop.rs index e8addb0..42e3998 100644 --- a/crates/icons/src/freedesktop.rs +++ b/crates/icons/src/freedesktop.rs @@ -392,12 +392,4 @@ mod tests { ); std::fs::remove_dir_all(&base).expect("cleanup"); } - - #[test] - fn every_role_states_freedesktop_candidates() { - for role in crate::IconRole::all() { - assert!(!role.freedesktop_names().is_empty()); - assert!(!role.slug().is_empty()); - } - } } diff --git a/crates/icons/src/lib.rs b/crates/icons/src/lib.rs index b1f9cf2..8f5d6c3 100644 --- a/crates/icons/src/lib.rs +++ b/crates/icons/src/lib.rs @@ -367,21 +367,4 @@ mod tests { ); } } - - #[test] - fn freedesktop_asset_keys_round_trip() { - let role = IconRole::ChevronDown; - let key = format!("icons/freedesktop/{}-16.svg", role.slug()); - let stem = key - .strip_prefix("icons/freedesktop/") - .unwrap() - .strip_suffix(".svg") - .unwrap(); - let (slug, parsed_size) = stem.rsplit_once('-').unwrap(); - let resolved = IconRole::all() - .iter() - .find(|candidate| candidate.slug() == slug); - assert_eq!(resolved, Some(&role)); - assert_eq!(parsed_size, "16"); - } } diff --git a/crates/icons/src/segoe.rs b/crates/icons/src/segoe.rs index c569593..b5fdaeb 100644 --- a/crates/icons/src/segoe.rs +++ b/crates/icons/src/segoe.rs @@ -49,9 +49,4 @@ mod tests { ); } } - - #[test] - fn the_close_role_uses_the_chrome_close_glyph() { - assert_eq!(glyph(IconRole::Close), Some('\u{E8BB}')); - } } diff --git a/crates/theme/src/resolve.rs b/crates/theme/src/resolve.rs index 7811e16..fcaf96f 100644 --- a/crates/theme/src/resolve.rs +++ b/crates/theme/src/resolve.rs @@ -339,19 +339,6 @@ mod tests { } } - #[test] - fn metric_priority_is_definition_then_system_then_fallback() { - let mut stated = system(ThemeVariant::Dark, PartialPalette::default(), None); - stated.control_radius = Some(px(4.)); - - let resolved = resolve(&stated, None, None); - assert_eq!(resolved.radius.control, px(4.)); - - let def = ThemeDefinition::parse_toml("control_radius = 12.0\n").unwrap(); - let overridden = resolve(&stated, Some(&def), None); - assert_eq!(overridden.radius.control, px(12.)); - } - #[test] fn empty_system_resolves_to_the_neutral_fallback() { for variant in [ThemeVariant::Light, ThemeVariant::Dark] { @@ -820,20 +807,4 @@ mod tests { let preset_stated = resolve(&system(ThemeVariant::Dark, palette, None), Some(&def), None); assert_eq!(preset_stated.colors.info, hsla(0x0ea5e9)); } - - #[test] - fn muted_hover_follows_the_overlay_chain() { - // No platform states a hover wash; a preset or user file is the only - // override above the neutral base. - let palette = PartialPalette { - muted_hover: Some(hsla(0x155e75)), - ..PartialPalette::default() - }; - let stated = resolve(&system(ThemeVariant::Dark, palette, None), None, None); - assert_eq!(stated.colors.muted_hover, hsla(0x155e75)); - - let def = ThemeDefinition::parse_toml("muted_hover = \"#0ea5e9\"\n").unwrap(); - let preset_stated = resolve(&system(ThemeVariant::Dark, palette, None), Some(&def), None); - assert_eq!(preset_stated.colors.muted_hover, hsla(0x0ea5e9)); - } } diff --git a/crates/theme/src/source/omarchy.rs b/crates/theme/src/source/omarchy.rs index 2476a6e..890c63b 100644 --- a/crates/theme/src/source/omarchy.rs +++ b/crates/theme/src/source/omarchy.rs @@ -238,9 +238,4 @@ color15 = \"#635d6f\" assert_eq!(system.titlebar_height, None); assert_eq!(system.root_font_size, None); } - - #[test] - fn non_toml_text_is_rejected() { - assert!(OmarchyTheme::parse("not a theme").is_none()); - } } diff --git a/crates/theme/src/source/portal.rs b/crates/theme/src/source/portal.rs index 2de322c..ab6d68a 100644 --- a/crates/theme/src/source/portal.rs +++ b/crates/theme/src/source/portal.rs @@ -206,16 +206,6 @@ mod tests { ); } - #[test] - fn an_unset_scheme_yields_the_callers_variant() { - let appearance = AppearanceValues::default().decode(); - assert_eq!( - appearance.variant(crate::ThemeVariant::Dark), - crate::ThemeVariant::Dark - ); - assert_eq!(appearance, Appearance::default()); - } - #[test] fn accessibility_flags_forward() { let accessibility = AppearanceValues { -- 2.54.0