audit popover and dialog

This commit is contained in:
2026-11-07 20:07:09 +07:00
parent 66961a1026
commit 2b3e3b4e99
7 changed files with 75 additions and 23 deletions
+5 -1
View File
@@ -130,7 +130,11 @@ impl RenderOnce for Dialog {
let theme = cx.theme(); let theme = cx.theme();
( (
theme.colors.overlay, theme.colors.overlay,
theme.colors.border, if theme.borders.overlays {
theme.colors.border
} else {
theme.colors.surface
},
theme.colors.foreground, theme.colors.foreground,
theme.colors.muted_foreground, theme.colors.muted_foreground,
theme.colors.surface, theme.colors.surface,
+4 -3
View File
@@ -7,7 +7,7 @@ use gpui::{
Entity, InteractiveElement as _, IntoElement, KeyDownEvent, MouseButton, MouseDownEvent, Entity, InteractiveElement as _, IntoElement, KeyDownEvent, MouseButton, MouseDownEvent,
ParentElement as _, Pixels, Point, RenderOnce, Role, ScrollHandle, SharedString, ParentElement as _, Pixels, Point, RenderOnce, Role, ScrollHandle, SharedString,
StatefulInteractiveElement as _, Styled as _, Window, anchored, canvas, deferred, div, 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_icons::{IconRole, icon as icon_glyph};
use gpui_native_theme::ActiveTheme as _; use gpui_native_theme::ActiveTheme as _;
@@ -374,11 +374,11 @@ impl RenderOnce for Menu {
let panel = v_flex() let panel = v_flex()
.id("menu-panel") .id("menu-panel")
.debug_selector(|| "menu-panel".to_string())
.occlude() .occlude()
.track_focus(&focus) .track_focus(&focus)
.role(Role::Menu) .role(Role::Menu)
.aria_label(label) .aria_label(label)
.mt_1()
.min_w_56() .min_w_56()
.p(chrome.spacing_unit) .p(chrome.spacing_unit)
.bg(chrome.surface) .bg(chrome.surface)
@@ -435,7 +435,8 @@ impl RenderOnce for Menu {
let anchored = match self.open_at { let anchored = match self.open_at {
Some(position) => anchored().position(position), Some(position) => anchored().position(position),
None => anchored(), None => anchored(),
}; }
.offset(point(px(0.), chrome.spacing_unit));
wrapper = wrapper.child(deferred(anchored.child(panel.with_animation( wrapper = wrapper.child(deferred(anchored.child(panel.with_animation(
"menu-enter", "menu-enter",
+19 -9
View File
@@ -6,7 +6,7 @@ use gpui::{
Animation, AnimationExt as _, AnyElement, App, Bounds, DispatchPhase, ElementId, Animation, AnimationExt as _, AnyElement, App, Bounds, DispatchPhase, ElementId,
InteractiveElement as _, IntoElement, KeyDownEvent, MouseButton, MouseDownEvent, InteractiveElement as _, IntoElement, KeyDownEvent, MouseButton, MouseDownEvent,
ParentElement as _, Pixels, RenderOnce, Role, SharedString, StatefulInteractiveElement as _, 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 _; use gpui_native_theme::ActiveTheme as _;
@@ -91,13 +91,19 @@ impl Popover {
impl RenderOnce for Popover { impl RenderOnce for Popover {
fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { 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(); let theme = cx.theme();
( (
theme.colors, theme.colors,
theme.radius, theme.radius,
theme.elevations.popover.shadows(), theme.elevations.popover.shadows(),
theme.motion.overlay, 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() let panel = v_flex()
.id("popover-panel") .id("popover-panel")
.debug_selector(|| "popover-panel".to_string())
.occlude() .occlude()
.track_focus(&focus) .track_focus(&focus)
.role(Role::Dialog) .role(Role::Dialog)
.aria_label(label) .aria_label(label)
.mt_1()
.min_w_32() .min_w_32()
.bg(colors.surface) .bg(colors.surface)
.text_color(colors.surface_foreground) .text_color(colors.surface_foreground)
.rounded(radius.overlay) .rounded(radius.overlay)
.border_1() .border_1()
.border_color(colors.border) .border_color(panel_border)
.shadow(shadows) .shadow(shadows)
.overflow_hidden() .overflow_hidden()
.on_mouse_down_out({ .on_mouse_down_out({
@@ -218,11 +224,15 @@ impl RenderOnce for Popover {
.inset_0(), .inset_0(),
); );
wrapper = wrapper.child(deferred(anchored().child(panel.with_animation( wrapper = wrapper.child(deferred(
"popover-enter", anchored()
Animation::new(enter_duration).with_easing(ease_out_quint()), .offset(point(px(0.), anchor_gap))
|panel, delta| panel.opacity(delta), .child(panel.with_animation(
)))); "popover-enter",
Animation::new(enter_duration).with_easing(ease_out_quint()),
|panel, delta| panel.opacity(delta),
)),
));
} }
wrapper wrapper
+24
View File
@@ -45,6 +45,7 @@ impl Render for Harness {
}) })
.trigger(|_, _| { .trigger(|_, _| {
div() div()
.debug_selector(|| "popover-trigger".to_string())
.w(px(100.)) .w(px(100.))
.h(px(32.)) .h(px(32.))
.bg(rgba(0x000000ff)) .bg(rgba(0x000000ff))
@@ -188,3 +189,26 @@ fn clicking_another_control_dismisses_and_reaches_it() {
assert_eq!(*changes.borrow(), vec![false]); assert_eq!(*changes.borrow(), vec![false]);
assert_eq!(*clicks.borrow(), 1); 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:?}"
);
}
+9 -2
View File
@@ -189,6 +189,13 @@ pub struct Elevation {
} }
impl 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 { fn single(key: Shadow) -> Self {
Self { Self {
layers: [key, Shadow::none()], layers: [key, Shadow::none()],
@@ -232,8 +239,8 @@ impl Elevations {
pub(crate) fn for_platform(source: SourceKind, variant: ThemeVariant) -> Self { pub(crate) fn for_platform(source: SourceKind, variant: ThemeVariant) -> Self {
if variant == ThemeVariant::Dark { if variant == ThemeVariant::Dark {
return Self { return Self {
popover: Elevation::single(Shadow::black(0., 4., 24., 0.3)), popover: Elevation::none(),
dialog: Elevation::single(Shadow::black(0., 8., 40., 0.35)), dialog: Elevation::none(),
control: Elevation::single(Shadow::black(0., 1., 2., 0.25)), control: Elevation::single(Shadow::black(0., 1., 2., 0.25)),
}; };
} }
+4 -2
View File
@@ -545,6 +545,8 @@ mod tests {
resolved.borders.overlays, resolved.borders.overlays,
"{source:?} frames floating layers in dark" "{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] #[test]
fn elevations_and_motion_resolve_with_defaults() { fn elevations_and_motion_resolve_with_defaults() {
let resolved = resolve( let resolved = resolve(
&system(ThemeVariant::Dark, PartialPalette::default(), None), &system(ThemeVariant::Light, PartialPalette::default(), None),
None, None,
None, None,
); );
@@ -590,7 +592,7 @@ mod tests {
); );
assert!(resolved.motion.overlay > std::time::Duration::ZERO); assert!(resolved.motion.overlay > std::time::Duration::ZERO);
let shadows = resolved.elevations.popover.shadows(); let shadows = resolved.elevations.popover.shadows();
assert_eq!(shadows.len(), 1); assert_eq!(shadows.len(), 2);
assert!(!shadows[0].inset); assert!(!shadows[0].inset);
} }
+10 -6
View File
@@ -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 Platform metric adoption continues one component at a time; each stop goes
through the review gate and a gallery pass. 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. - [ ] **Toast platform chrome** — overlay border gate.
- [ ] **Tooltip platform chrome** — overlay border gate. - [ ] **Tooltip platform chrome** — overlay border gate.
- [ ] **Alert 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 (GNOME gsettings `font-name`, KDE `kdeglobals font=`), per-source
border + focus chrome (`Theme::borders`), and a per-platform spacing border + focus chrome (`Theme::borders`), and a per-platform spacing
unit (`Theme::spacing`); overlay borders engage in dark mode, where unit (`Theme::spacing`); overlay borders engage in dark mode, where
the elevation shadow barely reads and floating layers separate by floating layers separate by border instead of shadow — dark elevation
border instead. Components adopt the tokens one by one shadows are dropped entirely, since they only muddied the dark scrim.
(see Next and Queue) Components adopt the tokens one by one (see Next and Queue)
### icons (`crates/icons`) ### 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 `Theme::spacing`, and every action row renders at the stated
`menu_item_height` with content centered, so the stated metric is the `menu_item_height` with content centered, so the stated metric is the
whole row; rows carry debug selectors for headless geometry tests 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, - [x] Button, Checkbox, Switch, Radio + RadioGroup, Toggle, Link, Slider,
Progress, Avatar (image + seeded pixel-art fallback) Progress, Avatar (image + seeded pixel-art fallback)
- [x] Spinner — painted ring + rotating-icon variant, indeterminate and - [x] Spinner — painted ring + rotating-icon variant, indeterminate and