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