diff --git a/Cargo.lock b/Cargo.lock index 3047a8f..6b6159d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4246,6 +4246,7 @@ dependencies = [ "enum-iterator", "futures", "html5ever", + "image", "indoc", "instant", "itertools 0.13.0", diff --git a/crates/moon-ui-components/Cargo.toml b/crates/moon-ui-components/Cargo.toml index 13da86f..c1a8f92 100644 --- a/crates/moon-ui-components/Cargo.toml +++ b/crates/moon-ui-components/Cargo.toml @@ -210,6 +210,7 @@ windows = { workspace = true, features = [ ] } [dev-dependencies] +image = { version = "0.25.1", default-features = false } gpui = { workspace = true, features = ["test-support"] } indoc = "2" diff --git a/crates/moon-ui-components/component-manifest.json b/crates/moon-ui-components/component-manifest.json index bc637d9..f45bacb 100644 --- a/crates/moon-ui-components/component-manifest.json +++ b/crates/moon-ui-components/component-manifest.json @@ -487,7 +487,7 @@ "theme_source": "MoonTheme tokens through theme bridge / Moon wrapper", "public_path": "moon_ui::MoonSelect", "upstream_ref": "Longbridge::select@cda0fc7fd4e4809dd2a8bae0337ac43b49e9f675", - "fork_reason": "Reviewed TrackedFork drift: Moon trigger variant/style hook lives in base select; Longbridge select state remains source-owned", + "fork_reason": "Reviewed TrackedFork drift: Moon trigger variant/style and configurable deferred menu layer live in base select; MoonSelect explicitly opts popover-hosted menus above their parent, while ordinary selects retain layer 1 and Longbridge selection behavior", "donor_drift_budget": 1, "contracts": ["select.open_select_lifecycle", "gallery.visual_coverage"] }, diff --git a/crates/moon-ui-components/src/moon/popover.rs b/crates/moon-ui-components/src/moon/popover.rs index 3e6ac01..037c855 100644 --- a/crates/moon-ui-components/src/moon/popover.rs +++ b/crates/moon-ui-components/src/moon/popover.rs @@ -7,7 +7,8 @@ use super::{ tokens::{MoonPalette, MoonRect, rgba_from}, }; -const MOON_POPOVER_PRIORITY: usize = 30_000; +/// Shared layer boundary for popovers and controls opening menus above them. +pub(super) const MOON_POPOVER_PRIORITY: usize = 30_000; const POPOVER_PADDING: f32 = 6.0; const POPOVER_BORDER: f32 = 1.0; diff --git a/crates/moon-ui-components/src/moon/select.rs b/crates/moon-ui-components/src/moon/select.rs index 5ffff7e..1611691 100644 --- a/crates/moon-ui-components/src/moon/select.rs +++ b/crates/moon-ui-components/src/moon/select.rs @@ -1,3 +1,5 @@ +//! Moon selection controls adapting core behavior and semantic overlay hosting. + use crate::searchable_list::{SearchableListItem, SearchableVec}; use crate::select::{ Select as CoreSelect, SelectEvent as CoreSelectEvent, SelectState as CoreSelectState, @@ -243,6 +245,7 @@ where } } +/// Single-selection control with an explicit opt-in for popover-hosted menus. #[derive(IntoElement)] pub struct MoonSelect where @@ -263,12 +266,14 @@ where menu_width: f32, menu_max_height: Option, menu_size: MoonMenuSize, + in_popover: bool, } impl MoonSelect where T: Clone + PartialEq + 'static, { + /// Create a select using the ordinary menu layer; popover hosts must opt in explicitly. pub fn new(state: &Entity>) -> Self { Self { id: SharedString::from(format!("moon-select:{}", state.entity_id())), @@ -286,6 +291,7 @@ where menu_width: 180.0, menu_max_height: None, menu_size: MoonMenuSize::Normal, + in_popover: false, } } @@ -358,12 +364,21 @@ where self.menu_size = size; self } + + /// Paint this select's menu above its hosting [`super::MoonPopover`]. + /// + /// Use only for controls inside a MoonPopover; ordinary selects keep their default layer. + pub fn in_popover(mut self) -> Self { + self.in_popover = true; + self + } } impl RenderOnce for MoonSelect where T: Clone + PartialEq + 'static, { + /// Forward the host's layering opt-in while preserving core selection behavior. fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { self.state.update(cx, |state, cx| { state.sync_core(self.searchable, window, cx); @@ -379,6 +394,10 @@ where .menu_width(px(self.menu_width)) .with_size(size_for(self.trigger_size, self.menu_size)); + if self.in_popover { + select = select.menu_priority(super::popover::MOON_POPOVER_PRIORITY + 1); + } + if let Some(trigger_variant) = self.trigger_variant { select = select.trigger_variant(trigger_variant.into()); } @@ -424,3 +443,6 @@ fn size_for(trigger: MoonButtonSize, _menu: MoonMenuSize) -> Size { MoonButtonSize::Custom { height, .. } => Size::Size(px(height)), } } + +#[cfg(test)] +mod tests; diff --git a/crates/moon-ui-components/src/moon/select/tests.rs b/crates/moon-ui-components/src/moon/select/tests.rs new file mode 100644 index 0000000..94bc878 --- /dev/null +++ b/crates/moon-ui-components/src/moon/select/tests.rs @@ -0,0 +1,195 @@ +//! Scene-order regression for select menus overlapping their hosting popover. + +use super::{MoonSelect, MoonSelectItem, MoonSelectState}; +use crate::moon::{IndexPath, MoonPopover, MoonTheme, MoonThemeConfig}; +use gpui::{ + AppContext as _, AtlasKey, AtlasTile, Context, DevicePixels, Entity, HeadlessAppContext, + IntoElement, NoopTextSystem, ParentElement as _, PlatformAtlas, PlatformHeadlessRenderer, Quad, + Render, Scene, Size, Styled as _, Window, div, point, px, size, +}; +use std::{borrow::Cow, cell::RefCell, rc::Rc, sync::Arc}; + +/// Records submitted quads without pretending to rasterize text or pixels. +struct SceneRecorder(Rc>>); + +impl PlatformHeadlessRenderer for SceneRecorder { + /// Record the final scene; image output is intentionally unsupported. + fn render_scene_to_image( + &mut self, + scene: &Scene, + size: Size, + ) -> anyhow::Result { + self.render_scene(scene, size)?; + anyhow::bail!("scene recorder does not rasterize images") + } + + /// Replace the preceding frame so assertions inspect only the final submission. + fn render_scene(&mut self, scene: &Scene, _size: Size) -> anyhow::Result<()> { + self.0.replace(scene.quads.clone()); + Ok(()) + } + + /// Quad ordering does not depend on glyph or icon textures. + fn sprite_atlas(&self) -> Arc { + Arc::new(QuadOnlyAtlas) + } +} + +/// Omits sprites from this quad-only scene probe. +struct QuadOnlyAtlas; + +impl PlatformAtlas for QuadOnlyAtlas { + /// No texture is allocated because the recorder only consumes solid quads. + fn get_or_insert_with<'a>( + &self, + _key: &AtlasKey, + _build: &mut dyn FnMut() -> anyhow::Result, Cow<'a, [u8]>)>>, + ) -> anyhow::Result> { + Ok(None) + } + + /// There are no allocated textures to remove. + fn remove(&self, _key: &AtlasKey) {} +} + +/// A tall popover whose surface overlaps the child select menu. +struct NestedSelectHarness { + state: Entity>, + opt_in: bool, +} + +impl Render for NestedSelectHarness { + /// Distinct fixed widths identify the actual parent and menu surface quads. + fn render(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { + let mut select = MoonSelect::new(&self.state).menu_width(180.0); + if self.opt_in { + select = select.in_popover(); + } + MoonPopover::new("nested-select") + .open(true) + .width(220.0) + .trigger(div().w(px(20.0)).h(px(20.0))) + .content( + div() + .w(px(200.0)) + .h(px(220.0)) + .child(div().w(px(180.0)).h(px(32.0)).child(select)), + ) + } +} + +/// Removing the Moon in_popover forwarding paints the overlapping menu below its parent. +/// Raising every select instead breaks the false case, which preserves ordinary paint order. +#[test] +fn popover_menu_opt_in_controls_overlapping_quad_order() { + for (theme, config) in [ + ("dark", MoonThemeConfig::moon_terminal()), + ("light", MoonThemeConfig::moon_light()), + ] { + for opt_in in [false, true] { + let quads = Rc::new(RefCell::new(Vec::new())); + let recorded = quads.clone(); + let mut cx = HeadlessAppContext::with_platform( + Arc::new(NoopTextSystem::new()), + Arc::new(()), + move || Some(Box::new(SceneRecorder(recorded.clone()))), + ); + cx.update(crate::init); + cx.update(|cx| MoonTheme::install_config(config.clone(), cx)); + let window = cx + .open_window(size(px(800.0), px(600.0)), |window, cx| { + let state = cx.new(|cx| { + MoonSelectState::new( + [ + MoonSelectItem::new(0, "First"), + MoonSelectItem::new(1, "Second"), + ], + Some(IndexPath::new(1)), + window, + cx, + ) + }); + state.update(cx, |state, _| state.set_open(true)); + cx.new(|_| NestedSelectHarness { state, opt_in }) + }) + .expect("headless window must open"); + for _ in 0..8 { + cx.update_window(window.into(), |_, window, cx| { + window.refresh(); + window.draw(cx).clear(); + }) + .expect("headless frame must draw"); + cx.run_until_parked(); + } + let scale = cx + .update_window(window.into(), |_, window, _| window.scale_factor()) + .expect("headless window must retain its scale"); + // The recorder captures submission and explicitly refuses pixel output. + let error = cx + .capture_screenshot(window.into()) + .expect_err("quad recorder has no pixels"); + assert_eq!( + error.to_string(), + "scene recorder does not rasterize images" + ); + let quads = quads.borrow(); + let parents: Vec<_> = quads + .iter() + .filter(|quad| { + !quad.background.is_transparent() + && quad.bounds.size.width.0 == 220.0 * scale + && quad.bounds.size.height.0 > 220.0 * scale + }) + .collect(); + let menus: Vec<_> = quads + .iter() + .filter(|quad| { + !quad.background.is_transparent() + && quad.bounds.size.width.0 == 180.0 * scale + && quad.bounds.size.height.0 > 40.0 * scale + }) + .collect(); + assert_eq!( + parents.len(), + 1, + "{theme}/{opt_in}: unique parent surface: {quads:?}" + ); + assert_eq!( + menus.len(), + 1, + "{theme}/{opt_in}: unique menu surface: {quads:?}" + ); + let parent = parents[0]; + let menu = menus[0]; + let target = point( + menu.bounds.center().x, + menu.bounds.top() + gpui::ScaledPixels(20.0 * scale), + ); + assert!(parent.bounds.contains(&target), "probe must overlap parent"); + assert!(menu.bounds.contains(&target), "probe must overlap menu"); + assert!( + parent.content_mask.bounds.contains(&target), + "parent must not be clipped at probe" + ); + assert!( + menu.content_mask.bounds.contains(&target), + "menu must not be clipped at probe" + ); + assert_ne!( + menu.order, parent.order, + "overlapping surfaces must have distinct draw orders" + ); + assert_eq!( + menu.order > parent.order, + opt_in, + "{theme}/{opt_in}: overlapping menu must paint above parent exactly when opted in; parent={}, menu={}", + parent.order, + menu.order + ); + eprintln!( + "{theme}/{opt_in}: parent={}, menu={}", + parent.order, menu.order + ); + } + } +} diff --git a/crates/moon-ui-components/src/select.rs b/crates/moon-ui-components/src/select.rs index 33e2f03..a2ef2af 100644 --- a/crates/moon-ui-components/src/select.rs +++ b/crates/moon-ui-components/src/select.rs @@ -1,3 +1,5 @@ +//! Base single-selection control with configurable deferred menu layering. + use gpui::{ AnyElement, App, ClickEvent, Context, DismissEvent, Edges, ElementId, Entity, EventEmitter, FocusHandle, Focusable, InteractiveElement, IntoElement, KeyBinding, Length, ParentElement, @@ -61,6 +63,7 @@ where // MARK: SelectOptions (builder only — applied to SearchableListState during render) +/// Per-render select configuration, including the host's menu layer. struct SelectOptions { style: StyleRefinement, size: Size, @@ -71,12 +74,14 @@ struct SelectOptions { search_placeholder: Option, menu_width: Length, menu_max_h: Length, + menu_priority: usize, disabled: bool, appearance: bool, trigger_variant: Option, } impl Default for SelectOptions { + /// Preserve the ordinary select layer unless a host explicitly overrides it. fn default() -> Self { Self { style: StyleRefinement::default(), @@ -87,6 +92,7 @@ impl Default for SelectOptions { title_prefix: None, menu_width: Length::Auto, menu_max_h: rems(20.).into(), + menu_priority: 1, disabled: false, appearance: true, search_placeholder: None, @@ -105,6 +111,7 @@ where pub(crate) state: SearchableListState, // Select-specific fields + menu_priority: usize, searchable: bool, icon: Option, title_prefix: Option, @@ -243,6 +250,7 @@ where Self { state, + menu_priority: SelectOptions::default().menu_priority, searchable: false, icon: None, title_prefix: None, @@ -487,6 +495,7 @@ where D: SearchableListDelegate + 'static, ::Value: PartialEq + Clone, { + /// Render the trigger and defer the open menu to its configured host layer. fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { let searchable = self.searchable; let is_focused = self.state.focus_handle.is_focused(window); @@ -616,6 +625,7 @@ where }) .child( v_flex() + .debug_selector(|| "select-menu".to_string()) .occlude() .mt_1p5() .bg(cx.theme().background) @@ -641,7 +651,7 @@ where })), ), ) - .with_priority(1), + .with_priority(self.menu_priority), ) }) } @@ -673,6 +683,12 @@ where self } + /// Set the deferred menu layer; higher priorities paint above lower ones (default: 1). + pub fn menu_priority(mut self, priority: usize) -> Self { + self.options.menu_priority = priority; + self + } + /// Set the placeholder shown when no value is selected. pub fn placeholder(mut self, placeholder: impl Into) -> Self { self.options.placeholder = Some(placeholder.into()); @@ -789,6 +805,7 @@ where D: SearchableListDelegate + 'static, ::Value: PartialEq + Clone, { + /// Apply per-render options before rendering the persistent select state. fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { let disabled = self.options.disabled; let focus_handle = self.state.focus_handle(cx); @@ -803,6 +820,7 @@ where this.state.search_placeholder = opts.search_placeholder; this.state.menu_width = opts.menu_width; this.state.menu_max_h = opts.menu_max_h; + this.menu_priority = opts.menu_priority; this.state.disabled = opts.disabled; this.state.appearance = opts.appearance; this.icon = opts.icon; diff --git a/docs/component-api-baseline.json b/docs/component-api-baseline.json index 9669ff3..5a55639 100644 --- a/docs/component-api-baseline.json +++ b/docs/component-api-baseline.json @@ -3241,6 +3241,10 @@ "file": "crates/moon-ui-components/src/moon/select.rs", "signature": "pub fn id(mut self, id: impl Into) -> Self" }, + { + "file": "crates/moon-ui-components/src/moon/select.rs", + "signature": "pub fn in_popover(mut self) -> Self" + }, { "file": "crates/moon-ui-components/src/moon/select.rs", "signature": "pub fn items(&self) -> &[MoonSelectItem]" diff --git a/docs/component-mirror-baseline.json b/docs/component-mirror-baseline.json index 21c5677..e52eea2 100644 --- a/docs/component-mirror-baseline.json +++ b/docs/component-mirror-baseline.json @@ -1214,12 +1214,12 @@ "local_paths": [ "select.rs" ], - "local_hash": "59d0c05b4ab849d3", + "local_hash": "ce7472eac2cb6c8a", "local_files": [ { "path": "select.rs", - "hash": "14ef5eaa26fe2139", - "bytes": 31301 + "hash": "1f0377cf08137e57", + "bytes": 32216 } ], "donor_hash": "245acdb36368c661",