From d73ad3de5507c75ce82cb1cffc9bfbded28bee85 Mon Sep 17 00:00:00 2001 From: Finn Eitreim <48069764+feitreim@users.noreply.github.com> Date: Thu, 30 Jul 2026 12:30:13 -0400 Subject: [PATCH] gpui: Give divs (the option for) a better scrolling experience (#61797) # Objective Closes #56127 Right now there is an issue where in certain contexts, specifically GPUI div elements, horizontal scrolling w/ a trackpad works very poorly, even when trying to scroll sideways, vertical scrolling will happen. This happens with both a trackpad and scrollwheel. The showcase video also shows the broken behavior. ## Solution The way that scrolling works in the editor is really nice, scrolls are locked to the axis they started on but with enough force can change axis. so by taking the ongoing scroll handler from the editor and moving it up into GPUI, we can get that same functionality for GPUI. I somewhat "take over" the functionality of the existing `restrict_scroll_to_axis` property, because it seems like its purpose was basically trying to fix this problem already, just unsuccessfully. The alternative, and would make things overall nicer to look at, would be to tie this to `allow_concurrent_scroll`, but the effect radius tied to that option is way bigger, so im holding off on that atleast right now. ## Testing Theres a good number of scroll related tests generally, so I don't think i've perturbed any common existing behaviors. attach the functionality to `restrict_scroll_to_axis` really limits the surface. There are also some new tests for the ongoing_scroll behaviors. Tested on my mac w/ both trackpad and mouse. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable ## Showcase Video show casing all the behaviors: broken w/ trackpad working w/ trackpad broken w/ scroll working w/ scroll https://github.com/user-attachments/assets/1935cd2d-a255-4a77-81d5-daa9f72470a6 --- Release Notes: - markdown_preview: Fix horizontal scrolling inside certain elements --- .../src/conversation_view/thread_view.rs | 4 +- crates/editor/src/editor.rs | 4 +- crates/editor/src/editor_tests.rs | 17 +- crates/editor/src/element/mouse.rs | 26 +-- crates/editor/src/scroll.rs | 104 +--------- crates/editor/src/scroll/actions.rs | 3 - crates/gpui/src/elements/div.rs | 71 +++++-- crates/gpui/src/gestures.rs | 193 +++++++++++++++++- crates/gpui/src/style.rs | 12 +- crates/markdown/src/markdown.rs | 5 +- .../src/markdown_preview_view.rs | 1 + crates/search/src/project_search.rs | 11 +- crates/ui/src/components/data_table.rs | 12 +- 13 files changed, 291 insertions(+), 172 deletions(-) diff --git a/crates/agent_ui/src/conversation_view/thread_view.rs b/crates/agent_ui/src/conversation_view/thread_view.rs index 97d4b5021a..4e0af6adee 100644 --- a/crates/agent_ui/src/conversation_view/thread_view.rs +++ b/crates/agent_ui/src/conversation_view/thread_view.rs @@ -404,14 +404,14 @@ fn render_cat_numbered_code_block( // `restrict_scroll_to_axis` then keeps vertical wheel events flowing through // to the outer thread scroller. This mirrors the standard markdown // code-block path in `crates/markdown/src/markdown.rs`. - let mut code_scroll = div() + let code_scroll = div() .id(code_scroll_id) .flex() .flex_1() .min_w_0() .overflow_x_scroll() + .restrict_scroll_to_axis() .child(div().flex_none().child(code_text)); - code_scroll.style().restrict_scroll_to_axis = Some(true); container .child( diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 89223e7f73..d8717dadce 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -227,7 +227,7 @@ use project::{ use rand::seq::SliceRandom; use regex::Regex; use rpc::{ErrorCode, ErrorExt, proto::PeerId}; -use scroll::{Autoscroll, OngoingScroll, ScrollAnchor, ScrollManager, SharedScrollAnchor}; +use scroll::{Autoscroll, ScrollAnchor, ScrollManager, SharedScrollAnchor}; use selections_collection::{MutableSelectionsCollection, SelectionsCollection}; use serde::{Deserialize, Serialize}; use settings::{ @@ -1222,7 +1222,6 @@ pub struct EditorSnapshot { pub placeholder_display_snapshot: Option, is_focused: bool, scroll_anchor: SharedScrollAnchor, - ongoing_scroll: OngoingScroll, current_line_highlight: CurrentLineHighlight, gutter_hovered: bool, semantic_tokens_enabled: bool, @@ -3003,7 +3002,6 @@ impl Editor { .placeholder_display_map .as_ref() .map(|display_map| display_map.update(cx, |map, cx| map.snapshot(cx))), - ongoing_scroll: self.scroll_manager.ongoing_scroll(), is_focused: self.focus_handle.is_focused(window), current_line_highlight: self .current_line_highlight diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 61c3312d4c..56905d3817 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -28926,7 +28926,7 @@ async fn test_expand_first_line_diff_hunk_keeps_deleted_lines_visible( cx.set_state("ˇnew\nsecond\nthird\n"); cx.set_head_text("old\nsecond\nthird\n"); cx.update_editor(|editor, window, cx| { - editor.scroll(gpui::Point { x: 0., y: 0. }, None, window, cx); + editor.scroll(gpui::Point { x: 0., y: 0. }, window, cx); }); executor.run_until_parked(); assert_eq!(cx.update_editor(|e, _, cx| e.scroll_position(cx)).y, 0.0); @@ -37546,7 +37546,7 @@ async fn test_sticky_scroll(cx: &mut TestAppContext) { let mut sticky_headers = |offset: ScrollOffset| { cx.update_editor(|e, window, cx| { - e.scroll(gpui::Point { x: 0., y: offset }, None, window, cx); + e.scroll(gpui::Point { x: 0., y: offset }, window, cx); }); cx.run_until_parked(); cx.update_editor(|e, window, cx| { @@ -37635,7 +37635,7 @@ async fn test_sticky_scroll_with_decoration_prefix_in_item(cx: &mut TestAppConte let mut sticky_headers = |offset: ScrollOffset| { cx.update_editor(|e, window, cx| { - e.scroll(gpui::Point { x: 0., y: offset }, None, window, cx); + e.scroll(gpui::Point { x: 0., y: offset }, window, cx); }); cx.run_until_parked(); cx.update_editor(|e, window, cx| { @@ -37698,7 +37698,7 @@ async fn test_sticky_scroll_anchors_multiline_c_signature_on_name_row(cx: &mut T let mut sticky_headers = |offset: ScrollOffset| { cx.update_editor(|editor, window, cx| { - editor.scroll(gpui::Point { x: 0., y: offset }, None, window, cx); + editor.scroll(gpui::Point { x: 0., y: offset }, window, cx); }); cx.run_until_parked(); cx.update_editor(|editor, window, cx| { @@ -37781,7 +37781,7 @@ async fn test_sticky_scroll_with_expanded_deleted_diff_hunks( let mut sticky_headers = |offset: ScrollOffset| { cx.update_editor(|e, window, cx| { - e.scroll(gpui::Point { x: 0., y: offset }, None, window, cx); + e.scroll(gpui::Point { x: 0., y: offset }, window, cx); }); cx.run_until_parked(); cx.update_editor(|e, window, cx| { @@ -37837,7 +37837,7 @@ async fn test_no_duplicated_sticky_headers(cx: &mut TestAppContext) { let mut sticky_headers = |offset: ScrollOffset| { cx.update_editor(|e, window, cx| { - e.scroll(gpui::Point { x: 0., y: offset }, None, window, cx); + e.scroll(gpui::Point { x: 0., y: offset }, window, cx); }); cx.run_until_parked(); cx.update_editor(|e, window, cx| { @@ -38133,7 +38133,6 @@ async fn test_scroll_by_clicking_sticky_header(cx: &mut TestAppContext) { x: 0., y: scroll_offset, }, - None, window, cx, ); @@ -38225,7 +38224,7 @@ async fn test_scroll_by_clicking_sticky_header(cx: &mut TestAppContext) { // The text "impl Bar {" starts at column 0, so column 5 = 'B'. let click_x = text_origin_x + em_width * 5.5; cx.update_editor(|e, window, cx| { - e.scroll(gpui::Point { x: 0., y: 4.5 }, None, window, cx); + e.scroll(gpui::Point { x: 0., y: 4.5 }, window, cx); }); cx.run_until_parked(); cx.simulate_click( @@ -38299,7 +38298,7 @@ async fn test_clicking_sticky_header_sets_character_select_mode(cx: &mut TestApp editor.end_selection(window, cx); // Scroll down one row to make `fn foo() {` a sticky header - editor.scroll(gpui::Point { x: 0., y: 1. }, None, window, cx); + editor.scroll(gpui::Point { x: 0., y: 1. }, window, cx); }); cx.run_until_parked(); diff --git a/crates/editor/src/element/mouse.rs b/crates/editor/src/element/mouse.rs index 78bb6bd404..63f4dd119e 100644 --- a/crates/editor/src/element/mouse.rs +++ b/crates/editor/src/element/mouse.rs @@ -538,19 +538,18 @@ impl EditorElement { editor.update(cx, |editor, cx| { let line_height = position_map.line_height; let glyph_width = position_map.em_layout_width; - let (delta, axis) = match delta { + let delta = match delta { gpui::ScrollDelta::Pixels(mut pixels) => { //Trackpad - let axis = - position_map.snapshot.ongoing_scroll.filter(&mut pixels); - (pixels, axis) + editor + .scroll_manager + .filter_scroll_delta(&mut pixels, event.touch_phase); + pixels } gpui::ScrollDelta::Lines(lines) => { //Not trackpad - let pixels = - point(lines.x * glyph_width, lines.y * line_height); - (pixels, None) + point(lines.x * glyph_width, lines.y * line_height) } }; @@ -572,17 +571,8 @@ impl EditorElement { } if scroll_position != current_scroll_position { - editor.scroll(scroll_position, axis, window, cx); + editor.scroll(scroll_position, window, cx); cx.stop_propagation(); - } else if y < 0. && !forbid_vertical_scroll { - // Due to clamping, we may fail to detect cases of overscroll to the top; - // We want the scroll manager to get an update in such cases and detect the change of direction - // on the next frame. - if editor.scroll_manager.should_notify_top_overscroll(axis) { - cx.notify(); - } - } else { - editor.scroll_manager.reset_top_overscroll_notification(); } }); } @@ -1244,7 +1234,7 @@ mod tests { }); cx.update_editor(|editor, window, cx| { - editor.scroll(gpui::Point { x: 0., y: 5.5 }, None, window, cx); + editor.scroll(gpui::Point { x: 0., y: 5.5 }, window, cx); }); cx.run_until_parked(); diff --git a/crates/editor/src/scroll.rs b/crates/editor/src/scroll.rs index dcd96c675c..829d3d05f8 100644 --- a/crates/editor/src/scroll.rs +++ b/crates/editor/src/scroll.rs @@ -13,21 +13,18 @@ use crate::{ pub use autoscroll::{Autoscroll, AutoscrollStrategy}; use core::fmt::Debug; use gpui::{ - Along, App, AppContext as _, Axis, Context, Entity, EntityId, Pixels, Task, Window, point, px, + Along, App, AppContext as _, Axis, Context, Entity, EntityId, OngoingScroll, Pixels, Task, + TouchPhase, Window, point, }; use language::language_settings::{AllLanguageSettings, SoftWrap}; use language::{Bias, Point}; pub use scroll_amount::ScrollAmount; use settings::Settings; -use std::{ - cmp::Ordering, - time::{Duration, Instant}, -}; +use std::{cmp::Ordering, time::Duration}; use ui::scrollbars::ScrollbarAutoHide; use util::ResultExt; use workspace::{ItemId, WorkspaceId}; -pub const SCROLL_EVENT_SEPARATION: Duration = Duration::from_millis(28); const SCROLLBAR_SHOW_INTERVAL: Duration = Duration::from_secs(1); pub struct WasScrolled(pub(crate) bool); @@ -64,12 +61,6 @@ impl ScrollAnchor { } } -#[derive(Clone, Copy, Debug)] -pub struct OngoingScroll { - last_event: Instant, - axis: Option, -} - /// In the split diff view, the two sides share a ScrollAnchor using this struct. /// Either side can set a ScrollAnchor that points to its own multibuffer, and we store the ID of the display map /// that the last-written anchor came from so that we know how to resolve it to a DisplayPoint. @@ -121,62 +112,6 @@ impl SharedScrollAnchor { } } -impl OngoingScroll { - fn new() -> Self { - Self { - last_event: Instant::now() - SCROLL_EVENT_SEPARATION, - axis: None, - } - } - - pub fn filter(&self, delta: &mut gpui::Point) -> Option { - const UNLOCK_PERCENT: f32 = 1.9; - const UNLOCK_LOWER_BOUND: Pixels = px(6.); - let mut axis = self.axis; - - let x = delta.x.abs(); - let y = delta.y.abs(); - let duration = Instant::now().duration_since(self.last_event); - if duration > SCROLL_EVENT_SEPARATION { - //New ongoing scroll will start, determine axis - axis = if x <= y { - Some(Axis::Vertical) - } else { - Some(Axis::Horizontal) - }; - } else if x.max(y) >= UNLOCK_LOWER_BOUND { - //Check if the current ongoing will need to unlock - match axis { - Some(Axis::Vertical) => { - if x > y && x >= y * UNLOCK_PERCENT { - axis = None; - } - } - - Some(Axis::Horizontal) => { - if y > x && y >= x * UNLOCK_PERCENT { - axis = None; - } - } - - None => {} - } - } - - match axis { - Some(Axis::Vertical) => { - *delta = point(px(0.), delta.y); - } - Some(Axis::Horizontal) => { - *delta = point(delta.x, px(0.)); - } - None => {} - } - - axis - } -} - #[derive(Copy, Clone, Default, PartialEq, Eq)] pub enum ScrollbarThumbState { #[default] @@ -227,7 +162,6 @@ pub struct ScrollManager { visible_line_count: Option, visible_column_count: Option, forbid_vertical_scroll: bool, - notified_top_overscroll: bool, minimap_thumb_state: Option, _save_scroll_position_task: Task<()>, } @@ -242,7 +176,7 @@ impl ScrollManager { vertical_scroll_margin: EditorSettings::get_global(cx).vertical_scroll_margin, anchor, scroll_max_x: None, - ongoing: OngoingScroll::new(), + ongoing: OngoingScroll::default(), autoscroll_request: None, show_scrollbars: true, hide_scrollbar_task: None, @@ -251,7 +185,6 @@ impl ScrollManager { visible_line_count: None, visible_column_count: None, forbid_vertical_scroll: false, - notified_top_overscroll: false, minimap_thumb_state: None, _save_scroll_position_task: Task::ready(()), } @@ -361,28 +294,12 @@ impl ScrollManager { }); } - pub fn ongoing_scroll(&self) -> OngoingScroll { - self.ongoing - } - - pub fn update_ongoing_scroll(&mut self, axis: Option) { - self.ongoing.last_event = Instant::now(); - self.ongoing.axis = axis; - } - - pub fn should_notify_top_overscroll(&mut self, axis: Option) -> bool { - let now = Instant::now(); - let new_scroll = now.duration_since(self.ongoing.last_event) > SCROLL_EVENT_SEPARATION; - let axis_changed = self.ongoing.axis != axis; - let should_notify = !self.notified_top_overscroll || new_scroll || axis_changed; - self.ongoing.last_event = now; - self.ongoing.axis = axis; - self.notified_top_overscroll = true; - should_notify - } - - pub fn reset_top_overscroll_notification(&mut self) { - self.notified_top_overscroll = false; + pub fn filter_scroll_delta( + &mut self, + delta: &mut gpui::Point, + touch_phase: TouchPhase, + ) { + self.ongoing.filter(delta, touch_phase); } pub fn scroll_position( @@ -486,7 +403,6 @@ impl ScrollManager { return WasScrolled(false); } - self.notified_top_overscroll = false; self.anchor.update(cx, |shared, _| { shared.scroll_anchor = adjusted_anchor; shared.display_map_id = Some(display_map.display_map_id); diff --git a/crates/editor/src/scroll/actions.rs b/crates/editor/src/scroll/actions.rs index 4685b1003e..53c3900a39 100644 --- a/crates/editor/src/scroll/actions.rs +++ b/crates/editor/src/scroll/actions.rs @@ -1,4 +1,3 @@ -use super::Axis; use crate::{ Autoscroll, Editor, EditorMode, NextScreen, NextScrollCursorCenterTopBottom, SCROLL_CENTER_TOP_BOTTOM_DEBOUNCE_TIMEOUT, ScrollCursorBottom, ScrollCursorCenter, @@ -26,11 +25,9 @@ impl Editor { pub fn scroll( &mut self, scroll_position: Point, - axis: Option, window: &mut Window, cx: &mut Context, ) { - self.scroll_manager.update_ongoing_scroll(axis); self.set_scroll_position(scroll_position, window, cx); } diff --git a/crates/gpui/src/elements/div.rs b/crates/gpui/src/elements/div.rs index 0c435543f5..811322546f 100644 --- a/crates/gpui/src/elements/div.rs +++ b/crates/gpui/src/elements/div.rs @@ -15,16 +15,16 @@ //! and Tailwind-like styling that you can use to build your own custom elements. Div is //! constructed by combining these two systems into an all-in-one element. -use crate::PinchEvent; use crate::{ Action, AnyDrag, AnyElement, AnyTooltip, AnyView, App, Bounds, ClickEvent, DispatchPhase, Display, Element, ElementId, Entity, EntityId, ExternalDragPayload, ExternalDragPayloadSource, FocusHandle, Global, GlobalElementId, Hitbox, HitboxBehavior, HitboxId, InspectorElementId, IntoElement, IsZero, KeyContext, KeyDownEvent, KeyUpEvent, KeyboardButton, KeyboardClickEvent, LayoutId, ModifiersChangedEvent, MouseButton, MouseClickEvent, MouseDownEvent, MouseExitEvent, - MouseMoveEvent, MousePressureEvent, MouseUpEvent, Overflow, ParentElement, Pixels, Point, - Render, ScrollWheelEvent, SharedString, Size, Style, StyleRefinement, Styled, Task, TooltipId, - Visibility, Window, WindowControlArea, point, px, size, + MouseMoveEvent, MousePressureEvent, MouseUpEvent, OngoingScroll, Overflow, ParentElement, + PinchEvent, Pixels, Point, Render, ScrollWheelEvent, SharedString, Size, Style, + StyleRefinement, Styled, Task, TooltipId, Visibility, Window, WindowControlArea, point, px, + size, }; use collections::HashMap; use gpui_util::ResultExt; @@ -1463,6 +1463,14 @@ pub trait StatefulInteractiveElement: InteractiveElement { self } + /// Restrict scrolling of this element to the axis of the input gesture. + /// + /// See [`Style::restrict_scroll_to_axis`](crate::Style::restrict_scroll_to_axis) for details. + fn restrict_scroll_to_axis(mut self) -> Self { + self.interactivity().base_style.restrict_scroll_to_axis = Some(true); + self + } + /// Track the scroll state of this element with the given handle. fn track_scroll(mut self, scroll_handle: &ScrollHandle) -> Self { self.interactivity().tracked_scroll_handle = Some(scroll_handle.clone()); @@ -2015,6 +2023,7 @@ pub struct Interactivity { pub(crate) tracked_scroll_handle: Option, pub(crate) scroll_anchor: Option, pub(crate) scroll_offset: Option>>>, + pub(crate) ongoing_scroll: Option>>, pub(crate) group: Option, /// The base style of the element, before any modifications are applied /// by focus, active, etc. @@ -2136,7 +2145,9 @@ impl Interactivity { } if let Some(scroll_handle) = self.tracked_scroll_handle.as_ref() { - self.scroll_offset = Some(scroll_handle.0.borrow().offset.clone()); + let scroll_handle_state = scroll_handle.0.borrow(); + self.scroll_offset = Some(scroll_handle_state.offset.clone()); + self.ongoing_scroll = Some(scroll_handle_state.ongoing_scroll.clone()); } else if (self.base_style.overflow.x == Some(Overflow::Scroll) || self.base_style.overflow.y == Some(Overflow::Scroll)) && let Some(element_state) = element_state.as_mut() @@ -2147,6 +2158,12 @@ impl Interactivity { .get_or_insert_with(Rc::default) .clone(), ); + self.ongoing_scroll = Some( + element_state + .ongoing_scroll + .get_or_insert_with(|| Rc::new(RefCell::new(OngoingScroll::default()))) + .clone(), + ); } let style = self.compute_style_internal(None, element_state.as_mut(), window, cx); @@ -3150,6 +3167,7 @@ impl Interactivity { _cx: &mut App, ) { if let Some(scroll_offset) = self.scroll_offset.clone() { + let ongoing_scroll = self.ongoing_scroll.clone(); let overflow = style.overflow; let allow_concurrent_scroll = style.allow_concurrent_scroll; let restrict_scroll_to_axis = style.restrict_scroll_to_axis; @@ -3160,24 +3178,35 @@ impl Interactivity { if phase == DispatchPhase::Bubble && hitbox.should_handle_scroll(window) { let mut scroll_offset = scroll_offset.borrow_mut(); let old_scroll_offset = *scroll_offset; - let delta = event.delta.pixel_delta(line_height); + let mut delta = event.delta.pixel_delta(line_height); - let mut delta_x = Pixels::ZERO; - if overflow.x == Overflow::Scroll { - if !delta.x.is_zero() { - delta_x = delta.x; - } else if !restrict_scroll_to_axis && overflow.y != Overflow::Scroll { - delta_x = delta.y; - } + if restrict_scroll_to_axis + && event.delta.precise() + && let Some(ongoing_scroll) = &ongoing_scroll + { + ongoing_scroll + .borrow_mut() + .filter(&mut delta, event.touch_phase); } - let mut delta_y = Pixels::ZERO; - if overflow.y == Overflow::Scroll { - if !delta.y.is_zero() { - delta_y = delta.y; - } else if !restrict_scroll_to_axis && overflow.x != Overflow::Scroll { - delta_y = delta.x; + + let mut delta_x = match overflow.x { + Overflow::Scroll if !delta.x.is_zero() => delta.x, + Overflow::Scroll + if !restrict_scroll_to_axis && overflow.y != Overflow::Scroll => + { + delta.y } - } + _ => Pixels::ZERO, + }; + let mut delta_y = match overflow.y { + Overflow::Scroll if !delta.y.is_zero() => delta.y, + Overflow::Scroll + if !restrict_scroll_to_axis && overflow.x != Overflow::Scroll => + { + delta.x + } + _ => Pixels::ZERO, + }; if !allow_concurrent_scroll && !delta_x.is_zero() && !delta_y.is_zero() { if delta_x.abs() > delta_y.abs() { delta_y = Pixels::ZERO; @@ -3425,6 +3454,7 @@ pub struct InteractiveElementState { /// blur). `None` means no activation key is pending. pub(crate) pending_keyboard_down: Option>>>, pub(crate) scroll_offset: Option>>>, + ongoing_scroll: Option>>, pub(crate) active_tooltip: Option>>>, } @@ -3960,6 +3990,7 @@ impl ScrollAnchor { #[derive(Default, Debug)] struct ScrollHandleState { offset: Rc>>, + ongoing_scroll: Rc>, bounds: Bounds, max_offset: Point, child_bounds: Vec>, diff --git a/crates/gpui/src/gestures.rs b/crates/gpui/src/gestures.rs index 5e14d4249b..99f0efc4a2 100644 --- a/crates/gpui/src/gestures.rs +++ b/crates/gpui/src/gestures.rs @@ -10,9 +10,80 @@ //! [`PinchEvent`](crate::PinchEvent)s — so components written against //! `on_click` and scroll containers work untouched on mobile. -use std::time::Duration; +use std::time::{Duration, Instant}; -use crate::{Pixels, Point, px}; +use crate::{Axis, IsZero, Pixels, Point, TouchPhase, px}; + +const SCROLL_EVENT_SEPARATION: Duration = Duration::from_millis(28); + +/// Tracks the dominant axis across the events in a scroll gesture. +#[derive(Clone, Copy, Debug, Default)] +pub struct OngoingScroll { + last_event: Option, + axis: Option, +} + +impl OngoingScroll { + /// Filters the given delta to the dominant axis of the current scroll gesture. + /// + /// Gestures are delimited by their touch phase when available, with a timeout + /// fallback for platforms that only emit [`TouchPhase::Moved`]. + pub fn filter(&mut self, delta: &mut Point, touch_phase: TouchPhase) { + self.filter_at(delta, touch_phase, Instant::now()) + } + + fn filter_at(&mut self, delta: &mut Point, touch_phase: TouchPhase, now: Instant) { + const UNLOCK_PERCENT: f32 = 1.9; + const UNLOCK_LOWER_BOUND: Pixels = px(6.); + + if matches!(touch_phase, TouchPhase::Ended | TouchPhase::Cancelled) { + self.last_event = None; + self.axis = None; + return; + } + + let x = delta.x.abs(); + let y = delta.y.abs(); + if x.is_zero() && y.is_zero() { + if touch_phase == TouchPhase::Started { + self.last_event = None; + self.axis = None; + } + return; + } + + let starts_new_gesture = touch_phase == TouchPhase::Started + || self + .last_event + .is_none_or(|last_event| now.duration_since(last_event) >= SCROLL_EVENT_SEPARATION); + let mut axis = self.axis; + if starts_new_gesture { + axis = if x <= y { + Some(Axis::Vertical) + } else { + Some(Axis::Horizontal) + }; + } else if x.max(y) >= UNLOCK_LOWER_BOUND { + match axis { + Some(Axis::Vertical) if x > y && x >= y * UNLOCK_PERCENT => { + axis = None; + } + Some(Axis::Horizontal) if y > x && y >= x * UNLOCK_PERCENT => { + axis = None; + } + _ => {} + } + } + + self.last_event = Some(now); + self.axis = axis; + match axis { + Some(Axis::Vertical) => delta.x = Pixels::ZERO, + Some(Axis::Horizontal) => delta.y = Pixels::ZERO, + None => {} + } + } +} /// Feel constants consumed by gesture recognizers. Provided on a best-effort /// basis, depending on each platform's support, defaulting to GPUI's own @@ -121,3 +192,121 @@ pub trait PlatformGestures { pub struct NullPlatformGestures; impl PlatformGestures for NullPlatformGestures {} + +#[cfg(test)] +mod tests { + use super::*; + use crate::point; + + #[test] + fn ongoing_scroll_locks_to_dominant_axis() { + let now = Instant::now(); + let mut ongoing_scroll = OngoingScroll::default(); + let mut horizontal_delta = point(px(10.), px(2.)); + ongoing_scroll.filter_at(&mut horizontal_delta, TouchPhase::Started, now); + assert_eq!(ongoing_scroll.axis, Some(Axis::Horizontal)); + assert_eq!(horizontal_delta, point(px(10.), px(0.))); + + let mut continued_delta = point(px(3.), px(2.)); + ongoing_scroll.filter_at( + &mut continued_delta, + TouchPhase::Moved, + now + Duration::from_millis(1), + ); + assert_eq!(ongoing_scroll.axis, Some(Axis::Horizontal)); + assert_eq!(continued_delta, point(px(3.), px(0.))); + } + + #[test] + fn ongoing_scroll_unlocks_when_direction_changes() { + let now = Instant::now(); + let mut ongoing_scroll = OngoingScroll::default(); + let mut horizontal_delta = point(px(10.), px(2.)); + ongoing_scroll.filter_at(&mut horizontal_delta, TouchPhase::Started, now); + + let mut vertical_delta = point(px(2.), px(10.)); + ongoing_scroll.filter_at( + &mut vertical_delta, + TouchPhase::Moved, + now + Duration::from_millis(1), + ); + assert_eq!(ongoing_scroll.axis, None); + assert_eq!(vertical_delta, point(px(2.), px(10.))); + } + + #[test] + fn ongoing_scroll_starts_new_gesture_at_timeout_boundary() { + let now = Instant::now(); + let mut ongoing_scroll = OngoingScroll::default(); + let mut horizontal_delta = point(px(10.), px(2.)); + ongoing_scroll.filter_at(&mut horizontal_delta, TouchPhase::Moved, now); + + let mut vertical_delta = point(px(2.), px(10.)); + ongoing_scroll.filter_at( + &mut vertical_delta, + TouchPhase::Moved, + now + SCROLL_EVENT_SEPARATION, + ); + assert_eq!(ongoing_scroll.axis, Some(Axis::Vertical)); + assert_eq!(vertical_delta, point(px(0.), px(10.))); + } + + #[test] + fn ongoing_scroll_ignores_zero_delta_and_resets_when_ended() { + let now = Instant::now(); + let mut ongoing_scroll = OngoingScroll::default(); + let mut horizontal_delta = point(px(10.), px(2.)); + ongoing_scroll.filter_at(&mut horizontal_delta, TouchPhase::Started, now); + + let mut zero_delta = Point::default(); + ongoing_scroll.filter_at( + &mut zero_delta, + TouchPhase::Ended, + now + Duration::from_millis(1), + ); + assert_eq!(ongoing_scroll.axis, None); + + let mut vertical_delta = point(px(2.), px(3.)); + ongoing_scroll.filter_at( + &mut vertical_delta, + TouchPhase::Moved, + now + Duration::from_millis(2), + ); + assert_eq!(ongoing_scroll.axis, Some(Axis::Vertical)); + assert_eq!(vertical_delta, point(px(0.), px(3.))); + } + + #[test] + fn ongoing_scroll_ignores_zero_delta_movement() { + let now = Instant::now(); + let mut ongoing_scroll = OngoingScroll::default(); + let mut horizontal_delta = point(px(10.), px(2.)); + ongoing_scroll.filter_at(&mut horizontal_delta, TouchPhase::Started, now); + + let mut zero_delta = Point::default(); + ongoing_scroll.filter_at( + &mut zero_delta, + TouchPhase::Moved, + now + SCROLL_EVENT_SEPARATION, + ); + + let mut vertical_delta = point(px(2.), px(10.)); + ongoing_scroll.filter_at( + &mut vertical_delta, + TouchPhase::Moved, + now + SCROLL_EVENT_SEPARATION, + ); + assert_eq!(ongoing_scroll.axis, Some(Axis::Vertical)); + assert_eq!(vertical_delta, point(px(0.), px(10.))); + } + + #[test] + fn ongoing_scroll_supports_moved_only_platforms() { + let now = Instant::now(); + let mut ongoing_scroll = OngoingScroll::default(); + let mut horizontal_delta = point(px(10.), px(2.)); + ongoing_scroll.filter_at(&mut horizontal_delta, TouchPhase::Moved, now); + assert_eq!(ongoing_scroll.axis, Some(Axis::Horizontal)); + assert_eq!(horizontal_delta, point(px(10.), px(0.))); + } +} diff --git a/crates/gpui/src/style.rs b/crates/gpui/src/style.rs index 39689b7626..9150fdded9 100644 --- a/crates/gpui/src/style.rs +++ b/crates/gpui/src/style.rs @@ -192,11 +192,15 @@ pub struct Style { pub scrollbar_width: AbsoluteLength, /// Whether both x and y axis should be scrollable at the same time. pub allow_concurrent_scroll: bool, - /// Whether scrolling should be restricted to the axis indicated by the mouse wheel. + /// Whether scrolling should be restricted to the input gesture's axis. /// - /// This means that: - /// - The mouse wheel alone will only ever scroll the Y axis. - /// - Holding `Shift` and using the mouse wheel will scroll the X axis. + /// Pixel-based scroll gestures are locked to their initially dominant axis. The lock may be + /// released when the gesture changes direction strongly. Touch phases delimit gestures when + /// available, with a timeout fallback for platforms that only emit moved events. + /// + /// This also prevents input from being remapped to another axis. For example, horizontal input + /// will not scroll a container that only has vertical overflow enabled. Mouse wheel platforms + /// typically report ordinary wheel input on the Y axis and Shift-modified input on the X axis. /// /// ## Motivation /// diff --git a/crates/markdown/src/markdown.rs b/crates/markdown/src/markdown.rs index 25dcd06528..06581ca306 100644 --- a/crates/markdown/src/markdown.rs +++ b/crates/markdown/src/markdown.rs @@ -2406,13 +2406,12 @@ impl Element for MarkdownElement { let code_block = div() .id(("code-block", range.start)) .rounded_lg() - .map(|mut code_block| { + .map(|code_block| { if let Some(scroll_handle) = scroll_handle.as_ref() { - code_block.style().restrict_scroll_to_axis = - Some(true); code_block .flex() .overflow_x_scroll() + .restrict_scroll_to_axis() .track_scroll(scroll_handle) } else { code_block.w_full() diff --git a/crates/markdown_preview/src/markdown_preview_view.rs b/crates/markdown_preview/src/markdown_preview_view.rs index be3a777ac6..0a546f9cf8 100644 --- a/crates/markdown_preview/src/markdown_preview_view.rs +++ b/crates/markdown_preview/src/markdown_preview_view.rs @@ -1598,6 +1598,7 @@ impl Render for MarkdownPreviewView { .size_full() .overflow_y_scroll() .track_scroll(&self.scroll_handle) + .restrict_scroll_to_axis() .p_4() .child({ let markdown_element = diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index 9484f50287..22a56c7e38 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -22,7 +22,7 @@ use editor::{ }; use futures::{StreamExt, stream::FuturesOrdered}; use gpui::{ - Action, AnyElement, App, AsyncApp, Axis, Context, Entity, EntityId, EventEmitter, FocusHandle, + Action, AnyElement, App, AsyncApp, Context, Entity, EntityId, EventEmitter, FocusHandle, Focusable, Global, Hsla, InteractiveElement, IntoElement, KeyContext, ParentElement, Point, Render, SharedString, Styled, Subscription, Task, TaskExt, UpdateGlobal, WeakEntity, Window, actions, div, @@ -1768,7 +1768,7 @@ impl ProjectSearchView { editor.change_selections(Default::default(), window, cx, |s| { s.select_ranges(range_to_select) }); - editor.scroll(Point::default(), Some(Axis::Vertical), window, cx); + editor.scroll(Point::default(), window, cx); } }); if is_new_search && self.query_editor.focus_handle(cx).is_focused(window) { @@ -5037,12 +5037,7 @@ pub mod tests { assert_eq!(results_editor.scroll_position(cx), Point::default()); // Scroll results all the way down - results_editor.scroll( - Point::new(0., f64::MAX), - Some(Axis::Vertical), - window, - cx, - ); + results_editor.scroll(Point::new(0., f64::MAX), window, cx); }); }) .expect("unable to update search view"); diff --git a/crates/ui/src/components/data_table.rs b/crates/ui/src/components/data_table.rs index 8d785fec8a..d7cdaf2a8d 100644 --- a/crates/ui/src/components/data_table.rs +++ b/crates/ui/src/components/data_table.rs @@ -692,6 +692,7 @@ pub fn render_table_row( .id(("table-row-scrollable", row_index as u64)) .flex_grow_1() .overflow_x_scroll() + .restrict_scroll_to_axis() .flex() .child( div().flex().flex_row().children( @@ -704,7 +705,6 @@ pub fn render_table_row( if let Some(ref handle) = table_context.h_scroll_handle { scrollable_section = scrollable_section.track_scroll(handle); } - scrollable_section.style().restrict_scroll_to_axis = Some(true); row = row.child(pinned_section).child(scrollable_section); } else { @@ -817,13 +817,13 @@ pub fn render_table_header( .id("table-header-scrollable") .flex_grow_1() .overflow_x_scroll() + .restrict_scroll_to_axis() .flex() .child(inner); if let Some(ref handle) = table_context.h_scroll_handle { scrollable_section = scrollable_section.track_scroll(handle); } - scrollable_section.style().restrict_scroll_to_axis = Some(true); outer .child(pinned_section) @@ -1062,12 +1062,12 @@ fn render_resize_handles_resizable( .left(pinned_width) .right_0() .overflow_x_scroll() + .restrict_scroll_to_axis() .child(inner); if let Some(handle) = h_scroll_handle { overlay = overlay.track_scroll(handle); } - overlay.style().restrict_scroll_to_axis = Some(true); div() .id("resize-handles-wrapper") @@ -1299,14 +1299,14 @@ impl RenderOnce for Table { if let Some(state) = interaction_state.as_ref() { let content = if is_resizable && !uses_pinned_layout { - let mut h_scroll_container = div() + let h_scroll_container = div() .id("table-h-scroll") .overflow_x_scroll() + .restrict_scroll_to_axis() .flex_grow_1() .h_full() .track_scroll(&state.read(cx).horizontal_scroll_handle) .child(table); - h_scroll_container.style().restrict_scroll_to_axis = Some(true); div().size_full().child(h_scroll_container) } else { table @@ -1355,9 +1355,9 @@ impl RenderOnce for Table { ); } } - content.style().restrict_scroll_to_axis = Some(true); content + .restrict_scroll_to_axis() .track_focus(&state.read(cx).focus_handle) .id(("table", state.entity_id())) .into_any_element()