Merge pull request #2282 from zed-industries/petros/z-283-make-pop-up-positioning-consistent

Petros Amoiridis created

Consistent pop-up menu positions

Change summary

crates/collab_ui/src/collab_titlebar_item.rs |  47 ++----
crates/workspace/src/pane.rs                 | 155 ++++++++++++++-------
styles/src/styleTree/contactsPopover.ts      |   1 
styles/src/styleTree/projectPanel.ts         | 148 ++++++++++----------
4 files changed, 195 insertions(+), 156 deletions(-)

Detailed changes

crates/collab_ui/src/collab_titlebar_item.rs 🔗

@@ -322,20 +322,7 @@ impl CollabTitlebarItem {
                 ]
             };
 
-            user_menu.show(
-                vec2f(
-                    theme
-                        .workspace
-                        .titlebar
-                        .user_menu_button
-                        .default
-                        .button_width,
-                    theme.workspace.titlebar.height,
-                ),
-                AnchorCorner::TopRight,
-                items,
-                cx,
-            );
+            user_menu.show(Default::default(), AnchorCorner::TopRight, items, cx);
         });
     }
 
@@ -402,7 +389,6 @@ impl CollabTitlebarItem {
                     theme.tooltip.clone(),
                     cx,
                 )
-                .aligned()
                 .boxed(),
             )
             .with_children(badge)
@@ -547,10 +533,15 @@ impl CollabTitlebarItem {
                 )
                 .contained()
                 .with_margin_left(theme.workspace.titlebar.item_spacing)
-                .aligned()
                 .boxed(),
             )
-            .with_child(ChildView::new(&self.user_menu, cx).boxed())
+            .with_child(
+                ChildView::new(&self.user_menu, cx)
+                    .aligned()
+                    .bottom()
+                    .right()
+                    .boxed(),
+            )
             .boxed()
     }
 
@@ -572,22 +563,18 @@ impl CollabTitlebarItem {
 
     fn render_contacts_popover_host<'a>(
         &'a self,
-        theme: &'a theme::Titlebar,
+        _theme: &'a theme::Titlebar,
         cx: &'a RenderContext<Self>,
     ) -> Option<ElementBox> {
         self.contacts_popover.as_ref().map(|popover| {
-            Overlay::new(
-                ChildView::new(popover, cx)
-                    .contained()
-                    .with_margin_top(theme.height)
-                    .with_margin_left(theme.toggle_contacts_button.default.button_width)
-                    .with_margin_right(-theme.toggle_contacts_button.default.button_width)
-                    .boxed(),
-            )
-            .with_fit_mode(OverlayFitMode::SwitchAnchor)
-            .with_anchor_corner(AnchorCorner::BottomLeft)
-            .with_z_index(999)
-            .boxed()
+            Overlay::new(ChildView::new(popover, cx).boxed())
+                .with_fit_mode(OverlayFitMode::SwitchAnchor)
+                .with_anchor_corner(AnchorCorner::TopRight)
+                .with_z_index(999)
+                .aligned()
+                .bottom()
+                .right()
+                .boxed()
         })
     }
 

crates/workspace/src/pane.rs 🔗

@@ -82,19 +82,13 @@ pub struct GoForward {
 }
 
 #[derive(Clone, PartialEq)]
-pub struct DeploySplitMenu {
-    position: Vector2F,
-}
+pub struct DeploySplitMenu;
 
 #[derive(Clone, PartialEq)]
-pub struct DeployDockMenu {
-    position: Vector2F,
-}
+pub struct DeployDockMenu;
 
 #[derive(Clone, PartialEq)]
-pub struct DeployNewMenu {
-    position: Vector2F,
-}
+pub struct DeployNewMenu;
 
 impl_actions!(pane, [GoBack, GoForward, ActivateItem]);
 impl_internal_actions!(
@@ -215,7 +209,7 @@ pub struct Pane {
     autoscroll: bool,
     nav_history: Rc<RefCell<NavHistory>>,
     toolbar: ViewHandle<Toolbar>,
-    tab_bar_context_menu: ViewHandle<ContextMenu>,
+    tab_bar_context_menu: TabBarContextMenu,
     docked: Option<DockAnchor>,
     _background_actions: BackgroundActions,
     _workspace_id: usize,
@@ -274,6 +268,27 @@ enum ItemType {
     All,
 }
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum TabBarContextMenuKind {
+    New,
+    Split,
+    Dock,
+}
+
+struct TabBarContextMenu {
+    kind: TabBarContextMenuKind,
+    handle: ViewHandle<ContextMenu>,
+}
+
+impl TabBarContextMenu {
+    fn handle_if_kind(&self, kind: TabBarContextMenuKind) -> Option<ViewHandle<ContextMenu>> {
+        if self.kind == kind {
+            return Some(self.handle.clone());
+        }
+        None
+    }
+}
+
 impl Pane {
     pub fn new(
         workspace_id: usize,
@@ -283,6 +298,10 @@ impl Pane {
     ) -> Self {
         let handle = cx.weak_handle();
         let context_menu = cx.add_view(ContextMenu::new);
+        context_menu.update(cx, |menu, _| {
+            menu.set_position_mode(OverlayPositionMode::Local)
+        });
+
         Self {
             items: Vec::new(),
             activation_history: Vec::new(),
@@ -299,7 +318,10 @@ impl Pane {
                 pane: handle.clone(),
             })),
             toolbar: cx.add_view(|_| Toolbar::new(handle)),
-            tab_bar_context_menu: context_menu,
+            tab_bar_context_menu: TabBarContextMenu {
+                kind: TabBarContextMenuKind::New,
+                handle: context_menu,
+            },
             docked,
             _background_actions: background_actions,
             _workspace_id: workspace_id,
@@ -1076,10 +1098,10 @@ impl Pane {
         cx.emit(Event::Split(direction));
     }
 
-    fn deploy_split_menu(&mut self, action: &DeploySplitMenu, cx: &mut ViewContext<Self>) {
-        self.tab_bar_context_menu.update(cx, |menu, cx| {
+    fn deploy_split_menu(&mut self, _: &DeploySplitMenu, cx: &mut ViewContext<Self>) {
+        self.tab_bar_context_menu.handle.update(cx, |menu, cx| {
             menu.show(
-                action.position,
+                Default::default(),
                 AnchorCorner::TopRight,
                 vec![
                     ContextMenuItem::item("Split Right", SplitRight),
@@ -1090,12 +1112,14 @@ impl Pane {
                 cx,
             );
         });
+
+        self.tab_bar_context_menu.kind = TabBarContextMenuKind::Split;
     }
 
-    fn deploy_dock_menu(&mut self, action: &DeployDockMenu, cx: &mut ViewContext<Self>) {
-        self.tab_bar_context_menu.update(cx, |menu, cx| {
+    fn deploy_dock_menu(&mut self, _: &DeployDockMenu, cx: &mut ViewContext<Self>) {
+        self.tab_bar_context_menu.handle.update(cx, |menu, cx| {
             menu.show(
-                action.position,
+                Default::default(),
                 AnchorCorner::TopRight,
                 vec![
                     ContextMenuItem::item("Anchor Dock Right", AnchorDockRight),
@@ -1105,12 +1129,14 @@ impl Pane {
                 cx,
             );
         });
+
+        self.tab_bar_context_menu.kind = TabBarContextMenuKind::Dock;
     }
 
-    fn deploy_new_menu(&mut self, action: &DeployNewMenu, cx: &mut ViewContext<Self>) {
-        self.tab_bar_context_menu.update(cx, |menu, cx| {
+    fn deploy_new_menu(&mut self, _: &DeployNewMenu, cx: &mut ViewContext<Self>) {
+        self.tab_bar_context_menu.handle.update(cx, |menu, cx| {
             menu.show(
-                action.position,
+                Default::default(),
                 AnchorCorner::TopRight,
                 vec![
                     ContextMenuItem::item("New File", NewFile),
@@ -1120,6 +1146,8 @@ impl Pane {
                 cx,
             );
         });
+
+        self.tab_bar_context_menu.kind = TabBarContextMenuKind::New;
     }
 
     pub fn toolbar(&self) -> &ViewHandle<Toolbar> {
@@ -1398,28 +1426,45 @@ impl Pane {
     ) -> ElementBox {
         Flex::row()
             // New menu
-            .with_child(tab_bar_button(0, "icons/plus_12.svg", cx, |position| {
-                DeployNewMenu { position }
-            }))
+            .with_child(render_tab_bar_button(
+                0,
+                "icons/plus_12.svg",
+                cx,
+                DeployNewMenu,
+                self.tab_bar_context_menu
+                    .handle_if_kind(TabBarContextMenuKind::New),
+            ))
             .with_child(
                 self.docked
                     .map(|anchor| {
                         // Add the dock menu button if this pane is a dock
                         let dock_icon = icon_for_dock_anchor(anchor);
 
-                        tab_bar_button(1, dock_icon, cx, |position| DeployDockMenu { position })
+                        render_tab_bar_button(
+                            1,
+                            dock_icon,
+                            cx,
+                            DeployDockMenu,
+                            self.tab_bar_context_menu
+                                .handle_if_kind(TabBarContextMenuKind::Dock),
+                        )
                     })
                     .unwrap_or_else(|| {
                         // Add the split menu if this pane is not a dock
-                        tab_bar_button(2, "icons/split_12.svg", cx, |position| DeploySplitMenu {
-                            position,
-                        })
+                        render_tab_bar_button(
+                            2,
+                            "icons/split_12.svg",
+                            cx,
+                            DeploySplitMenu,
+                            self.tab_bar_context_menu
+                                .handle_if_kind(TabBarContextMenuKind::Split),
+                        )
                     }),
             )
             // Add the close dock button if this pane is a dock
             .with_children(
                 self.docked
-                    .map(|_| tab_bar_button(3, "icons/x_mark_8.svg", cx, |_| HideDock)),
+                    .map(|_| render_tab_bar_button(3, "icons/x_mark_8.svg", cx, HideDock, None)),
             )
             .contained()
             .with_style(theme.workspace.tab_bar.pane_button_container)
@@ -1554,7 +1599,6 @@ impl View for Pane {
                 })
                 .boxed(),
             )
-            .with_child(ChildView::new(&self.tab_bar_context_menu, cx).boxed())
             .named("pane")
     }
 
@@ -1575,7 +1619,7 @@ impl View for Pane {
                 }
 
                 cx.focus(active_item);
-            } else if focused != self.tab_bar_context_menu {
+            } else if focused != self.tab_bar_context_menu.handle {
                 self.last_focused_view_by_item
                     .insert(active_item.id(), focused.downgrade());
             }
@@ -1591,34 +1635,41 @@ impl View for Pane {
     }
 }
 
-fn tab_bar_button<A: Action>(
+fn render_tab_bar_button<A: Action + Clone>(
     index: usize,
     icon: &'static str,
     cx: &mut RenderContext<Pane>,
-    action_builder: impl 'static + Fn(Vector2F) -> A,
+    action: A,
+    context_menu: Option<ViewHandle<ContextMenu>>,
 ) -> ElementBox {
     enum TabBarButton {}
 
-    MouseEventHandler::<TabBarButton>::new(index, cx, |mouse_state, cx| {
-        let theme = &cx.global::<Settings>().theme.workspace.tab_bar;
-        let style = theme.pane_button.style_for(mouse_state, false);
-        Svg::new(icon)
-            .with_color(style.color)
-            .constrained()
-            .with_width(style.icon_width)
-            .aligned()
-            .constrained()
-            .with_width(style.button_width)
-            .with_height(style.button_width)
-            // .aligned()
-            .boxed()
-    })
-    .with_cursor_style(CursorStyle::PointingHand)
-    .on_click(MouseButton::Left, move |e, cx| {
-        cx.dispatch_action(action_builder(e.region.lower_right()));
-    })
-    .flex(1., false)
-    .boxed()
+    Stack::new()
+        .with_child(
+            MouseEventHandler::<TabBarButton>::new(index, cx, |mouse_state, cx| {
+                let theme = &cx.global::<Settings>().theme.workspace.tab_bar;
+                let style = theme.pane_button.style_for(mouse_state, false);
+                Svg::new(icon)
+                    .with_color(style.color)
+                    .constrained()
+                    .with_width(style.icon_width)
+                    .aligned()
+                    .constrained()
+                    .with_width(style.button_width)
+                    .with_height(style.button_width)
+                    .boxed()
+            })
+            .with_cursor_style(CursorStyle::PointingHand)
+            .on_click(MouseButton::Left, move |_, cx| {
+                cx.dispatch_action(action.clone());
+            })
+            .boxed(),
+        )
+        .with_children(
+            context_menu.map(|menu| ChildView::new(menu, cx).aligned().bottom().right().boxed()),
+        )
+        .flex(1., false)
+        .boxed()
 }
 
 impl ItemNavHistory {

styles/src/styleTree/contactsPopover.ts 🔗

@@ -8,7 +8,6 @@ export default function contactsPopover(colorScheme: ColorScheme) {
         background: background(layer),
         cornerRadius: 6,
         padding: { top: 6, bottom: 6 },
-        margin: { top: -6 },
         shadow: colorScheme.popoverShadow,
         border: border(layer),
         width: 300,

styles/src/styleTree/projectPanel.ts 🔗

@@ -3,80 +3,82 @@ import { withOpacity } from "../utils/color"
 import { background, border, foreground, text } from "./components"
 
 export default function projectPanel(colorScheme: ColorScheme) {
-  let layer = colorScheme.middle
+    let layer = colorScheme.middle
 
-  let baseEntry = {
-    height: 24,
-    iconColor: foreground(layer, "variant"),
-    iconSize: 8,
-    iconSpacing: 8,
-  }
+    let baseEntry = {
+        height: 24,
+        iconColor: foreground(layer, "variant"),
+        iconSize: 8,
+        iconSpacing: 8,
+    }
 
-  let entry = {
-    ...baseEntry,
-    text: text(layer, "mono", "variant", { size: "sm" }),
-    hover: {
-      background: background(layer, "variant", "hovered"),
-    },
-    active: {
-      background: colorScheme.isLight ? withOpacity(background(layer, "active"), 0.5) : background(layer, "active"),
-      text: text(layer, "mono", "active", { size: "sm" }),
-    },
-    activeHover: {
-      background: background(layer, "active"),
-      text: text(layer, "mono", "active", { size: "sm" }),
-    },
-  }
+    let entry = {
+        ...baseEntry,
+        text: text(layer, "mono", "variant", { size: "sm" }),
+        hover: {
+            background: background(layer, "variant", "hovered"),
+        },
+        active: {
+            background: colorScheme.isLight
+                ? withOpacity(background(layer, "active"), 0.5)
+                : background(layer, "active"),
+            text: text(layer, "mono", "active", { size: "sm" }),
+        },
+        activeHover: {
+            background: background(layer, "active"),
+            text: text(layer, "mono", "active", { size: "sm" }),
+        },
+    }
 
-  return {
-    openProjectButton: {
-      background: background(layer),
-      border: border(layer, "active"),
-      cornerRadius: 4,
-      margin: {
-        top: 16,
-        left: 16,
-        right: 16,
-      },
-      padding: {
-        top: 3,
-        bottom: 3,
-        left: 7,
-        right: 7,
-      },
-      ...text(layer, "sans", "default", { size: "sm" }),
-      hover: {
-        ...text(layer, "sans", "default", { size: "sm" }),
-        background: background(layer, "hovered"),
-        border: border(layer, "active"),
-      },
-    },
-    background: background(layer),
-    padding: { left: 12, right: 12, top: 6, bottom: 6 },
-    indentWidth: 8,
-    entry,
-    draggedEntry: {
-      ...baseEntry,
-      text: text(layer, "mono", "on", { size: "sm" }),
-      background: withOpacity(background(layer, "on"), 0.9),
-      border: border(layer),
-    },
-    ignoredEntry: {
-      ...entry,
-      text: text(layer, "mono", "disabled"),
-    },
-    cutEntry: {
-      ...entry,
-      text: text(layer, "mono", "disabled"),
-      active: {
-        background: background(layer, "active"),
-        text: text(layer, "mono", "disabled", { size: "sm" }),
-      },
-    },
-    filenameEditor: {
-      background: background(layer, "on"),
-      text: text(layer, "mono", "on", { size: "sm" }),
-      selection: colorScheme.players[0],
-    },
-  }
+    return {
+        openProjectButton: {
+            background: background(layer),
+            border: border(layer, "active"),
+            cornerRadius: 4,
+            margin: {
+                top: 16,
+                left: 16,
+                right: 16,
+            },
+            padding: {
+                top: 3,
+                bottom: 3,
+                left: 7,
+                right: 7,
+            },
+            ...text(layer, "sans", "default", { size: "sm" }),
+            hover: {
+                ...text(layer, "sans", "default", { size: "sm" }),
+                background: background(layer, "hovered"),
+                border: border(layer, "active"),
+            },
+        },
+        background: background(layer),
+        padding: { left: 12, right: 12, top: 6, bottom: 6 },
+        indentWidth: 8,
+        entry,
+        draggedEntry: {
+            ...baseEntry,
+            text: text(layer, "mono", "on", { size: "sm" }),
+            background: withOpacity(background(layer, "on"), 0.9),
+            border: border(layer),
+        },
+        ignoredEntry: {
+            ...entry,
+            text: text(layer, "mono", "disabled"),
+        },
+        cutEntry: {
+            ...entry,
+            text: text(layer, "mono", "disabled"),
+            active: {
+                background: background(layer, "active"),
+                text: text(layer, "mono", "disabled", { size: "sm" }),
+            },
+        },
+        filenameEditor: {
+            background: background(layer, "on"),
+            text: text(layer, "mono", "on", { size: "sm" }),
+            selection: colorScheme.players[0],
+        },
+    }
 }