Switch to using mouse navigation events instead of other in order to get rid of opaque button id

crates/gpui/src/elements/event_handler.rs

@@ -3,7 +3,7 @@ use serde_json::json;
 
 use crate::{
     geometry::vector::Vector2F, DebugContext, Element, ElementBox, Event, EventContext,
-    LayoutContext, PaintContext, SizeConstraint,
+    LayoutContext, NavigationDirection, PaintContext, SizeConstraint,
 };
 
 pub struct EventHandler {
@@ -11,7 +11,7 @@ pub struct EventHandler {
     capture: Option<Box<dyn FnMut(&Event, RectF, &mut EventContext) -> bool>>,
     mouse_down: Option<Box<dyn FnMut(&mut EventContext) -> bool>>,
     right_mouse_down: Option<Box<dyn FnMut(&mut EventContext) -> bool>>,
-    other_mouse_down: Option<Box<dyn FnMut(u16, &mut EventContext) -> bool>>,
+    navigate_mouse_down: Option<Box<dyn FnMut(NavigationDirection, &mut EventContext) -> bool>>,
 }
 
 impl EventHandler {
@@ -21,7 +21,7 @@ impl EventHandler {
             capture: None,
             mouse_down: None,
             right_mouse_down: None,
-            other_mouse_down: None,
+            navigate_mouse_down: None,
         }
     }
 
@@ -41,11 +41,11 @@ impl EventHandler {
         self
     }
 
-    pub fn on_other_mouse_down<F>(mut self, callback: F) -> Self
+    pub fn on_navigate_mouse_down<F>(mut self, callback: F) -> Self
     where
-        F: 'static + FnMut(u16, &mut EventContext) -> bool,
+        F: 'static + FnMut(NavigationDirection, &mut EventContext) -> bool,
     {
-        self.other_mouse_down = Some(Box::new(callback));
+        self.navigate_mouse_down = Some(Box::new(callback));
         self
     }
 
@@ -106,7 +106,7 @@ impl Element for EventHandler {
                         }
                     }
                     false
-                },
+                }
                 Event::RightMouseDown { position, .. } => {
                     if let Some(callback) = self.right_mouse_down.as_mut() {
                         if bounds.contains_point(*position) {
@@ -114,15 +114,19 @@ impl Element for EventHandler {
                         }
                     }
                     false
-                },
+                }
-                Event::OtherMouseDown { position, button, .. } => {
+                Event::NavigateMouseDown {
-                    if let Some(callback) = self.other_mouse_down.as_mut() {
+                    position,
+                    direction,
+                    ..
+                } => {
+                    if let Some(callback) = self.navigate_mouse_down.as_mut() {
                         if bounds.contains_point(*position) {
-                            return callback(*button, cx);
+                            return callback(*direction, cx);
                         }
                     }
                     false
-                },
+                }
                 _ => false,
             }
         }

crates/gpui/src/gpui.rs

@@ -29,7 +29,7 @@ pub mod keymap;
 pub mod platform;
 pub use gpui_macros::test;
 pub use platform::FontSystem;
-pub use platform::{Event, PathPromptOptions, Platform, PromptLevel};
+pub use platform::{Event, NavigationDirection, PathPromptOptions, Platform, PromptLevel};
 pub use presenter::{
     Axis, DebugContext, EventContext, LayoutContext, PaintContext, SizeConstraint, Vector2FExt,
 };

crates/gpui/src/platform.rs

@@ -19,7 +19,7 @@ use crate::{
 };
 use anyhow::Result;
 use async_task::Runnable;
-pub use event::Event;
+pub use event::{Event, NavigationDirection};
 use postage::oneshot;
 use std::{
     any::Any,

crates/gpui/src/platform/event.rs

@@ -1,5 +1,11 @@
 use crate::{geometry::vector::Vector2F, keymap::Keystroke};
 
+#[derive(Copy, Clone, Debug)]
+pub enum NavigationDirection {
+    Back,
+    Forward,
+}
+
 #[derive(Clone, Debug)]
 pub enum Event {
     KeyDown {
@@ -37,18 +43,18 @@ pub enum Event {
     RightMouseUp {
         position: Vector2F,
     },
-    OtherMouseDown {
+    NavigateMouseDown {
         position: Vector2F,
-        button: u16,
+        direction: NavigationDirection,
         ctrl: bool,
         alt: bool,
         shift: bool,
         cmd: bool,
         click_count: usize,
     },
-    OtherMouseUp {
+    NavigateMouseUp {
         position: Vector2F,
-        button: u16,
+        direction: NavigationDirection,
     },
     MouseMoved {
         position: Vector2F,

crates/gpui/src/platform/mac/event.rs

@@ -1,4 +1,8 @@
-use crate::{geometry::vector::vec2f, keymap::Keystroke, platform::Event};
+use crate::{
+    geometry::vector::vec2f,
+    keymap::Keystroke,
+    platform::{Event, NavigationDirection},
+};
 use cocoa::{
     appkit::{NSEvent, NSEventModifierFlags, NSEventType},
     base::{id, nil, YES},
@@ -146,13 +150,20 @@ impl Event {
                 ),
             }),
             NSEventType::NSOtherMouseDown => {
+                let direction = match native_event.buttonNumber() {
+                    3 => NavigationDirection::Back,
+                    4 => NavigationDirection::Forward,
+                    // Other mouse buttons aren't tracked currently
+                    _ => return None,
+                };
+
                 let modifiers = native_event.modifierFlags();
-                window_height.map(|window_height| Self::OtherMouseDown {
+                window_height.map(|window_height| Self::NavigateMouseDown {
                     position: vec2f(
                         native_event.locationInWindow().x as f32,
                         window_height - native_event.locationInWindow().y as f32,
                     ),
-                    button: native_event.buttonNumber() as u16,
+                    direction,
                     ctrl: modifiers.contains(NSEventModifierFlags::NSControlKeyMask),
                     alt: modifiers.contains(NSEventModifierFlags::NSAlternateKeyMask),
                     shift: modifiers.contains(NSEventModifierFlags::NSShiftKeyMask),
@@ -160,13 +171,22 @@ impl Event {
                     click_count: native_event.clickCount() as usize,
                 })
             }
-            NSEventType::NSOtherMouseUp => window_height.map(|window_height| Self::OtherMouseUp {
+            NSEventType::NSOtherMouseUp => {
-                position: vec2f(
+                let direction = match native_event.buttonNumber() {
-                    native_event.locationInWindow().x as f32,
+                    3 => NavigationDirection::Back,
-                    window_height - native_event.locationInWindow().y as f32,
+                    4 => NavigationDirection::Forward,
-                ),
+                    // Other mouse buttons aren't tracked currently
-                button: native_event.buttonNumber() as u16,
+                    _ => return None,
-            }),
+                };
+
+                window_height.map(|window_height| Self::NavigateMouseUp {
+                    position: vec2f(
+                        native_event.locationInWindow().x as f32,
+                        window_height - native_event.locationInWindow().y as f32,
+                    ),
+                    direction,
+                })
+            }
             NSEventType::NSLeftMouseDragged => {
                 window_height.map(|window_height| Self::LeftMouseDragged {
                     position: vec2f(

crates/workspace/src/pane.rs

@@ -6,7 +6,7 @@ use gpui::{
     elements::*,
     geometry::{rect::RectF, vector::vec2f},
     keymap::Binding,
-    platform::CursorStyle,
+    platform::{CursorStyle, NavigationDirection},
     AnyViewHandle, Entity, MutableAppContext, Quad, RenderContext, Task, View, ViewContext,
     ViewHandle, WeakViewHandle,
 };
@@ -57,16 +57,24 @@ pub fn init(cx: &mut MutableAppContext) {
     cx.add_action(|workspace: &mut Workspace, action: &GoBack, cx| {
         Pane::go_back(
             workspace,
-            action.0.as_ref().and_then(|weak_handle| weak_handle.upgrade(cx)),
+            action
-            cx
+                .0
-        ).detach();
+                .as_ref()
+                .and_then(|weak_handle| weak_handle.upgrade(cx)),
+            cx,
+        )
+        .detach();
     });
     cx.add_action(|workspace: &mut Workspace, action: &GoForward, cx| {
         Pane::go_forward(
             workspace,
-            action.0.as_ref().and_then(|weak_handle| weak_handle.upgrade(cx)),
+            action
-            cx
+                .0
-        ).detach();
+                .as_ref()
+                .and_then(|weak_handle| weak_handle.upgrade(cx)),
+            cx,
+        )
+        .detach();
     });
 
     cx.add_bindings(vec![
@@ -171,7 +179,11 @@ impl Pane {
         cx.emit(Event::Activate);
     }
 
-    pub fn go_back(workspace: &mut Workspace, pane: Option<ViewHandle<Pane>>, cx: &mut ViewContext<Workspace>) -> Task<()> {
+    pub fn go_back(
+        workspace: &mut Workspace,
+        pane: Option<ViewHandle<Pane>>,
+        cx: &mut ViewContext<Workspace>,
+    ) -> Task<()> {
         Self::navigate_history(
             workspace,
             pane.unwrap_or_else(|| workspace.active_pane().clone()),
@@ -180,7 +192,11 @@ impl Pane {
         )
     }
 
-    pub fn go_forward(workspace: &mut Workspace, pane: Option<ViewHandle<Pane>>, cx: &mut ViewContext<Workspace>) -> Task<()> {
+    pub fn go_forward(
+        workspace: &mut Workspace,
+        pane: Option<ViewHandle<Pane>>,
+        cx: &mut ViewContext<Workspace>,
+    ) -> Task<()> {
         Self::navigate_history(
             workspace,
             pane.unwrap_or_else(|| workspace.active_pane().clone()),
@@ -646,31 +662,29 @@ impl View for Pane {
     fn render(&mut self, cx: &mut RenderContext<Self>) -> ElementBox {
         let this = cx.handle();
 
-        EventHandler::new(
+        EventHandler::new(if let Some(active_item) = self.active_item() {
-            if let Some(active_item) = self.active_item() {
+            Flex::column()
-                Flex::column()
+                .with_child(self.render_tabs(cx))
-                    .with_child(self.render_tabs(cx))
+                .with_children(
-                    .with_children(
+                    self.active_toolbar()
-                        self.active_toolbar()
+                        .as_ref()
-                            .as_ref()
+                        .map(|view| ChildView::new(view).boxed()),
-                            .map(|view| ChildView::new(view).boxed()),
+                )
-                    )
+                .with_child(ChildView::new(active_item).flexible(1., true).boxed())
-                    .with_child(ChildView::new(active_item).flexible(1., true).boxed())
+                .boxed()
-                    .boxed()
+        } else {
-            } else {
+            Empty::new().boxed()
-                Empty::new().boxed()
+        })
+        .on_navigate_mouse_down(move |direction, cx| {
+            let this = this.clone();
+            match direction {
+                NavigationDirection::Back => cx.dispatch_action(GoBack(Some(this))),
+                NavigationDirection::Forward => cx.dispatch_action(GoForward(Some(this))),
             }
-        )
+
-        .on_other_mouse_down(move |button, cx| {
-            match button {
-                3 => cx.dispatch_action(GoBack(Some(this.clone()))),
-                4 => cx.dispatch_action(GoForward(Some(this.clone()))),
-                _ => return false,
-            };
             true
         })
         .named("pane")
-
     }
 
     fn on_focus(&mut self, cx: &mut ViewContext<Self>) {

crates/zed/src/zed.rs

@@ -747,44 +747,58 @@ mod tests {
             (file3.clone(), DisplayPoint::new(15, 0))
         );
 
-        workspace.update(cx, |w, cx| Pane::go_back(w, None, cx)).await;
+        workspace
+            .update(cx, |w, cx| Pane::go_back(w, None, cx))
+            .await;
         assert_eq!(
             active_location(&workspace, cx),
             (file3.clone(), DisplayPoint::new(0, 0))
         );
 
-        workspace.update(cx, |w, cx| Pane::go_back(w, None, cx)).await;
+        workspace
+            .update(cx, |w, cx| Pane::go_back(w, None, cx))
+            .await;
         assert_eq!(
             active_location(&workspace, cx),
             (file2.clone(), DisplayPoint::new(0, 0))
         );
 
-        workspace.update(cx, |w, cx| Pane::go_back(w, None, cx)).await;
+        workspace
+            .update(cx, |w, cx| Pane::go_back(w, None, cx))
+            .await;
         assert_eq!(
             active_location(&workspace, cx),
             (file1.clone(), DisplayPoint::new(10, 0))
         );
 
-        workspace.update(cx, |w, cx| Pane::go_back(w, None, cx)).await;
+        workspace
+            .update(cx, |w, cx| Pane::go_back(w, None, cx))
+            .await;
         assert_eq!(
             active_location(&workspace, cx),
             (file1.clone(), DisplayPoint::new(0, 0))
         );
 
         // Go back one more time and ensure we don't navigate past the first item in the history.
-        workspace.update(cx, |w, cx| Pane::go_back(w, None, cx)).await;
+        workspace
+            .update(cx, |w, cx| Pane::go_back(w, None, cx))
+            .await;
         assert_eq!(
             active_location(&workspace, cx),
             (file1.clone(), DisplayPoint::new(0, 0))
         );
 
-        workspace.update(cx, |w, cx| Pane::go_forward(w, None, cx)).await;
+        workspace
+            .update(cx, |w, cx| Pane::go_forward(w, None, cx))
+            .await;
         assert_eq!(
             active_location(&workspace, cx),
             (file1.clone(), DisplayPoint::new(10, 0))
         );
 
-        workspace.update(cx, |w, cx| Pane::go_forward(w, None, cx)).await;
+        workspace
+            .update(cx, |w, cx| Pane::go_forward(w, None, cx))
+            .await;
         assert_eq!(
             active_location(&workspace, cx),
             (file2.clone(), DisplayPoint::new(0, 0))
@@ -798,7 +812,9 @@ mod tests {
                 .update(cx, |pane, cx| pane.close_item(editor3.id(), cx));
             drop(editor3);
         });
-        workspace.update(cx, |w, cx| Pane::go_forward(w, None, cx)).await;
+        workspace
+            .update(cx, |w, cx| Pane::go_forward(w, None, cx))
+            .await;
         assert_eq!(
             active_location(&workspace, cx),
             (file3.clone(), DisplayPoint::new(0, 0))
@@ -818,12 +834,16 @@ mod tests {
             })
             .await
             .unwrap();
-        workspace.update(cx, |w, cx| Pane::go_back(w, None, cx)).await;
+        workspace
+            .update(cx, |w, cx| Pane::go_back(w, None, cx))
+            .await;
         assert_eq!(
             active_location(&workspace, cx),
             (file1.clone(), DisplayPoint::new(10, 0))
         );
-        workspace.update(cx, |w, cx| Pane::go_forward(w, None, cx)).await;
+        workspace
+            .update(cx, |w, cx| Pane::go_forward(w, None, cx))
+            .await;
         assert_eq!(
             active_location(&workspace, cx),
             (file3.clone(), DisplayPoint::new(0, 0))