Detailed changes
@@ -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());
+}
@@ -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)
});
@@ -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,
@@ -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
@@ -110,7 +110,7 @@ pub struct Style {
/// The mouse cursor style shown when the mouse pointer is over an element.
pub mouse_cursor: Option<CursorStyle>,
- pub z_index: Option<u16>,
+ pub z_index: Option<u8>,
#[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();
@@ -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
}
@@ -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<Pixels>)>,
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<ContentMask<Pixels>>,
element_offset_stack: Vec<Point<Pixels>>,
requested_input_handler: Option<RequestedInputHandler>,
@@ -318,6 +325,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 +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<Window> + BorrowMut<AppContext> {
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<R>(&mut self, z_index: u16, f: impl FnOnce(&mut Self) -> R) -> R {
+ fn with_z_index<R>(&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)
@@ -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 }
}
}
@@ -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,
@@ -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 {
@@ -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))
@@ -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();
@@ -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,
@@ -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,
@@ -4334,7 +4334,7 @@ impl Element for DisconnectedOverlay {
}
fn paint(&mut self, bounds: Bounds<Pixels>, 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);
})