From 7321c814ced25ad70eceb1f345575ea71f9b487b Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Mon, 3 Mar 2025 17:32:08 -0500 Subject: [PATCH] gpui: Add `restrict_scroll_to_axis` to match web scrolling behavior (#25963) This PR adds a new `restrict_scroll_to_axis` style to allow consumers to opt-in to the scrolling behavior found on the web. When this is enabled the behavior will be such that: - Scrolling using the mouse wheel will only scroll the Y axis - Scrolling using the mouse wheel with Shift held will only scroll the X axis This behavior is useful in scenarios where you have some vertically-scrollable content that is interspersed with horizontally-scrollable elements, as otherwise the scroll will be constantly hijacked by the horizontally-scrollable elements while trying to scroll up and down in the vertically-scrollable container. I think that this behavior should be the default, but it's a bit of a sweeping change to make all at once, so for now it remains opt-in. Release Notes: - N/A --- crates/gpui/src/elements/div.rs | 5 +++-- crates/gpui/src/style.rs | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/crates/gpui/src/elements/div.rs b/crates/gpui/src/elements/div.rs index 26c43df702..ab0b278182 100644 --- a/crates/gpui/src/elements/div.rs +++ b/crates/gpui/src/elements/div.rs @@ -2127,6 +2127,7 @@ impl Interactivity { if let Some(scroll_offset) = self.scroll_offset.clone() { let overflow = style.overflow; let allow_concurrent_scroll = style.allow_concurrent_scroll; + let restrict_scroll_to_axis = style.restrict_scroll_to_axis; let line_height = window.line_height(); let hitbox = hitbox.clone(); window.on_mouse_event(move |event: &ScrollWheelEvent, phase, window, cx| { @@ -2139,7 +2140,7 @@ impl Interactivity { if overflow.x == Overflow::Scroll { if !delta.x.is_zero() { delta_x = delta.x; - } else if overflow.y != Overflow::Scroll { + } else if !restrict_scroll_to_axis && overflow.y != Overflow::Scroll { delta_x = delta.y; } } @@ -2147,7 +2148,7 @@ impl Interactivity { if overflow.y == Overflow::Scroll { if !delta.y.is_zero() { delta_y = delta.y; - } else if overflow.x != Overflow::Scroll { + } else if !restrict_scroll_to_axis && overflow.x != Overflow::Scroll { delta_y = delta.x; } } diff --git a/crates/gpui/src/style.rs b/crates/gpui/src/style.rs index 8c1c20c2ac..cee1793e70 100644 --- a/crates/gpui/src/style.rs +++ b/crates/gpui/src/style.rs @@ -159,6 +159,28 @@ pub struct Style { pub scrollbar_width: f32, /// 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. + /// + /// 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. + /// + /// ## Motivation + /// + /// On the web when scrolling with the mouse wheel, scrolling up and down will always scroll the Y axis, even when + /// the mouse is over a horizontally-scrollable element. + /// + /// The only way to scroll horizontally is to hold down `Shift` while scrolling, which then changes the scroll axis + /// to the X axis. + /// + /// Currently, GPUI operates differently from the web in that it will scroll an element in either the X or Y axis + /// when scrolling with just the mouse wheel. This causes problems when scrolling in a vertical list that contains + /// horizontally-scrollable elements, as when you get to the horizontally-scrollable elements the scroll will be + /// hijacked. + /// + /// Ideally we would match the web's behavior and not have a need for this, but right now we're adding this opt-in + /// style property to limit the potential blast radius. + pub restrict_scroll_to_axis: bool, // Position properties /// What should the `position` value of this struct use as a base offset? @@ -702,6 +724,7 @@ impl Default for Style { y: Overflow::Visible, }, allow_concurrent_scroll: false, + restrict_scroll_to_axis: false, scrollbar_width: 0.0, position: Position::Relative, inset: Edges::auto(),