audit select

This commit is contained in:
2026-11-07 19:30:04 +07:00
parent ffbb732f5f
commit a859704185
8 changed files with 262 additions and 138 deletions
+31 -39
View File
@@ -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<RefCell<Option<Bounds<Pixels>>>> = Rc::new(RefCell::new(None));
let panel_bounds: Rc<RefCell<Option<Bounds<Pixels>>>> = 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(),
);
+23 -43
View File
@@ -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(),
);
+27 -5
View File
@@ -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()
+68 -35
View File
@@ -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<RefCell<Vec<bool>>>,
target_clicks: Rc<RefCell<usize>>,
}
impl Render for Harness {
fn render(&mut self, _window: &mut Window, cx: &mut Context<Self>) -> 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<Harness>,
Rc<RefCell<Vec<bool>>>,
) {
Rc<RefCell<usize>>,
);
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<gpui::Pixels>) {
#[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);
}
+81 -3
View File
@@ -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::<Option<Strategy>>::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::<Option<Strategy>>::new());
assert_eq!(*second_changes.borrow(), vec![Some(Strategy::Curated)]);
}
struct HeightsHarness {
state: Entity<InputState>,
}
impl Render for HeightsHarness {
fn render(&mut self, _window: &mut Window, _cx: &mut Context<Self>) -> 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));
}
+8 -8
View File
@@ -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 {
+14 -1
View File
@@ -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(