diff --git a/crates/moon-ui-components/src/combobox.rs b/crates/moon-ui-components/src/combobox.rs index 2206385..f3cac61 100644 --- a/crates/moon-ui-components/src/combobox.rs +++ b/crates/moon-ui-components/src/combobox.rs @@ -741,7 +741,7 @@ where dismiss_handler, cx, )) - .with_priority(1), + .with_priority(crate::layer::LAYER_OVERLAY), ) }) } diff --git a/crates/moon-ui-components/src/layer.rs b/crates/moon-ui-components/src/layer.rs new file mode 100644 index 0000000..1694cbd --- /dev/null +++ b/crates/moon-ui-components/src/layer.rs @@ -0,0 +1,27 @@ +//! Named deferred-draw priority bands for MoonUI overlay surfaces. +//! +//! GPUI deferred draws are sorted into one global ascending order by `priority` and painted in +//! that order (`moon-gpui` `window.rs`, `deferred_draw_traversal_order`). This file is the only +//! place these bands are numbered. A higher number paints later and therefore on top. +//! +//! The band is absolute, not relative to the hosting surface: a child overlay inside a +//! higher-band parent must opt into a band above that parent or it is painted underneath. +//! `MoonSelect::in_popover()` (`moon/select.rs:378`) is the existing opt-in; the managed +//! tooltip avoids the problem by sitting above every band unconditionally. + +/// Longbridge default overlay band: plain popovers, select menus, comboboxes, context menus, +/// native-menu fallback, and dropdown submenus. +pub(crate) const LAYER_OVERLAY: usize = 1; + +/// Moon popover and Moon dropdown surfaces. +pub(crate) const LAYER_MOON_POPOVER: usize = 30_000; + +/// A menu opened from inside a Moon popover (`MoonSelect::in_popover`). +pub(crate) const LAYER_MOON_POPOVER_MENU: usize = 31_000; + +/// The managed tooltip overlay. The tooltip is the topmost transient surface and must outrank +/// every other deferred band. +pub(crate) const LAYER_TOOLTIP: usize = 100_000; + +#[cfg(test)] +mod tests; diff --git a/crates/moon-ui-components/src/layer/tests.rs b/crates/moon-ui-components/src/layer/tests.rs new file mode 100644 index 0000000..ddc5780 --- /dev/null +++ b/crates/moon-ui-components/src/layer/tests.rs @@ -0,0 +1,20 @@ +//! Band-ordering tests for the deferred-draw layer constants live here and are authored by the +//! breakage prover. + +use super::{LAYER_MOON_POPOVER, LAYER_MOON_POPOVER_MENU, LAYER_OVERLAY, LAYER_TOOLTIP}; + +/// `layer.rs` is the only place these bands are numbered. Collapsing two bands together or +/// renumbering `LAYER_TOOLTIP` under `LAYER_MOON_POPOVER_MENU` lets a Moon popover or its +/// in-popover select menu repaint over a tooltip again, reproducing MoonUI #628. +#[test] +fn tooltip_outranks_every_other_deferred_band() { + assert!(LAYER_TOOLTIP > LAYER_MOON_POPOVER_MENU); + assert!(LAYER_MOON_POPOVER_MENU > LAYER_MOON_POPOVER); + assert!(LAYER_MOON_POPOVER > LAYER_OVERLAY); + + // Independently derived from the frozen literal at + // crates/moon-ui-components/src/time/date_picker.rs:512 (`.with_priority(2)`), a + // component-mirror donor file this ordering must never regress below. + let date_picker_calendar_priority: usize = 2; + assert!(LAYER_TOOLTIP > date_picker_calendar_priority); +} diff --git a/crates/moon-ui-components/src/lib.rs b/crates/moon-ui-components/src/lib.rs index d7d70f5..205e41c 100644 --- a/crates/moon-ui-components/src/lib.rs +++ b/crates/moon-ui-components/src/lib.rs @@ -12,6 +12,7 @@ mod icon; mod index_path; #[cfg(any(feature = "inspector", debug_assertions))] mod inspector; +mod layer; pub mod moon; mod root; mod styled; diff --git a/crates/moon-ui-components/src/menu/context_menu.rs b/crates/moon-ui-components/src/menu/context_menu.rs index 73b5a55..8b0b652 100644 --- a/crates/moon-ui-components/src/menu/context_menu.rs +++ b/crates/moon-ui-components/src/menu/context_menu.rs @@ -205,7 +205,7 @@ impl Element for ContextMenu< ), ), ) - .with_priority(1) + .with_priority(crate::layer::LAYER_OVERLAY) .into_any(), ); } diff --git a/crates/moon-ui-components/src/moon/dropdown/popup.rs b/crates/moon-ui-components/src/moon/dropdown/popup.rs index 9116c5c..77ab92a 100644 --- a/crates/moon-ui-components/src/moon/dropdown/popup.rs +++ b/crates/moon-ui-components/src/moon/dropdown/popup.rs @@ -888,7 +888,12 @@ impl MoonPopupMenu { } .into_any_element(), }) - .with_priority(1), + // Deliberately the base band even though the parent dropdown sits at + // LAYER_MOON_POPOVER (moon/dropdown/trigger.rs:654, set unconditionally), + // so an overlapping submenu paints under its parent. Pre-existing, not + // fixed here; do not "correct" it to LAYER_MOON_POPOVER without checking + // the paint-order consequences. + .with_priority(crate::layer::LAYER_OVERLAY), ); } diff --git a/crates/moon-ui-components/src/moon/dropdown/trigger.rs b/crates/moon-ui-components/src/moon/dropdown/trigger.rs index 4ffc0cc..14f1d71 100644 --- a/crates/moon-ui-components/src/moon/dropdown/trigger.rs +++ b/crates/moon-ui-components/src/moon/dropdown/trigger.rs @@ -651,7 +651,7 @@ impl RenderOnce for MoonDropdown { let mut popover = CorePopover::new(ElementId::from(self.id.clone())) .appearance(false) .anchor(Anchor::TopLeft) - .deferred_priority(30_000) + .deferred_priority(crate::layer::LAYER_MOON_POPOVER) .open(open) .trigger_any(trigger) .content(move |_, window, cx| { diff --git a/crates/moon-ui-components/src/moon/popover.rs b/crates/moon-ui-components/src/moon/popover.rs index 037c855..2a7d1b8 100644 --- a/crates/moon-ui-components/src/moon/popover.rs +++ b/crates/moon-ui-components/src/moon/popover.rs @@ -7,8 +7,6 @@ use super::{ tokens::{MoonPalette, MoonRect, rgba_from}, }; -/// 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; @@ -368,7 +366,7 @@ impl RenderOnce for MoonPopover { let mut popover = CorePopover::new(ElementId::from(self.id.clone())) .anchor(anchor_for(self.placement)) .appearance(false) - .deferred_priority(MOON_POPOVER_PRIORITY) + .deferred_priority(crate::layer::LAYER_MOON_POPOVER) .overlay_closable(self.overlay_closable) .open(open) .trigger_any(trigger) diff --git a/crates/moon-ui-components/src/moon/popover/tests.rs b/crates/moon-ui-components/src/moon/popover/tests.rs index dec2297..90b0d50 100644 --- a/crates/moon-ui-components/src/moon/popover/tests.rs +++ b/crates/moon-ui-components/src/moon/popover/tests.rs @@ -100,3 +100,220 @@ fn intrinsic_popover_shrink_wraps_its_rendered_child(cx: &mut gpui::TestAppConte assert_eq!(child.size.width, px(73.0)); assert_eq!(popup.size.width, child.size.width + px(chrome)); } + +// ---- Scene-order proof: managed tooltip must outrank its hosting Moon popover ---- + +use crate::ElementExt as _; +use crate::Root; +use crate::moon::{MoonButton, MoonThemeConfig}; +use crate::tooltip::{TooltipContent, TooltipOverlay}; +use gpui::{ + AnyWindowHandle, AppContext as _, AtlasKey, AtlasTile, Bounds, DevicePixels, + HeadlessAppContext, NoopTextSystem, ParentElement as _, Pixels, PlatformAtlas, + PlatformHeadlessRenderer, Quad, Scene, Size, size, +}; +use std::{ + borrow::Cow, + cell::{Cell, RefCell}, + rc::Rc, + sync::Arc, + time::Duration, +}; + +/// Fresh copy of moon/select/tests.rs's module-private recorder pair: exporting it would mean +/// rewriting a test file that already carries a passing test, out of scope for a proof dispatch. +struct HostedTooltipSceneRecorder(Rc>>); +impl PlatformHeadlessRenderer for HostedTooltipSceneRecorder { + 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") + } + fn render_scene(&mut self, scene: &Scene, _size: Size) -> anyhow::Result<()> { + self.0.replace(scene.quads.clone()); + Ok(()) + } + fn sprite_atlas(&self) -> Arc { + Arc::new(HostedTooltipQuadOnlyAtlas) + } +} + +struct HostedTooltipQuadOnlyAtlas; +impl PlatformAtlas for HostedTooltipQuadOnlyAtlas { + fn get_or_insert_with<'a>( + &self, + _key: &AtlasKey, + _build: &mut dyn FnMut() -> anyhow::Result, Cow<'a, [u8]>)>>, + ) -> anyhow::Result> { + Ok(None) + } + fn remove(&self, _key: &AtlasKey) {} +} + +const HOSTED_TOOLTIP_PROBE_W: f32 = 96.0; +const HOSTED_TOOLTIP_PROBE_H: f32 = 28.0; + +/// Fixed-size tooltip body so its scene quad cannot be mistaken for the popover chrome. +struct HostedTooltipProbeBody; +impl Render for HostedTooltipProbeBody { + fn render(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { + div() + .w(px(HOSTED_TOOLTIP_PROBE_W)) + .h(px(HOSTED_TOOLTIP_PROBE_H)) + .bg(gpui::black()) + } +} + +/// Always-open Moon popover hosting a button positioned well inside the popover box on every side. +struct HostedTooltipHarness { + button_bounds: Rc>>, +} +impl Render for HostedTooltipHarness { + fn render(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { + let button_bounds = self.button_bounds.clone(); + MoonPopover::new("hosted-tooltip") + .open(true) + .width(400.0) + .trigger(div().w(px(20.0)).h(px(20.0))) + .content( + div().relative().w(px(360.0)).h(px(240.0)).child( + div() + .id("hosted-tooltip:button") + .absolute() + .left(px(140.0)) + .top(px(100.0)) + .w(px(80.0)) + .h(px(32.0)) + .on_prepaint(move |bounds, _, _| button_bounds.set(bounds)) + .child(MoonButton::new("hosted-button").tooltip("Hosted tooltip")), + ), + ) + } +} + +/// Pumps N draw frames so prepaint capture and animation state settle deterministically. +fn hosted_tooltip_pump(cx: &mut HeadlessAppContext, window: AnyWindowHandle, frames: usize) { + for _ in 0..frames { + cx.update_window(window, |_, window, cx| { + window.refresh(); + window.draw(cx).clear(); + }) + .expect("headless frame must draw"); + cx.run_until_parked(); + } +} + +/// tooltip.rs TooltipOverlay::render .with_priority(LAYER_TOOLTIP) must keep the managed tooltip +/// above its hosting Moon popover (LAYER_TOOLTIP == 100_000 vs LAYER_MOON_POPOVER == 30_000). +/// Lowering it back to a literal, or pointing it at LAYER_OVERLAY, reintroduces MoonUI #628. +/// layer::tests pins the constants RELATION; this pins that the overlay actually USES it. +#[test] +fn popover_hosted_tooltip_paints_above_its_popover() { + for (theme, config) in [ + ("dark", MoonThemeConfig::moon_terminal()), + ("light", MoonThemeConfig::moon_light()), + ] { + 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(HostedTooltipSceneRecorder(recorded.clone()))), + ); + cx.update(crate::init); + cx.update(|cx| MoonTheme::install_config(config.clone(), cx)); + + let button_bounds = Rc::new(Cell::new(Bounds::default())); + let window = cx + .open_window(size(px(800.0), px(600.0)), { + let button_bounds = button_bounds.clone(); + move |window, cx| { + let view = cx.new(|_| HostedTooltipHarness { button_bounds }); + cx.new(|cx| Root::new(view, window, cx)) + } + }) + .expect("headless window must open"); + + hosted_tooltip_pump(&mut cx, window.into(), 4); + let trigger_bounds = button_bounds.get(); + + cx.update_window(window.into(), |_, window, cx| { + let overlay = + Root::tooltip_overlay(window, cx).expect("Root must own a tooltip overlay"); + overlay.update(cx, |overlay: &mut TooltipOverlay, cx| { + let content = TooltipContent { + build: Rc::new(|_, cx| cx.new(|_| HostedTooltipProbeBody).into()), + trigger_bounds, + }; + overlay.request_show(content, window, cx); + }); + }) + .expect("headless window must accept the direct request_show drive"); + + // Cold-start timer (tooltip.rs:409-434) never fires under run_until_parked alone + // (dispatcher.rs:76-78); advance the simulated clock by hand. + cx.advance_clock(Duration::from_millis(500)); + let scale = cx + .update_window(window.into(), |_, window, _| window.scale_factor()) + .expect("headless window must retain its scale"); + hosted_tooltip_pump(&mut cx, window.into(), 8); + // The recorder only fires on an actual render pass; a plain draw() never reaches it. + let _ = cx.capture_screenshot(window.into()); + + let quads = quads.borrow(); + let popovers: Vec<_> = quads + .iter() + .filter(|q| !q.background.is_transparent() && q.bounds.size.width.0 == 400.0 * scale) + .collect(); + let tooltips: Vec<_> = quads + .iter() + .filter(|q| { + !q.background.is_transparent() + && q.bounds.size.width.0 == HOSTED_TOOLTIP_PROBE_W * scale + && q.bounds.size.height.0 == HOSTED_TOOLTIP_PROBE_H * scale + }) + .collect(); + + assert_eq!( + popovers.len(), + 1, + "{theme}: unique hosting popover: {quads:?}" + ); + // Must exist BEFORE any order comparison -- a "not found" path that skips the compare + // could never redden under the named mutation. + assert_eq!( + tooltips.len(), + 1, + "{theme}: tooltip quad must exist: {quads:?}" + ); + let popover = popovers[0]; + let tooltip = tooltips[0]; + let target = tooltip.bounds.center(); + + assert!( + popover.bounds.contains(&target), + "{theme}: tooltip must render inside its popover" + ); + assert!( + popover.content_mask.bounds.contains(&target), + "{theme}: popover must not be clipped at the probe point" + ); + assert!( + tooltip.content_mask.bounds.contains(&target), + "{theme}: tooltip must not be clipped at the probe point" + ); + assert_ne!( + tooltip.order, popover.order, + "{theme}: overlapping surfaces must have distinct draw orders" + ); + assert!( + tooltip.order > popover.order, + "{theme}: tooltip must paint above its hosting popover; popover={}, tooltip={}", + popover.order, + tooltip.order + ); + } +} diff --git a/crates/moon-ui-components/src/moon/select.rs b/crates/moon-ui-components/src/moon/select.rs index 99986f9..275c845 100644 --- a/crates/moon-ui-components/src/moon/select.rs +++ b/crates/moon-ui-components/src/moon/select.rs @@ -408,7 +408,7 @@ where .with_size(size_for(trigger_size, menu_size)); if self.in_popover { - select = select.menu_priority(super::popover::MOON_POPOVER_PRIORITY + 1); + select = select.menu_priority(crate::layer::LAYER_MOON_POPOVER_MENU); } if let Some(trigger_variant) = self.trigger_variant { diff --git a/crates/moon-ui-components/src/native_menu/fallback.rs b/crates/moon-ui-components/src/native_menu/fallback.rs index 52aaf55..71b2454 100644 --- a/crates/moon-ui-components/src/native_menu/fallback.rs +++ b/crates/moon-ui-components/src/native_menu/fallback.rs @@ -107,7 +107,7 @@ impl Render for FallbackMenuOverlay { .snap_to_window_with_margin(px(8.)) .child(active.menu.clone()), ) - .with_priority(1), + .with_priority(crate::layer::LAYER_OVERLAY), ); } root diff --git a/crates/moon-ui-components/src/popover.rs b/crates/moon-ui-components/src/popover.rs index f75c834..f667635 100644 --- a/crates/moon-ui-components/src/popover.rs +++ b/crates/moon-ui-components/src/popover.rs @@ -59,7 +59,7 @@ impl Popover { appearance: true, overlay_closable: true, close_on_content_click: false, - deferred_priority: 1, + deferred_priority: crate::layer::LAYER_OVERLAY, default_open: false, open: None, on_open_change: None, @@ -153,7 +153,7 @@ impl Popover { self } - /// Override deferred overlay priority. + /// Override deferred overlay priority. The default is `LAYER_OVERLAY`. pub fn deferred_priority(mut self, priority: usize) -> Self { self.deferred_priority = priority; self diff --git a/crates/moon-ui-components/src/select.rs b/crates/moon-ui-components/src/select.rs index a2ef2af..01c6523 100644 --- a/crates/moon-ui-components/src/select.rs +++ b/crates/moon-ui-components/src/select.rs @@ -92,7 +92,7 @@ impl Default for SelectOptions { title_prefix: None, menu_width: Length::Auto, menu_max_h: rems(20.).into(), - menu_priority: 1, + menu_priority: crate::layer::LAYER_OVERLAY, disabled: false, appearance: true, search_placeholder: None, diff --git a/crates/moon-ui-components/src/tooltip.rs b/crates/moon-ui-components/src/tooltip.rs index 9671512..7424260 100644 --- a/crates/moon-ui-components/src/tooltip.rs +++ b/crates/moon-ui-components/src/tooltip.rs @@ -12,6 +12,7 @@ use crate::{ animation::{Transition, ease_in_out_cubic, ease_out_cubic}, h_flex, kbd::Kbd, + layer::LAYER_TOOLTIP, root::Root, text::Text, }; @@ -498,6 +499,14 @@ impl Render for TooltipOverlay { let is_switching = self.is_switching; let prev_trigger_bounds = self.prev_trigger_bounds; + // The managed tooltip overlay must outrank every other deferred band, including + // LAYER_MOON_POPOVER, because a tooltip is the topmost transient surface. The matching + // literal 2 in src/time/date_picker.rs:512 is deliberately left alone: that file is a + // Mirror component with a zero donor-drift budget, and any byte change there fails + // `cargo xtask component-mirror`. That literal is the date picker's calendar overlay, so + // it is still subject to this bug: a MoonDatePicker inside a MoonPopover has its calendar + // painted underneath. Fixing it needs a donor-side change or a component-class change; + // it is a follow-up, not resolved. deferred( tooltip_overlay_positioner(trigger_bounds).child(div().child(content_view).map(|el| { if is_switching { @@ -538,7 +547,7 @@ impl Render for TooltipOverlay { } })), ) - .with_priority(2) + .with_priority(LAYER_TOOLTIP) .into_any_element() } } diff --git a/crates/moon-ui-gallery/src/gallery.rs b/crates/moon-ui-gallery/src/gallery.rs index 3afd912..5d619d9 100644 --- a/crates/moon-ui-gallery/src/gallery.rs +++ b/crates/moon-ui-gallery/src/gallery.rs @@ -1229,6 +1229,7 @@ impl Gallery { MoonButton::new("popover-action") .label("Action") .variant(MoonButtonVariant::Blue) + .tooltip("Runs the popover action") .render(), ), ), diff --git a/docs/component-mirror-baseline.json b/docs/component-mirror-baseline.json index cd10992..4c46351 100644 --- a/docs/component-mirror-baseline.json +++ b/docs/component-mirror-baseline.json @@ -227,12 +227,12 @@ "local_paths": [ "combobox.rs" ], - "local_hash": "e645b6303a418133", + "local_hash": "4c7d616e46a4fae8", "local_files": [ { "path": "combobox.rs", - "hash": "709e82aad68cabcb", - "bytes": 52680 + "hash": "b87ebd3a62f14230", + "bytes": 52706 } ], "donor_hash": "2324438aee108e17", @@ -928,12 +928,12 @@ "local_paths": [ "native_menu" ], - "local_hash": "d8df00f5803a0e99", + "local_hash": "8d4cd7ecde3da41b", "local_files": [ { "path": "native_menu/fallback.rs", - "hash": "ca4beeae7b3c113f", - "bytes": 4341 + "hash": "d7aa4e43ed0816e6", + "bytes": 4367 }, { "path": "native_menu/macos.rs", @@ -1040,12 +1040,12 @@ "local_paths": [ "popover.rs" ], - "local_hash": "24b2a32c0455c8f0", + "local_hash": "529dd018e2c7f81b", "local_files": [ { "path": "popover.rs", - "hash": "0404b56976204327", - "bytes": 23015 + "hash": "b3316c5b02f1d758", + "bytes": 23073 } ], "donor_hash": "da9409c2c18e2a14", @@ -1214,12 +1214,12 @@ "local_paths": [ "select.rs" ], - "local_hash": "ce7472eac2cb6c8a", + "local_hash": "8da0aea74824b10f", "local_files": [ { "path": "select.rs", - "hash": "1f0377cf08137e57", - "bytes": 32216 + "hash": "f068cbebb4147784", + "bytes": 32242 } ], "donor_hash": "245acdb36368c661",