Fix right click handler for tabs (#4130)

Also, some fun test helpers

Co-Authored-By: Mikayla <mikayla@zed.dev>

You can now use .debug_selector() to make it possible for tests to find
a given element,
and .debug_bounds() to find the coordinates of where it was painted.


Release Notes:

- (Added|Fixed|Improved) ...
([#<public_issue_number_if_exists>](https://github.com/zed-industries/community/issues/<public_issue_number_if_exists>)).

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());
+}

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<Pixels>) {
+        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<Bounds<Pixels>> {
+        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,

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<core::panic::Location<'static>>,
+
+    #[cfg(any(test, feature = "test-support"))]
+    pub debug_selector: Option<String>,
 }
 
 #[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

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,
@@ -318,6 +318,9 @@ pub(crate) struct Frame {
     requested_cursor_style: Option<CursorStyle>,
     pub(crate) view_stack: Vec<EntityId>,
     pub(crate) reused_views: FxHashSet<EntityId>,
+
+    #[cfg(any(test, feature = "test-support"))]
+    pub(crate) debug_bounds: collections::FxHashMap<String, Bounds<Pixels>>,
 }
 
 impl Frame {
@@ -341,6 +344,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(),
         }
     }
 
@@ -3380,6 +3386,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)

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,

crates/ui/src/components/button/icon_button.rs

@@ -24,14 +24,16 @@ pub struct IconButton {
 
 impl IconButton {
     pub fn new(id: impl Into<ElementId>, 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 {

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))

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<M: ManagedView> {
@@ -136,10 +136,14 @@ impl<M: ManagedView> Element for RightClickMenu<M> {
         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();

crates/ui/src/components/tab.rs

@@ -37,8 +37,11 @@ pub struct Tab {
 
 impl Tab {
     pub fn new(id: impl Into<ElementId>) -> 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,