From c28a4204ee77e964f798c8186a156aab5a277de6 Mon Sep 17 00:00:00 2001 From: Shane Friedman Date: Thu, 30 Jan 2025 19:40:20 -0500 Subject: [PATCH] Use click event to determine modifier keys (#22988) 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. Closes #12752, #16074, #17892 (the latter two seem to be duplicates of the former!) Release Notes: - Fixed a bug where pressing ctrl/cmd (or other modifiers) after mouse down but before mouse up still triggered ctrl/cmd+click behavior (e.g. "go to definition") --- crates/editor/src/editor.rs | 8 ++- crates/editor/src/element.rs | 85 +++++++++++++++++++---- crates/gpui/src/interactive.rs | 14 ++++ crates/picker/src/picker.rs | 2 +- crates/project_panel/src/project_panel.rs | 6 +- 5 files changed, 96 insertions(+), 19 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index ffd390f5da1add9cfa24784a77560dbd9f146039..dff1c55f156526cdf1d54616180f9dfffedf68e0 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -81,9 +81,9 @@ use gpui::{ AsyncWindowContext, AvailableSpace, Bounds, ClipboardEntry, ClipboardItem, Context, DispatchPhase, ElementId, Entity, EntityInputHandler, EventEmitter, FocusHandle, FocusOutEvent, Focusable, FontId, FontWeight, Global, HighlightStyle, Hsla, InteractiveText, KeyContext, - MouseButton, PaintQuad, ParentElement, Pixels, Render, SharedString, Size, Styled, StyledText, - Subscription, Task, TextStyle, TextStyleRefinement, UTF16Selection, UnderlineStyle, - UniformListScrollHandle, WeakEntity, WeakFocusHandle, Window, + MouseButton, MouseDownEvent, PaintQuad, ParentElement, Pixels, Render, SharedString, Size, + Styled, StyledText, Subscription, Task, TextStyle, TextStyleRefinement, UTF16Selection, + UnderlineStyle, UniformListScrollHandle, WeakEntity, WeakFocusHandle, Window, }; use highlight_matching_bracket::refresh_matching_bracket_highlights; use hover_popover::{hide_hover, HoverState}; @@ -681,6 +681,7 @@ pub struct Editor { leader_peer_id: Option, remote_id: Option, hover_state: HoverState, + pending_mouse_down: Option>>>, gutter_hovered: bool, hovered_link_state: Option, inline_completion_provider: Option, @@ -1375,6 +1376,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, diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 01c87e31c406c6c4384dea79dba5d06d0489196a..2a98ea4d6b929c00e0f8bb8f180c8cc1fd8b1699 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -736,6 +736,10 @@ impl EditorElement { fn mouse_up( editor: &mut Editor, event: &MouseUpEvent, + #[cfg_attr( + not(any(target_os = "linux", target_os = "freebsd")), + allow(unused_variables) + )] position_map: &PositionMap, text_hitbox: &Hitbox, window: &mut Window, @@ -748,18 +752,7 @@ impl EditorElement { editor.select(SelectPhase::End, window, 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(window) { - let point = position_map.point_for_position(text_hitbox.bounds, event.position); - editor.handle_click_hovered_link(point, event.modifiers, window, 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 @@ -791,6 +784,30 @@ impl EditorElement { } } + fn click( + editor: &mut Editor, + event: &ClickEvent, + position_map: &PositionMap, + text_hitbox: &Hitbox, + window: &mut Window, + cx: &mut Context, + ) { + 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(window) { + let point = position_map.point_for_position(text_hitbox.bounds, event.up.position); + editor.handle_click_hovered_link(point, event.modifiers(), window, cx); + + cx.stop_propagation(); + } + } + fn mouse_dragged( editor: &mut Editor, event: &MouseMoveEvent, @@ -5305,6 +5322,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, @@ -5356,6 +5380,43 @@ impl EditorElement { } } }); + + window.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, window, 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(window) { + captured_mouse_down = pending_mouse_down.take(); + window.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, window, cx); + } + }), + } + }); + window.on_mouse_event({ let position_map = layout.position_map.clone(); let editor = self.editor.clone(); diff --git a/crates/gpui/src/interactive.rs b/crates/gpui/src/interactive.rs index 2bffa81b9f4ed0d228e54d62640c0a731ed948d0..f1f6e60e7f4c834a5ea3c596c74f66e3a497fc58 100644 --- a/crates/gpui/src/interactive.rs +++ b/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 { diff --git a/crates/picker/src/picker.rs b/crates/picker/src/picker.rs index f26bda81ec95d814f7bab2f9049c5beb270ada86..c3eb25558116b9eb5fb544130ba51fc7c4abc834 100644 --- a/crates/picker/src/picker.rs +++ b/crates/picker/src/picker.rs @@ -604,7 +604,7 @@ impl Picker { .id(("item", ix)) .cursor_pointer() .on_click(cx.listener(move |this, event: &ClickEvent, window, cx| { - this.handle_click(ix, event.down.modifiers.secondary(), window, cx) + this.handle_click(ix, event.modifiers().secondary(), window, cx) })) // As of this writing, GPUI intercepts `ctrl-[mouse-event]`s on macOS // and produces right mouse button events. This matches platforms norms diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index a5fbfed689ff5190ca72e86642cc36034e33671f..f1cc83f698baea3b8e1d8b41a4c1c45c97f48f5f 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -3707,7 +3707,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, @@ -3741,7 +3741,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 { @@ -3752,7 +3752,7 @@ impl ProjectPanel { } } else if kind.is_dir() { this.marked_entries.clear(); - if event.down.modifiers.alt { + if event.modifiers().alt { this.toggle_expand_all(entry_id, window, cx); } else { this.toggle_expanded(entry_id, window, cx);