Add click out events to GPUI (#2659)

Mikayla Maki created

This PR adds a new mouse event type for catching when a click happens
outside of a given region.

This was added because I noticed a 'race condition' between the context
menu and the buttons which deploy a context menu. Buttons use on
an`on_click()` handler to deploy the context menu, but the context menu
was closing itself with an `on_down_out()` handler. This meant that the
order of operations was:

0. Context menu is open
1. User presses down on the button, _outside of the context menu_ 
2. `on_down_out()` is fired, closing the context menu
3. User releases the mouse
4. `click()` is fired, checks the state of the context menu, finds that
it's closed, and so opens it

You can see this behavior demonstrated with this video with a long-click
here:


https://github.com/zed-industries/zed/assets/2280405/588234c3-1567-477f-9a12-9e6a70643527

~~Switching from `on_down_out()` to `on_click_out()` means that the
click handler for the button can close the menu before the context menu
gets a chance to close itself.~~

~~However, GPUI does not have an `on_click_out()` event, hence this
PR.~~

~~Here's an example of the new behavior, with the same long-click
action:~~


https://github.com/zed-industries/zed/assets/2280405/a59f4d6f-db24-403f-a281-2c1148499413

Unfortunately, this `click_out` is the incorrect event for this to
happen on. This PR now adds a mechanism for delaying the firing of a
cancel action so that toggle buttons can signal that this on_down event
should not result in a menu closure.

Release Notes:

* Made context menus deployed from buttons toggle, instead of
hide-and-re-show, visibility on click

Change summary

crates/collab_ui/src/collab_titlebar_item.rs    |  5 +
crates/context_menu/src/context_menu.rs         | 54 ++++++++++++++++--
crates/copilot_button/src/copilot_button.rs     |  7 +
crates/gpui/src/app/window.rs                   | 13 +++-
crates/gpui/src/elements/mouse_event_handler.rs | 13 +++
crates/gpui/src/scene/mouse_event.rs            | 22 +++++++
crates/gpui/src/scene/mouse_region.rs           | 37 ++++++++++++
crates/terminal_view/src/terminal_element.rs    | 21 +++---
crates/terminal_view/src/terminal_panel.rs      |  2 
crates/workspace/src/pane.rs                    | 24 +++++++-
10 files changed, 167 insertions(+), 31 deletions(-)

Detailed changes

crates/collab_ui/src/collab_titlebar_item.rs 🔗

@@ -317,7 +317,7 @@ impl CollabTitlebarItem {
                     ),
                 ]
             };
-            user_menu.show(Default::default(), AnchorCorner::TopRight, items, cx);
+            user_menu.toggle(Default::default(), AnchorCorner::TopRight, items, cx);
         });
     }
 
@@ -683,6 +683,9 @@ impl CollabTitlebarItem {
                         .into_any()
                 })
                 .with_cursor_style(CursorStyle::PointingHand)
+                .on_down(MouseButton::Left, move |_, this, cx| {
+                    this.user_menu.update(cx, |menu, _| menu.delay_cancel());
+                })
                 .on_click(MouseButton::Left, move |_, this, cx| {
                     this.toggle_user_menu(&Default::default(), cx)
                 })

crates/context_menu/src/context_menu.rs 🔗

@@ -124,6 +124,7 @@ pub struct ContextMenu {
     items: Vec<ContextMenuItem>,
     selected_index: Option<usize>,
     visible: bool,
+    delay_cancel: bool,
     previously_focused_view_id: Option<usize>,
     parent_view_id: usize,
     _actions_observation: Subscription,
@@ -178,6 +179,7 @@ impl ContextMenu {
     pub fn new(parent_view_id: usize, cx: &mut ViewContext<Self>) -> Self {
         Self {
             show_count: 0,
+            delay_cancel: false,
             anchor_position: Default::default(),
             anchor_corner: AnchorCorner::TopLeft,
             position_mode: OverlayPositionMode::Window,
@@ -232,15 +234,23 @@ impl ContextMenu {
         }
     }
 
+    pub fn delay_cancel(&mut self) {
+        self.delay_cancel = true;
+    }
+
     fn cancel(&mut self, _: &Cancel, cx: &mut ViewContext<Self>) {
-        self.reset(cx);
-        let show_count = self.show_count;
-        cx.defer(move |this, cx| {
-            if cx.handle().is_focused(cx) && this.show_count == show_count {
-                let window_id = cx.window_id();
-                (**cx).focus(window_id, this.previously_focused_view_id.take());
-            }
-        });
+        if !self.delay_cancel {
+            self.reset(cx);
+            let show_count = self.show_count;
+            cx.defer(move |this, cx| {
+                if cx.handle().is_focused(cx) && this.show_count == show_count {
+                    let window_id = cx.window_id();
+                    (**cx).focus(window_id, this.previously_focused_view_id.take());
+                }
+            });
+        } else {
+            self.delay_cancel = false;
+        }
     }
 
     fn reset(&mut self, cx: &mut ViewContext<Self>) {
@@ -293,6 +303,34 @@ impl ContextMenu {
         }
     }
 
+    pub fn toggle(
+        &mut self,
+        anchor_position: Vector2F,
+        anchor_corner: AnchorCorner,
+        items: Vec<ContextMenuItem>,
+        cx: &mut ViewContext<Self>,
+    ) {
+        if self.visible() {
+            self.cancel(&Cancel, cx);
+        } else {
+            let mut items = items.into_iter().peekable();
+            if items.peek().is_some() {
+                self.items = items.collect();
+                self.anchor_position = anchor_position;
+                self.anchor_corner = anchor_corner;
+                self.visible = true;
+                self.show_count += 1;
+                if !cx.is_self_focused() {
+                    self.previously_focused_view_id = cx.focused_view_id();
+                }
+                cx.focus_self();
+            } else {
+                self.visible = false;
+            }
+        }
+        cx.notify();
+    }
+
     pub fn show(
         &mut self,
         anchor_position: Vector2F,

crates/copilot_button/src/copilot_button.rs 🔗

@@ -102,6 +102,9 @@ impl View for CopilotButton {
                     }
                 })
                 .with_cursor_style(CursorStyle::PointingHand)
+                .on_down(MouseButton::Left, |_, this, cx| {
+                    this.popup_menu.update(cx, |menu, _| menu.delay_cancel());
+                })
                 .on_click(MouseButton::Left, {
                     let status = status.clone();
                     move |_, this, cx| match status {
@@ -186,7 +189,7 @@ impl CopilotButton {
         }));
 
         self.popup_menu.update(cx, |menu, cx| {
-            menu.show(
+            menu.toggle(
                 Default::default(),
                 AnchorCorner::BottomRight,
                 menu_options,
@@ -266,7 +269,7 @@ impl CopilotButton {
         menu_options.push(ContextMenuItem::action("Sign Out", SignOut));
 
         self.popup_menu.update(cx, |menu, cx| {
-            menu.show(
+            menu.toggle(
                 Default::default(),
                 AnchorCorner::BottomRight,
                 menu_options,

crates/gpui/src/app/window.rs 🔗

@@ -8,8 +8,8 @@ use crate::{
         MouseButton, MouseMovedEvent, PromptLevel, WindowBounds,
     },
     scene::{
-        CursorRegion, MouseClick, MouseDown, MouseDownOut, MouseDrag, MouseEvent, MouseHover,
-        MouseMove, MouseMoveOut, MouseScrollWheel, MouseUp, MouseUpOut, Scene,
+        CursorRegion, MouseClick, MouseClickOut, MouseDown, MouseDownOut, MouseDrag, MouseEvent,
+        MouseHover, MouseMove, MouseMoveOut, MouseScrollWheel, MouseUp, MouseUpOut, Scene,
     },
     text_layout::TextLayoutCache,
     util::post_inc,
@@ -524,6 +524,10 @@ impl<'a> WindowContext<'a> {
                     region: Default::default(),
                     platform_event: e.clone(),
                 }));
+                mouse_events.push(MouseEvent::ClickOut(MouseClickOut {
+                    region: Default::default(),
+                    platform_event: e.clone(),
+                }));
             }
 
             Event::MouseMoved(
@@ -712,7 +716,10 @@ impl<'a> WindowContext<'a> {
                     }
                 }
 
-                MouseEvent::MoveOut(_) | MouseEvent::UpOut(_) | MouseEvent::DownOut(_) => {
+                MouseEvent::MoveOut(_)
+                | MouseEvent::UpOut(_)
+                | MouseEvent::DownOut(_)
+                | MouseEvent::ClickOut(_) => {
                     for (mouse_region, _) in self.window.mouse_regions.iter().rev() {
                         // NOT contains
                         if !mouse_region

crates/gpui/src/elements/mouse_event_handler.rs 🔗

@@ -7,8 +7,8 @@ use crate::{
     platform::CursorStyle,
     platform::MouseButton,
     scene::{
-        CursorRegion, HandlerSet, MouseClick, MouseDown, MouseDownOut, MouseDrag, MouseHover,
-        MouseMove, MouseMoveOut, MouseScrollWheel, MouseUp, MouseUpOut,
+        CursorRegion, HandlerSet, MouseClick, MouseClickOut, MouseDown, MouseDownOut, MouseDrag,
+        MouseHover, MouseMove, MouseMoveOut, MouseScrollWheel, MouseUp, MouseUpOut,
     },
     AnyElement, Element, EventContext, LayoutContext, MouseRegion, MouseState, SceneBuilder,
     SizeConstraint, View, ViewContext,
@@ -136,6 +136,15 @@ impl<Tag, V: View> MouseEventHandler<Tag, V> {
         self
     }
 
+    pub fn on_click_out(
+        mut self,
+        button: MouseButton,
+        handler: impl Fn(MouseClickOut, &mut V, &mut EventContext<V>) + 'static,
+    ) -> Self {
+        self.handlers = self.handlers.on_click_out(button, handler);
+        self
+    }
+
     pub fn on_down_out(
         mut self,
         button: MouseButton,

crates/gpui/src/scene/mouse_event.rs 🔗

@@ -99,6 +99,20 @@ impl Deref for MouseClick {
     }
 }
 
+#[derive(Debug, Default, Clone)]
+pub struct MouseClickOut {
+    pub region: RectF,
+    pub platform_event: MouseButtonEvent,
+}
+
+impl Deref for MouseClickOut {
+    type Target = MouseButtonEvent;
+
+    fn deref(&self) -> &Self::Target {
+        &self.platform_event
+    }
+}
+
 #[derive(Debug, Default, Clone)]
 pub struct MouseDownOut {
     pub region: RectF,
@@ -150,6 +164,7 @@ pub enum MouseEvent {
     Down(MouseDown),
     Up(MouseUp),
     Click(MouseClick),
+    ClickOut(MouseClickOut),
     DownOut(MouseDownOut),
     UpOut(MouseUpOut),
     ScrollWheel(MouseScrollWheel),
@@ -165,6 +180,7 @@ impl MouseEvent {
             MouseEvent::Down(r) => r.region = region,
             MouseEvent::Up(r) => r.region = region,
             MouseEvent::Click(r) => r.region = region,
+            MouseEvent::ClickOut(r) => r.region = region,
             MouseEvent::DownOut(r) => r.region = region,
             MouseEvent::UpOut(r) => r.region = region,
             MouseEvent::ScrollWheel(r) => r.region = region,
@@ -182,6 +198,7 @@ impl MouseEvent {
             MouseEvent::Down(_) => true,
             MouseEvent::Up(_) => true,
             MouseEvent::Click(_) => true,
+            MouseEvent::ClickOut(_) => true,
             MouseEvent::DownOut(_) => false,
             MouseEvent::UpOut(_) => false,
             MouseEvent::ScrollWheel(_) => true,
@@ -222,6 +239,10 @@ impl MouseEvent {
         discriminant(&MouseEvent::Click(Default::default()))
     }
 
+    pub fn click_out_disc() -> Discriminant<MouseEvent> {
+        discriminant(&MouseEvent::ClickOut(Default::default()))
+    }
+
     pub fn down_out_disc() -> Discriminant<MouseEvent> {
         discriminant(&MouseEvent::DownOut(Default::default()))
     }
@@ -239,6 +260,7 @@ impl MouseEvent {
             MouseEvent::Down(e) => HandlerKey::new(Self::down_disc(), Some(e.button)),
             MouseEvent::Up(e) => HandlerKey::new(Self::up_disc(), Some(e.button)),
             MouseEvent::Click(e) => HandlerKey::new(Self::click_disc(), Some(e.button)),
+            MouseEvent::ClickOut(e) => HandlerKey::new(Self::click_out_disc(), Some(e.button)),
             MouseEvent::UpOut(e) => HandlerKey::new(Self::up_out_disc(), Some(e.button)),
             MouseEvent::DownOut(e) => HandlerKey::new(Self::down_out_disc(), Some(e.button)),
             MouseEvent::ScrollWheel(_) => HandlerKey::new(Self::scroll_wheel_disc(), None),

crates/gpui/src/scene/mouse_region.rs 🔗

@@ -14,7 +14,7 @@ use super::{
         MouseClick, MouseDown, MouseDownOut, MouseDrag, MouseEvent, MouseHover, MouseMove, MouseUp,
         MouseUpOut,
     },
-    MouseMoveOut, MouseScrollWheel,
+    MouseClickOut, MouseMoveOut, MouseScrollWheel,
 };
 
 #[derive(Clone)]
@@ -89,6 +89,15 @@ impl MouseRegion {
         self
     }
 
+    pub fn on_click_out<V, F>(mut self, button: MouseButton, handler: F) -> Self
+    where
+        V: View,
+        F: Fn(MouseClickOut, &mut V, &mut EventContext<V>) + 'static,
+    {
+        self.handlers = self.handlers.on_click_out(button, handler);
+        self
+    }
+
     pub fn on_down_out<V, F>(mut self, button: MouseButton, handler: F) -> Self
     where
         V: View,
@@ -246,6 +255,10 @@ impl HandlerSet {
                 HandlerKey::new(MouseEvent::click_disc(), Some(button)),
                 SmallVec::from_buf([Rc::new(|_, _, _, _| true)]),
             );
+            set.insert(
+                HandlerKey::new(MouseEvent::click_out_disc(), Some(button)),
+                SmallVec::from_buf([Rc::new(|_, _, _, _| true)]),
+            );
             set.insert(
                 HandlerKey::new(MouseEvent::down_out_disc(), Some(button)),
                 SmallVec::from_buf([Rc::new(|_, _, _, _| true)]),
@@ -405,6 +418,28 @@ impl HandlerSet {
         self
     }
 
+    pub fn on_click_out<V, F>(mut self, button: MouseButton, handler: F) -> Self
+    where
+        V: View,
+        F: Fn(MouseClickOut, &mut V, &mut EventContext<V>) + 'static,
+    {
+        self.insert(MouseEvent::click_out_disc(), Some(button),
+            Rc::new(move |region_event, view, cx, view_id| {
+                if let MouseEvent::ClickOut(e) = region_event {
+                    let view = view.downcast_mut().unwrap();
+                    let mut cx = ViewContext::mutable(cx, view_id);
+                    let mut cx = EventContext::new(&mut cx);
+                    handler(e, view, &mut cx);
+                    cx.handled
+                } else {
+                    panic!(
+                        "Mouse Region Event incorrectly called with mismatched event type. Expected MouseRegionEvent::ClickOut, found {:?}",
+                        region_event);
+                }
+            }));
+        self
+    }
+
     pub fn on_down_out<V, F>(mut self, button: MouseButton, handler: F) -> Self
     where
         V: View,

crates/terminal_view/src/terminal_element.rs 🔗

@@ -395,16 +395,17 @@ impl TerminalElement {
         // Terminal Emulator controlled behavior:
         region = region
             // Start selections
-            .on_down(
-                MouseButton::Left,
-                TerminalElement::generic_button_handler(
-                    connection,
-                    origin,
-                    move |terminal, origin, e, _cx| {
-                        terminal.mouse_down(&e, origin);
-                    },
-                ),
-            )
+            .on_down(MouseButton::Left, move |event, v: &mut TerminalView, cx| {
+                cx.focus_parent();
+                v.context_menu.update(cx, |menu, _cx| menu.delay_cancel());
+                if let Some(conn_handle) = connection.upgrade(cx) {
+                    conn_handle.update(cx, |terminal, cx| {
+                        terminal.mouse_down(&event, origin);
+
+                        cx.notify();
+                    })
+                }
+            })
             // Update drag selections
             .on_drag(MouseButton::Left, move |event, _: &mut TerminalView, cx| {
                 if cx.is_self_focused() {

crates/terminal_view/src/terminal_panel.rs 🔗

@@ -87,6 +87,7 @@ impl TerminalPanel {
                                 }
                             })
                         },
+                        |_, _| {},
                         None,
                     ))
                     .with_child(Pane::render_tab_bar_button(
@@ -100,6 +101,7 @@ impl TerminalPanel {
                         Some(("Toggle Zoom".into(), Some(Box::new(workspace::ToggleZoom)))),
                         cx,
                         move |pane, cx| pane.toggle_zoom(&Default::default(), cx),
+                        |_, _| {},
                         None,
                     ))
                     .into_any()

crates/workspace/src/pane.rs 🔗

@@ -273,6 +273,11 @@ impl Pane {
                         Some(("New...".into(), None)),
                         cx,
                         |pane, cx| pane.deploy_new_menu(cx),
+                        |pane, cx| {
+                            pane.tab_bar_context_menu
+                                .handle
+                                .update(cx, |menu, _| menu.delay_cancel())
+                        },
                         pane.tab_bar_context_menu
                             .handle_if_kind(TabBarContextMenuKind::New),
                     ))
@@ -283,6 +288,11 @@ impl Pane {
                         Some(("Split Pane".into(), None)),
                         cx,
                         |pane, cx| pane.deploy_split_menu(cx),
+                        |pane, cx| {
+                            pane.tab_bar_context_menu
+                                .handle
+                                .update(cx, |menu, _| menu.delay_cancel())
+                        },
                         pane.tab_bar_context_menu
                             .handle_if_kind(TabBarContextMenuKind::Split),
                     ))
@@ -304,6 +314,7 @@ impl Pane {
                             Some((tooltip_label, Some(Box::new(ToggleZoom)))),
                             cx,
                             move |pane, cx| pane.toggle_zoom(&Default::default(), cx),
+                            move |_, _| {},
                             None,
                         )
                     })
@@ -988,7 +999,7 @@ impl Pane {
 
     fn deploy_split_menu(&mut self, cx: &mut ViewContext<Self>) {
         self.tab_bar_context_menu.handle.update(cx, |menu, cx| {
-            menu.show(
+            menu.toggle(
                 Default::default(),
                 AnchorCorner::TopRight,
                 vec![
@@ -1006,7 +1017,7 @@ impl Pane {
 
     fn deploy_new_menu(&mut self, cx: &mut ViewContext<Self>) {
         self.tab_bar_context_menu.handle.update(cx, |menu, cx| {
-            menu.show(
+            menu.toggle(
                 Default::default(),
                 AnchorCorner::TopRight,
                 vec![
@@ -1416,13 +1427,17 @@ impl Pane {
             .into_any()
     }
 
-    pub fn render_tab_bar_button<F: 'static + Fn(&mut Pane, &mut EventContext<Pane>)>(
+    pub fn render_tab_bar_button<
+        F1: 'static + Fn(&mut Pane, &mut EventContext<Pane>),
+        F2: 'static + Fn(&mut Pane, &mut EventContext<Pane>),
+    >(
         index: usize,
         icon: &'static str,
         is_active: bool,
         tooltip: Option<(String, Option<Box<dyn Action>>)>,
         cx: &mut ViewContext<Pane>,
-        on_click: F,
+        on_click: F1,
+        on_down: F2,
         context_menu: Option<ViewHandle<ContextMenu>>,
     ) -> AnyElement<Pane> {
         enum TabBarButton {}
@@ -1440,6 +1455,7 @@ impl Pane {
                 .with_height(style.button_width)
         })
         .with_cursor_style(CursorStyle::PointingHand)
+        .on_down(MouseButton::Left, move |_, pane, cx| on_down(pane, cx))
         .on_click(MouseButton::Left, move |_, pane, cx| on_click(pane, cx))
         .into_any();
         if let Some((tooltip, action)) = tooltip {