diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index e68fd10d8d16b29300c3fa658596468df06be880..a8e52f4094c83ac79a29c9b30eae666566de96e2 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -7,7 +7,10 @@ use client::{User, RECEIVE_TIMEOUT}; use collections::{HashMap, HashSet}; use fs::{repository::GitFileStatus, FakeFs, Fs as _, RemoveOptions}; use futures::StreamExt as _; -use gpui::{AppContext, BackgroundExecutor, Model, TestAppContext}; +use gpui::{ + px, size, AppContext, BackgroundExecutor, Model, Modifiers, MouseButton, MouseDownEvent, + TestAppContext, +}; use language::{ language_settings::{AllLanguageSettings, Formatter}, tree_sitter_rust, Diagnostic, DiagnosticEntry, FakeLspAdapter, Language, LanguageConfig, @@ -5903,3 +5906,42 @@ async fn test_join_call_after_screen_was_shared( ); }); } + +#[gpui::test] +async fn test_right_click_menu_behind_collab_panel(cx: &mut TestAppContext) { + let mut server = TestServer::start(cx.executor().clone()).await; + let client_a = server.create_client(cx, "user_a").await; + let (_workspace_a, cx) = client_a.build_test_workspace(cx).await; + + cx.simulate_resize(size(px(300.), px(300.))); + + cx.simulate_keystrokes("cmd-n cmd-n cmd-n"); + cx.update(|cx| cx.refresh()); + + let tab_bounds = cx.debug_bounds("TAB-2").unwrap(); + let new_tab_button_bounds = cx.debug_bounds("ICON-Plus").unwrap(); + + assert!( + tab_bounds.intersects(&new_tab_button_bounds), + "Tab should overlap with the new tab button, if this is failing check if there's been a redesign!" + ); + + cx.simulate_event(MouseDownEvent { + button: MouseButton::Right, + position: new_tab_button_bounds.center(), + modifiers: Modifiers::default(), + click_count: 1, + }); + + // regression test that the right click menu for tabs does not open. + assert!(cx.debug_bounds("MENU_ITEM-Close").is_none()); + + let tab_bounds = cx.debug_bounds("TAB-1").unwrap(); + cx.simulate_event(MouseDownEvent { + button: MouseButton::Right, + position: tab_bounds.center(), + modifiers: Modifiers::default(), + click_count: 1, + }); + assert!(cx.debug_bounds("MENU_ITEM-Close").is_some()); +} diff --git a/crates/collab_ui/src/face_pile.rs b/crates/collab_ui/src/face_pile.rs index 31132b298148944a95cde28918c51bcf94c33766..fb6c59cc8079073acbb6b481e214b54619f9eea6 100644 --- a/crates/collab_ui/src/face_pile.rs +++ b/crates/collab_ui/src/face_pile.rs @@ -13,7 +13,7 @@ impl RenderOnce for FacePile { let isnt_last = ix < player_count - 1; div() - .z_index((player_count - ix) as u16) + .z_index((player_count - ix) as u8) .when(isnt_last, |div| div.neg_mr_1()) .child(player) }); diff --git a/crates/gpui/src/app/test_context.rs b/crates/gpui/src/app/test_context.rs index d95558f058a91bb4a7cd9a3ab347ee10584bffba..d11c1239dd576d9db944f605cad7d4140624feb9 100644 --- a/crates/gpui/src/app/test_context.rs +++ b/crates/gpui/src/app/test_context.rs @@ -2,7 +2,7 @@ use crate::{ Action, AnyElement, AnyView, AnyWindowHandle, AppCell, AppContext, AsyncAppContext, - AvailableSpace, BackgroundExecutor, ClipboardItem, Context, Entity, EventEmitter, + AvailableSpace, BackgroundExecutor, Bounds, ClipboardItem, Context, Entity, EventEmitter, ForegroundExecutor, InputEvent, Keystroke, Model, ModelContext, Pixels, Platform, Point, Render, Result, Size, Task, TestDispatcher, TestPlatform, TestWindow, TextSystem, View, ViewContext, VisualContext, WindowContext, WindowHandle, WindowOptions, @@ -618,6 +618,16 @@ impl<'a> VisualTestContext { self.cx.simulate_input(self.window, input) } + /// Simulates the user resizing the window to the new size. + pub fn simulate_resize(&self, size: Size) { + self.simulate_window_resize(self.window, size) + } + + /// debug_bounds returns the bounds of the element with the given selector. + pub fn debug_bounds(&mut self, selector: &'static str) -> Option> { + self.update(|cx| cx.window.rendered_frame.debug_bounds.get(selector).copied()) + } + /// Draw an element to the window. Useful for simulating events or actions pub fn draw( &mut self, diff --git a/crates/gpui/src/elements/div.rs b/crates/gpui/src/elements/div.rs index 74000da0512ffac35e4cf87c122bdecf08d67aed..aa912eadbe9c986969dd197927af8963a3c58d0c 100644 --- a/crates/gpui/src/elements/div.rs +++ b/crates/gpui/src/elements/div.rs @@ -416,6 +416,18 @@ pub trait InteractiveElement: Sized { self } + #[cfg(any(test, feature = "test-support"))] + fn debug_selector(mut self, f: impl FnOnce() -> String) -> Self { + self.interactivity().debug_selector = Some(f()); + self + } + + #[cfg(not(any(test, feature = "test-support")))] + #[inline] + fn debug_selector(self, _: impl FnOnce() -> String) -> Self { + self + } + fn capture_any_mouse_down( mut self, listener: impl Fn(&MouseDownEvent, &mut WindowContext) + 'static, @@ -911,6 +923,9 @@ pub struct Interactivity { #[cfg(debug_assertions)] pub location: Option>, + + #[cfg(any(test, feature = "test-support"))] + pub debug_selector: Option, } #[derive(Clone, Debug)] @@ -980,6 +995,14 @@ impl Interactivity { let style = self.compute_style(Some(bounds), element_state, cx); let z_index = style.z_index.unwrap_or(0); + #[cfg(any(feature = "test-support", test))] + if let Some(debug_selector) = &self.debug_selector { + cx.window + .next_frame + .debug_bounds + .insert(debug_selector.clone(), bounds); + } + let paint_hover_group_handler = |cx: &mut WindowContext| { let hover_group_bounds = self .group_hover_style diff --git a/crates/gpui/src/style.rs b/crates/gpui/src/style.rs index bfc36ef6b116f355be5911151a32831cbd73f362..095233280edefc0b11d85e3a4ee255f54c8da13d 100644 --- a/crates/gpui/src/style.rs +++ b/crates/gpui/src/style.rs @@ -110,7 +110,7 @@ pub struct Style { /// The mouse cursor style shown when the mouse pointer is over an element. pub mouse_cursor: Option, - pub z_index: Option, + pub z_index: Option, #[cfg(debug_assertions)] pub debug: bool, @@ -386,7 +386,7 @@ impl Style { let background_color = self.background.as_ref().and_then(Fill::color); if background_color.map_or(false, |color| !color.is_transparent()) { - cx.with_z_index(1, |cx| { + cx.with_z_index(0, |cx| { let mut border_color = background_color.unwrap_or_default(); border_color.a = 0.; cx.paint_quad(quad( @@ -399,12 +399,12 @@ impl Style { }); } - cx.with_z_index(2, |cx| { + cx.with_z_index(0, |cx| { continuation(cx); }); if self.is_border_visible() { - cx.with_z_index(3, |cx| { + cx.with_z_index(0, |cx| { let corner_radii = self.corner_radii.to_pixels(bounds.size, rem_size); let border_widths = self.border_widths.to_pixels(rem_size); let max_border_width = border_widths.max(); diff --git a/crates/gpui/src/styled.rs b/crates/gpui/src/styled.rs index e8800d1ce9dea7831e7c5ab186c96def45434d1b..0eba1771f52d47bde32f465a887e52547f3a89b2 100644 --- a/crates/gpui/src/styled.rs +++ b/crates/gpui/src/styled.rs @@ -12,7 +12,7 @@ pub trait Styled: Sized { gpui_macros::style_helpers!(); - fn z_index(mut self, z_index: u16) -> Self { + fn z_index(mut self, z_index: u8) -> Self { self.style().z_index = Some(z_index); self } diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs index c61379e814656b45e96de5fa506d90fe6cb6d43c..2329a5251ed010791e043bc08b70fb2e5e19d895 100644 --- a/crates/gpui/src/window.rs +++ b/crates/gpui/src/window.rs @@ -30,7 +30,7 @@ use std::{ borrow::{Borrow, BorrowMut, Cow}, cell::RefCell, collections::hash_map::Entry, - fmt::Debug, + fmt::{Debug, Display}, future::Future, hash::{Hash, Hasher}, marker::PhantomData, @@ -43,23 +43,30 @@ use std::{ }; use util::{post_inc, ResultExt}; -const ACTIVE_DRAG_Z_INDEX: u16 = 1; +const ACTIVE_DRAG_Z_INDEX: u8 = 1; /// A global stacking order, which is created by stacking successive z-index values. /// Each z-index will always be interpreted in the context of its parent z-index. -#[derive(Debug, Deref, DerefMut, Clone, Ord, PartialOrd, PartialEq, Eq, Default)] -pub struct StackingOrder(SmallVec<[StackingContext; 64]>); - -/// A single entry in a primitive's z-index stacking order -#[derive(Clone, Ord, PartialOrd, PartialEq, Eq, Default)] -pub struct StackingContext { - z_index: u16, - id: u16, +#[derive(Deref, DerefMut, Clone, Ord, PartialOrd, PartialEq, Eq, Default)] +pub struct StackingOrder { + #[deref] + #[deref_mut] + context_stack: SmallVec<[u8; 64]>, + id: u32, } -impl std::fmt::Debug for StackingContext { +impl std::fmt::Debug for StackingOrder { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{{{}.{}}} ", self.z_index, self.id) + let mut stacks = self.context_stack.iter().peekable(); + write!(f, "[({}): ", self.id)?; + while let Some(z_index) = stacks.next() { + write!(f, "{z_index}")?; + if stacks.peek().is_some() { + write!(f, "->")?; + } + } + write!(f, "]")?; + Ok(()) } } @@ -308,8 +315,8 @@ pub(crate) struct Frame { pub(crate) scene: Scene, pub(crate) depth_map: Vec<(StackingOrder, EntityId, Bounds)>, pub(crate) z_index_stack: StackingOrder, - next_stacking_order_id: u16, - next_root_z_index: u16, + pub(crate) next_stacking_order_id: u32, + next_root_z_index: u8, content_mask_stack: Vec>, element_offset_stack: Vec>, requested_input_handler: Option, @@ -318,6 +325,9 @@ pub(crate) struct Frame { requested_cursor_style: Option, pub(crate) view_stack: Vec, pub(crate) reused_views: FxHashSet, + + #[cfg(any(test, feature = "test-support"))] + pub(crate) debug_bounds: collections::FxHashMap>, } impl Frame { @@ -341,6 +351,9 @@ impl Frame { requested_cursor_style: None, view_stack: Vec::new(), reused_views: FxHashSet::default(), + + #[cfg(any(test, feature = "test-support"))] + debug_bounds: FxHashMap::default(), } } @@ -1098,11 +1111,7 @@ impl<'a> WindowContext<'a> { if level >= opaque_level { break; } - if opaque_level - .first() - .map(|c| c.z_index == ACTIVE_DRAG_Z_INDEX) - .unwrap_or(false) - { + if opaque_level.starts_with(&[ACTIVE_DRAG_Z_INDEX]) { continue; } @@ -2449,40 +2458,36 @@ pub trait BorrowWindow: BorrowMut + BorrowMut { size: self.window().viewport_size, }, }; - - let new_root_z_index = post_inc(&mut self.window_mut().next_frame.next_root_z_index); let new_stacking_order_id = post_inc(&mut self.window_mut().next_frame.next_stacking_order_id); - let new_context = StackingContext { - z_index: new_root_z_index, - id: new_stacking_order_id, - }; - + let new_root_z_index = post_inc(&mut self.window_mut().next_frame.next_root_z_index); let old_stacking_order = mem::take(&mut self.window_mut().next_frame.z_index_stack); - - self.window_mut().next_frame.z_index_stack.push(new_context); + self.window_mut().next_frame.z_index_stack.id = new_stacking_order_id; + self.window_mut() + .next_frame + .z_index_stack + .push(new_root_z_index); self.window_mut().next_frame.content_mask_stack.push(mask); let result = f(self); self.window_mut().next_frame.content_mask_stack.pop(); self.window_mut().next_frame.z_index_stack = old_stacking_order; - result } /// Called during painting to invoke the given closure in a new stacking context. The given /// z-index is interpreted relative to the previous call to `stack`. - fn with_z_index(&mut self, z_index: u16, f: impl FnOnce(&mut Self) -> R) -> R { + fn with_z_index(&mut self, z_index: u8, f: impl FnOnce(&mut Self) -> R) -> R { let new_stacking_order_id = post_inc(&mut self.window_mut().next_frame.next_stacking_order_id); - let new_context = StackingContext { - z_index, - id: new_stacking_order_id, - }; - - self.window_mut().next_frame.z_index_stack.push(new_context); + let old_stacking_order_id = mem::replace( + &mut self.window_mut().next_frame.z_index_stack.id, + new_stacking_order_id, + ); + self.window_mut().next_frame.z_index_stack.id = new_stacking_order_id; + self.window_mut().next_frame.z_index_stack.push(z_index); let result = f(self); + self.window_mut().next_frame.z_index_stack.id = old_stacking_order_id; self.window_mut().next_frame.z_index_stack.pop(); - result } @@ -3380,6 +3385,20 @@ pub enum ElementId { NamedInteger(SharedString, usize), } +impl Display for ElementId { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ElementId::View(entity_id) => write!(f, "view-{}", entity_id)?, + ElementId::Integer(ix) => write!(f, "{}", ix)?, + ElementId::Name(name) => write!(f, "{}", name)?, + ElementId::FocusHandle(__) => write!(f, "FocusHandle")?, + ElementId::NamedInteger(s, i) => write!(f, "{}-{}", s, i)?, + } + + Ok(()) + } +} + impl ElementId { pub(crate) fn from_entity_id(entity_id: EntityId) -> Self { ElementId::View(entity_id) diff --git a/crates/storybook/src/stories/z_index.rs b/crates/storybook/src/stories/z_index.rs index 63ee1af7591ee8a62f07f052b320db8a70077b9d..b6e49bfae32b046242e96a5de34d30c3be806b94 100644 --- a/crates/storybook/src/stories/z_index.rs +++ b/crates/storybook/src/stories/z_index.rs @@ -76,7 +76,7 @@ impl Styles for Div {} #[derive(IntoElement)] struct ZIndexExample { - z_index: u16, + z_index: u8, } impl RenderOnce for ZIndexExample { @@ -166,7 +166,7 @@ impl RenderOnce for ZIndexExample { } impl ZIndexExample { - pub fn new(z_index: u16) -> Self { + pub fn new(z_index: u8) -> Self { Self { z_index } } } diff --git a/crates/ui/src/components/button/button_like.rs b/crates/ui/src/components/button/button_like.rs index c2910acfc04bee67630c3d55b2ce2394559379f6..aafb33cd6f541a7573f7910fc559a8d170416689 100644 --- a/crates/ui/src/components/button/button_like.rs +++ b/crates/ui/src/components/button/button_like.rs @@ -293,7 +293,7 @@ impl ButtonSize { /// This is also used to build the prebuilt buttons. #[derive(IntoElement)] pub struct ButtonLike { - base: Div, + pub base: Div, id: ElementId, pub(super) style: ButtonStyle, pub(super) disabled: bool, diff --git a/crates/ui/src/components/button/icon_button.rs b/crates/ui/src/components/button/icon_button.rs index cc1e31b65cb0836be9a307107302ba8705597d6c..6de32c0eab29a22da541e2617d1adbe60f3656b2 100644 --- a/crates/ui/src/components/button/icon_button.rs +++ b/crates/ui/src/components/button/icon_button.rs @@ -24,14 +24,16 @@ pub struct IconButton { impl IconButton { pub fn new(id: impl Into, icon: IconName) -> Self { - Self { + let mut this = Self { base: ButtonLike::new(id), shape: IconButtonShape::Wide, icon, icon_size: IconSize::default(), icon_color: Color::Default, selected_icon: None, - } + }; + this.base.base = this.base.base.debug_selector(|| format!("ICON-{:?}", icon)); + this } pub fn shape(mut self, shape: IconButtonShape) -> Self { diff --git a/crates/ui/src/components/context_menu.rs b/crates/ui/src/components/context_menu.rs index 4b6837799976579b6bc469753c51cb964e49e7b0..470483cc0a713cbfc53d1a9e7db343a604918a2e 100644 --- a/crates/ui/src/components/context_menu.rs +++ b/crates/ui/src/components/context_menu.rs @@ -303,6 +303,7 @@ impl Render for ContextMenu { .w_full() .justify_between() .child(label_element) + .debug_selector(|| format!("MENU_ITEM-{}", label)) .children(action.as_ref().and_then(|action| { KeyBinding::for_action(&**action, cx) .map(|binding| div().ml_1().child(binding)) diff --git a/crates/ui/src/components/right_click_menu.rs b/crates/ui/src/components/right_click_menu.rs index 55cdd93a5bee9d4be4c3f293a262b868b6b7825a..9d32073dbdb077614f1c23dda58ce0d4bae35ce8 100644 --- a/crates/ui/src/components/right_click_menu.rs +++ b/crates/ui/src/components/right_click_menu.rs @@ -1,9 +1,9 @@ use std::{cell::RefCell, rc::Rc}; use gpui::{ - overlay, AnchorCorner, AnyElement, Bounds, DismissEvent, DispatchPhase, Element, ElementId, - IntoElement, LayoutId, ManagedView, MouseButton, MouseDownEvent, ParentElement, Pixels, Point, - View, VisualContext, WindowContext, + overlay, AnchorCorner, AnyElement, BorrowWindow, Bounds, DismissEvent, DispatchPhase, Element, + ElementId, InteractiveBounds, IntoElement, LayoutId, ManagedView, MouseButton, MouseDownEvent, + ParentElement, Pixels, Point, View, VisualContext, WindowContext, }; pub struct RightClickMenu { @@ -136,10 +136,14 @@ impl Element for RightClickMenu { let child_layout_id = element_state.child_layout_id.clone(); let child_bounds = cx.layout_bounds(child_layout_id.unwrap()); + let interactive_bounds = InteractiveBounds { + bounds: bounds.intersect(&cx.content_mask().bounds), + stacking_order: cx.stacking_order().clone(), + }; cx.on_mouse_event(move |event: &MouseDownEvent, phase, cx| { if phase == DispatchPhase::Bubble && event.button == MouseButton::Right - && bounds.contains(&event.position) + && interactive_bounds.visibly_contains(&event.position, cx) { cx.stop_propagation(); cx.prevent_default(); diff --git a/crates/ui/src/components/tab.rs b/crates/ui/src/components/tab.rs index ade939fdaabcddfcdc6ac0a22b0222ff4f2b4170..7f1fcca721b9524b8879f2c6051e7481dc8db6ab 100644 --- a/crates/ui/src/components/tab.rs +++ b/crates/ui/src/components/tab.rs @@ -37,8 +37,11 @@ pub struct Tab { impl Tab { pub fn new(id: impl Into) -> Self { + let id = id.into(); Self { - div: div().id(id), + div: div() + .id(id.clone()) + .debug_selector(|| format!("TAB-{}", id)), selected: false, position: TabPosition::First, close_side: TabCloseSide::End, diff --git a/crates/ui/src/styles/elevation.rs b/crates/ui/src/styles/elevation.rs index c2605fd152df49d04db57d6784bc3a54aaefd80d..0aa3786a279242c8a3a301b497a60ab0c2c537ec 100644 --- a/crates/ui/src/styles/elevation.rs +++ b/crates/ui/src/styles/elevation.rs @@ -20,7 +20,7 @@ pub enum ElevationIndex { } impl ElevationIndex { - pub fn z_index(self) -> u16 { + pub fn z_index(self) -> u8 { match self { ElevationIndex::Background => 0, ElevationIndex::Surface => 42, diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index a8aaa403fbf5d4cc11af3abe10f567f049138035..20c8bfc94a8adffb1eeb680d95c4f6dc602b34f5 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -4334,7 +4334,7 @@ impl Element for DisconnectedOverlay { } fn paint(&mut self, bounds: Bounds, overlay: &mut Self::State, cx: &mut WindowContext) { - cx.with_z_index(u16::MAX, |cx| { + cx.with_z_index(u8::MAX, |cx| { cx.add_opaque_layer(bounds); overlay.paint(cx); })