Use click event to determine modifier keys

Previously, editor elements had to listen for mouse_up
events to determine when a click had completed. This meant
that they only had access to modifier keys that were
pressed during the mouse_up event.

This led to some incorrect user experiences, such as
executing a ctrl+click if the user pressed ctrl after
pressing the mouse button, but before releasing it.

This change adds a click event handler to EditorElement,
and adds a modifier() method to the ClickEvent, which
only includes the modifier keys that were pressed during
both mouse down and mouse up. The code for handling link
clicks has been moved into the click event handler, so that
it's only triggered when the non-multi-cursor modifier was
held for both the mouse down and mouse up events.

crates/editor/src/editor.rs

@@ -81,10 +81,10 @@ use gpui::{
     AsyncWindowContext, AvailableSpace, Bounds, ClipboardEntry, ClipboardItem, Context,
     DispatchPhase, ElementId, EventEmitter, FocusHandle, FocusOutEvent, FocusableView, FontId,
     FontWeight, Global, HighlightStyle, Hsla, InteractiveText, KeyContext, Model, ModelContext,
-    MouseButton, PaintQuad, ParentElement, Pixels, Render, SharedString, Size, Styled, StyledText,
-    Subscription, Task, TextStyle, TextStyleRefinement, UTF16Selection, UnderlineStyle,
-    UniformListScrollHandle, View, ViewContext, ViewInputHandler, VisualContext, WeakFocusHandle,
-    WeakView, WindowContext,
+    MouseButton, MouseDownEvent, PaintQuad, ParentElement, Pixels, Render, SharedString, Size,
+    Styled, StyledText, Subscription, Task, TextStyle, TextStyleRefinement, UTF16Selection,
+    UnderlineStyle, UniformListScrollHandle, View, ViewContext, ViewInputHandler, VisualContext,
+    WeakFocusHandle, WeakView, WindowContext,
 };
 use highlight_matching_bracket::refresh_matching_bracket_highlights;
 use hover_popover::{hide_hover, HoverState};
@@ -648,6 +648,7 @@ pub struct Editor {
     leader_peer_id: Option<PeerId>,
     remote_id: Option<ViewId>,
     hover_state: HoverState,
+    pending_mouse_down: Option<Rc<RefCell<Option<MouseDownEvent>>>>,
     gutter_hovered: bool,
     hovered_link_state: Option<HoveredLinkState>,
     inline_completion_provider: Option<RegisteredInlineCompletionProvider>,
@@ -1297,6 +1298,7 @@ impl Editor {
             leader_peer_id: None,
             remote_id: None,
             hover_state: Default::default(),
+            pending_mouse_down: None,
             hovered_link_state: Default::default(),
             inline_completion_provider: None,
             active_inline_completion: None,

crates/editor/src/element.rs

@@ -677,18 +677,7 @@ impl EditorElement {
             editor.select(SelectPhase::End, cx);
         }
 
-        let multi_cursor_setting = EditorSettings::get_global(cx).multi_cursor_modifier;
-        let multi_cursor_modifier = match multi_cursor_setting {
-            MultiCursorModifier::Alt => event.modifiers.secondary(),
-            MultiCursorModifier::CmdOrCtrl => event.modifiers.alt,
-        };
-
-        if !pending_nonempty_selections && multi_cursor_modifier && text_hitbox.is_hovered(cx) {
-            let point = position_map.point_for_position(text_hitbox.bounds, event.position);
-            editor.handle_click_hovered_link(point, event.modifiers, cx);
-
-            cx.stop_propagation();
-        } else if end_selection && pending_nonempty_selections {
+        if end_selection && pending_nonempty_selections {
             cx.stop_propagation();
         } else if cfg!(any(target_os = "linux", target_os = "freebsd"))
             && event.button == MouseButton::Middle
@@ -719,6 +708,29 @@ impl EditorElement {
         }
     }
 
+    fn click(
+        editor: &mut Editor,
+        event: &ClickEvent,
+        position_map: &PositionMap,
+        text_hitbox: &Hitbox,
+        cx: &mut ViewContext<Editor>,
+    ) {
+        let pending_nonempty_selections = editor.has_pending_nonempty_selection();
+
+        let multi_cursor_setting = EditorSettings::get_global(cx).multi_cursor_modifier;
+        let multi_cursor_modifier = match multi_cursor_setting {
+            MultiCursorModifier::Alt => event.modifiers().secondary(),
+            MultiCursorModifier::CmdOrCtrl => event.modifiers().alt,
+        };
+
+        if !pending_nonempty_selections && multi_cursor_modifier && text_hitbox.is_hovered(cx) {
+            let point = position_map.point_for_position(text_hitbox.bounds, event.up.position);
+            editor.handle_click_hovered_link(point, event.modifiers(), cx);
+
+            cx.stop_propagation();
+        }
+    }
+
     fn mouse_dragged(
         editor: &mut Editor,
         event: &MouseMoveEvent,
@@ -4995,6 +5007,13 @@ impl EditorElement {
                 if phase == DispatchPhase::Bubble {
                     match event.button {
                         MouseButton::Left => editor.update(cx, |editor, cx| {
+                            let pending_mouse_down = editor
+                                .pending_mouse_down
+                                .get_or_insert_with(Default::default)
+                                .clone();
+
+                            *pending_mouse_down.borrow_mut() = Some(event.clone());
+
                             Self::mouse_left_down(
                                 editor,
                                 event,
@@ -5031,6 +5050,43 @@ impl EditorElement {
                 }
             }
         });
+
+        cx.on_mouse_event({
+            let editor = self.editor.clone();
+            let position_map = layout.position_map.clone();
+            let text_hitbox = layout.text_hitbox.clone();
+
+            let mut captured_mouse_down = None;
+
+            move |event: &MouseUpEvent, phase, cx| match phase {
+                // Clear the pending mouse down during the capture phase,
+                // so that it happens even if another event handler stops
+                // propagation.
+                DispatchPhase::Capture => editor.update(cx, |editor, cx| {
+                    let pending_mouse_down = editor
+                        .pending_mouse_down
+                        .get_or_insert_with(Default::default)
+                        .clone();
+
+                    let mut pending_mouse_down = pending_mouse_down.borrow_mut();
+                    if pending_mouse_down.is_some() && text_hitbox.is_hovered(cx) {
+                        captured_mouse_down = pending_mouse_down.take();
+                        cx.refresh();
+                    }
+                }),
+                // Fire click handlers during the bubble phase.
+                DispatchPhase::Bubble => editor.update(cx, |editor, cx| {
+                    if let Some(mouse_down) = captured_mouse_down.take() {
+                        let event = ClickEvent {
+                            down: mouse_down,
+                            up: event.clone(),
+                        };
+                        Self::click(editor, &event, &position_map, &text_hitbox, cx);
+                    }
+                }),
+            }
+        });
+
         cx.on_mouse_event({
             let position_map = layout.position_map.clone();
             let editor = self.editor.clone();

crates/gpui/src/interactive.rs

@@ -147,6 +147,20 @@ pub struct ClickEvent {
     pub up: MouseUpEvent,
 }
 
+impl ClickEvent {
+    /// Returns the modifiers that were held down during both the
+    /// mouse down and mouse up events
+    pub fn modifiers(&self) -> Modifiers {
+        Modifiers {
+            control: self.up.modifiers.control && self.down.modifiers.control,
+            alt: self.up.modifiers.alt && self.down.modifiers.alt,
+            shift: self.up.modifiers.shift && self.down.modifiers.shift,
+            platform: self.up.modifiers.platform && self.down.modifiers.platform,
+            function: self.up.modifiers.function && self.down.modifiers.function,
+        }
+    }
+}
+
 /// An enum representing the mouse button that was pressed.
 #[derive(Hash, PartialEq, Eq, Copy, Clone, Debug)]
 pub enum MouseButton {

crates/picker/src/picker.rs

@@ -519,7 +519,7 @@ impl<D: PickerDelegate> Picker<D> {
             .id(("item", ix))
             .cursor_pointer()
             .on_click(cx.listener(move |this, event: &ClickEvent, cx| {
-                this.handle_click(ix, event.down.modifiers.secondary(), cx)
+                this.handle_click(ix, event.modifiers().secondary(), cx)
             }))
             // As of this writing, GPUI intercepts `ctrl-[mouse-event]`s on macOS
             // and produces right mouse button events. This matches platforms norms

crates/project_panel/src/project_panel.rs

@@ -3390,7 +3390,7 @@ impl ProjectPanel {
                 }
                 cx.stop_propagation();
 
-                if let Some(selection) = this.selection.filter(|_| event.down.modifiers.shift) {
+                if let Some(selection) = this.selection.filter(|_| event.modifiers().shift) {
                     let current_selection = this.index_for_selection(selection);
                     let clicked_entry = SelectedEntry {
                         entry_id,
@@ -3423,7 +3423,7 @@ impl ProjectPanel {
                         this.selection = Some(clicked_entry);
                         this.marked_entries.insert(clicked_entry);
                     }
-                } else if event.down.modifiers.secondary() {
+                } else if event.modifiers().secondary() {
                     if event.down.click_count > 1 {
                         this.split_entry(entry_id, cx);
                     } else {