Tidy up z-index handling

Co-Authored-By: Antonio Scandurra <antonio@zed.dev>

crates/collab_ui2/src/collab_titlebar_item.rs

@@ -68,7 +68,7 @@ impl Render for CollabTitlebarItem {
 
         h_stack()
             .id("titlebar")
-            .z_index(160)
+            .z_index(160) // todo!("z-index")
             .justify_between()
             .w_full()
             .h(rems(1.75))

crates/gpui2/src/elements/div.rs

@@ -1448,14 +1448,15 @@ impl Interactivity {
                     }
 
                     cx.with_z_index(style.z_index.unwrap_or(0), |cx| {
-                    if style.background.as_ref().is_some_and(|fill| {
+                        if style.background.as_ref().is_some_and(|fill| {
-                        fill.color().is_some_and(|color| !color.is_transparent())
+                            fill.color().is_some_and(|color| !color.is_transparent())
-                    }) {
+                        }) {
-                        cx.add_opaque_layer(bounds)
+                            cx.add_opaque_layer(bounds)
-                    }f(style, scroll_offset.unwrap_or_default(), cx)
+                        }
-                })
+                        f(style, scroll_offset.unwrap_or_default(), cx)
-            },
+                    })
-        );
+                },
+            );
 
             if let Some(group) = self.group.as_ref() {
                 GroupBounds::pop(group, cx);

crates/gpui2/src/elements/img.rs

@@ -1,7 +1,7 @@
 use std::sync::Arc;
 
 use crate::{
-    point, size, Bounds, DevicePixels, Element, ImageData, InteractiveElement,
+    point, size, BorrowWindow, Bounds, DevicePixels, Element, ImageData, InteractiveElement,
     InteractiveElementState, Interactivity, IntoElement, LayoutId, Pixels, SharedString, Size,
     StyleRefinement, Styled, WindowContext,
 };

crates/gpui2/src/scene.rs

@@ -856,117 +856,3 @@ impl Bounds<ScaledPixels> {
         .expect("Polygon should not be empty")
     }
 }
-
-// todo!()
-// #[cfg(test)]
-// mod tests {
-//     use super::*;
-//     use crate::{point, px, size, Size};
-//     use smallvec::smallvec;
-
-// #[test]
-// fn test_scene() {
-//     let mut scene = SceneBuilder::default();
-//     assert_eq!(scene.layers_by_order.len(), 0);
-
-//     // div with z_index(1)
-//     //     glyph with z_index(1)
-//     // div with z_index(1)
-//     //      glyph with z_index(1)
-
-//     scene.insert(
-//         &smallvec![1].into(),
-//         quad(
-//             point(px(0.), px(0.)),
-//             size(px(100.), px(100.)),
-//             crate::black(),
-//         ),
-//     );
-//     scene.insert(
-//         &smallvec![1, 1].into(),
-//         sprite(
-//             point(px(0.), px(0.)),
-//             size(px(10.), px(10.)),
-//             crate::white(),
-//         ),
-//     );
-//     scene.insert(
-//         &smallvec![1].into(),
-//         quad(
-//             point(px(10.), px(10.)),
-//             size(px(20.), px(20.)),
-//             crate::green(),
-//         ),
-//     );
-//     scene.insert(
-//         &smallvec![1, 1].into(),
-//         sprite(point(px(15.), px(15.)), size(px(5.), px(5.)), crate::blue()),
-//     );
-
-//     assert!(!scene.layers_by_order.is_empty());
-
-//     for batch in scene.build().batches() {
-//         println!("new batch");
-//         match batch {
-//             PrimitiveBatch::Quads(quads) => {
-//                 for quad in quads {
-//                     if quad.background == crate::black() {
-//                         println!("  black quad");
-//                     } else if quad.background == crate::green() {
-//                         println!("  green quad");
-//                     } else {
-//                         todo!("  ((( bad quad");
-//                     }
-//                 }
-//             }
-//             PrimitiveBatch::MonochromeSprites { sprites, .. } => {
-//                 for sprite in sprites {
-//                     if sprite.color == crate::white() {
-//                         println!("  white sprite");
-//                     } else if sprite.color == crate::blue() {
-//                         println!("  blue sprite");
-//                     } else {
-//                         todo!("  ((( bad sprite")
-//                     }
-//                 }
-//             }
-//             _ => todo!(),
-//         }
-//     }
-// }
-
-//     fn quad(origin: Point<Pixels>, size: Size<Pixels>, background: Hsla) -> Quad {
-//         Quad {
-//             order: 0,
-//             bounds: Bounds { origin, size }.scale(1.),
-//             background,
-//             content_mask: ContentMask {
-//                 bounds: Bounds { origin, size },
-//             }
-//             .scale(1.),
-//             border_color: Default::default(),
-//             corner_radii: Default::default(),
-//             border_widths: Default::default(),
-//         }
-//     }
-
-//     fn sprite(origin: Point<Pixels>, size: Size<Pixels>, color: Hsla) -> MonochromeSprite {
-//         MonochromeSprite {
-//             order: 0,
-//             bounds: Bounds { origin, size }.scale(1.),
-//             content_mask: ContentMask {
-//                 bounds: Bounds { origin, size },
-//             }
-//             .scale(1.),
-//             color,
-//             tile: AtlasTile {
-//                 texture_id: AtlasTextureId {
-//                     index: 0,
-//                     kind: crate::AtlasTextureKind::Monochrome,
-//                 },
-//                 tile_id: crate::TileId(0),
-//                 bounds: Default::default(),
-//             },
-//         }
-//     }
-// }

crates/gpui2/src/window.rs

@@ -39,7 +39,7 @@ use std::{
         Arc,
     },
 };
-use util::ResultExt;
+use util::{post_inc, ResultExt};
 
 const ACTIVE_DRAG_Z_INDEX: u8 = 1;
 
@@ -939,21 +939,6 @@ impl<'a> WindowContext<'a> {
         self.window.requested_cursor_style = Some(style)
     }
 
-    /// 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`.
-    pub fn with_z_index<R>(&mut self, z_index: u8, f: impl FnOnce(&mut Self) -> R) -> R {
-        let new_stacking_order_id = self.window.next_frame.next_stacking_order_id;
-        let new_next_stacking_order_id = new_stacking_order_id + 1;
-
-        self.window.next_frame.next_stacking_order_id = 0;
-        self.window.next_frame.z_index_stack.id = new_stacking_order_id;
-        self.window.next_frame.z_index_stack.push(z_index);
-        let result = f(self);
-        self.window.next_frame.next_stacking_order_id = new_next_stacking_order_id;
-        self.window.next_frame.z_index_stack.pop();
-        result
-    }
-
     /// Called during painting to track which z-index is on top at each pixel position
     pub fn add_opaque_layer(&mut self, bounds: Bounds<Pixels>) {
         let stacking_order = self.window.next_frame.z_index_stack.clone();
@@ -2066,14 +2051,11 @@ pub trait BorrowWindow: BorrowMut<Window> + BorrowMut<AppContext> {
     /// 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: u8, f: impl FnOnce(&mut Self) -> R) -> R {
-        let new_stacking_order_id = self.window_mut().next_frame.next_stacking_order_id;
+        let new_stacking_order_id =
-        let new_next_stacking_order_id = new_stacking_order_id + 1;
+            post_inc(&mut self.window_mut().next_frame.next_stacking_order_id);
-
-        self.window_mut().next_frame.next_stacking_order_id = 0;
         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.next_stacking_order_id = new_next_stacking_order_id;
         self.window_mut().next_frame.z_index_stack.pop();
         result
     }

crates/terminal_view2/src/terminal_element.rs

@@ -1,11 +1,11 @@
 use editor::{Cursor, HighlightedRange, HighlightedRangeLine};
 use gpui::{
     black, div, fill, point, px, red, relative, AnyElement, AsyncWindowContext, AvailableSpace,
-    Bounds, DispatchPhase, Element, ElementId, ExternalPaths, FocusHandle, Font, FontStyle,
+    BorrowWindow, Bounds, DispatchPhase, Element, ElementId, ExternalPaths, FocusHandle, Font,
-    FontWeight, HighlightStyle, Hsla, InteractiveElement, InteractiveElementState, Interactivity,
+    FontStyle, FontWeight, HighlightStyle, Hsla, InteractiveElement, InteractiveElementState,
-    IntoElement, LayoutId, Model, ModelContext, ModifiersChangedEvent, MouseButton, Pixels,
+    Interactivity, IntoElement, LayoutId, Model, ModelContext, ModifiersChangedEvent, MouseButton,
-    PlatformInputHandler, Point, Rgba, ShapedLine, Size, StatefulInteractiveElement, Styled,
+    Pixels, PlatformInputHandler, Point, Rgba, ShapedLine, Size, StatefulInteractiveElement,
-    TextRun, TextStyle, TextSystem, UnderlineStyle, WhiteSpace, WindowContext,
+    Styled, TextRun, TextStyle, TextSystem, UnderlineStyle, WhiteSpace, WindowContext,
 };
 use itertools::Itertools;
 use language::CursorShape;

crates/ui2/src/components/context_menu.rs

@@ -3,8 +3,8 @@ use crate::{
     ListSeparator, ListSubHeader,
 };
 use gpui::{
-    px, Action, AnyElement, AppContext, DismissEvent, Div, EventEmitter, FocusHandle,
+    px, Action, AppContext, DismissEvent, Div, EventEmitter, FocusHandle, FocusableView,
-    FocusableView, IntoElement, Render, Subscription, View, VisualContext,
+    IntoElement, Render, Subscription, View, VisualContext,
 };
 use menu::{SelectFirst, SelectLast, SelectNext, SelectPrev};
 use std::{rc::Rc, time::Duration};
@@ -18,9 +18,6 @@ pub enum ContextMenuItem {
         handler: Rc<dyn Fn(&mut WindowContext)>,
         action: Option<Box<dyn Action>>,
     },
-    CustomEntry {
-        entry_render: Box<dyn Fn(&mut WindowContext) -> AnyElement>,
-    },
 }
 
 pub struct ContextMenu {
@@ -86,16 +83,6 @@ impl ContextMenu {
         self
     }
 
-    pub fn custom_entry(
-        mut self,
-        entry_render: impl Fn(&mut WindowContext) -> AnyElement + 'static,
-    ) -> Self {
-        self.items.push(ContextMenuItem::CustomEntry {
-            entry_render: Box::new(entry_render),
-        });
-        self
-    }
-
     pub fn action(mut self, label: impl Into<SharedString>, action: Box<dyn Action>) -> Self {
         self.items.push(ContextMenuItem::Entry {
             label: label.into(),
@@ -243,9 +230,9 @@ impl Render for ContextMenu {
                     el
                 })
                 .flex_none()
-                .child(List::new().children(self.items.iter_mut().enumerate().map(
+                .child(
-                    |(ix, item)| {
+                    List::new().children(self.items.iter().enumerate().map(
-                        match item {
+                        |(ix, item)| match item {
                             ContextMenuItem::Separator => ListSeparator.into_any_element(),
                             ContextMenuItem::Header(header) => {
                                 ListSubHeader::new(header.clone()).into_any_element()
@@ -268,7 +255,7 @@ impl Render for ContextMenu {
                                     Label::new(label.clone()).into_any_element()
                                 };
 
-                                ListItem::new(ix)
+                                ListItem::new(label.clone())
                                     .inset(true)
                                     .selected(Some(ix) == self.selected_index)
                                     .on_click(move |_, cx| handler(cx))
@@ -284,14 +271,9 @@ impl Render for ContextMenu {
                                     )
                                     .into_any_element()
                             }
-                            ContextMenuItem::CustomEntry { entry_render } => ListItem::new(ix)
+                        },
-                                .inset(true)
+                    )),
-                                .selected(Some(ix) == self.selected_index)
+                ),
-                                .child(entry_render(cx))
-                                .into_any_element(),
-                        }
-                    },
-                ))),
         )
     }
 }

crates/ui2/src/components/list/list_item.rs

@@ -171,8 +171,7 @@ impl RenderOnce for ListItem {
                             })
                     })
                     .when_some(self.on_click, |this, on_click| {
-                        this.cursor_pointer()
+                        this.cursor_pointer().on_click(on_click)
-                            .on_click(move |event, cx| on_click(event, cx))
                     })
                     .when_some(self.on_secondary_mouse_down, |this, on_mouse_down| {
                         this.on_mouse_down(MouseButton::Right, move |event, cx| {

crates/ui2/src/components/tab_bar.rs

@@ -96,7 +96,7 @@ impl RenderOnce for TabBar {
 
         div()
             .id(self.id)
-            .z_index(120)
+            .z_index(120) // todo!("z-index")
             .group("tab_bar")
             .flex()
             .flex_none()

crates/workspace2/src/toolbar.rs

@@ -105,8 +105,7 @@ impl Render for Toolbar {
         v_stack()
             .p_1()
             .gap_2()
-            // todo!() use a proper constant here (ask Marshall & Nate)
+            .z_index(80) // todo!("z-index")
-            .z_index(80)
             .border_b()
             .border_color(cx.theme().colors().border_variant)
             .bg(cx.theme().colors().toolbar_background)