Merge pull request #1644 from zed-industries/scroll-in-overlays

Mikayla Maki created

Fix scroll problems

Change summary

crates/gpui/src/elements/flex.rs                | 122 +++++++++---------
crates/gpui/src/elements/list.rs                |  39 ++---
crates/gpui/src/elements/mouse_event_handler.rs |  11 +
crates/gpui/src/elements/uniform_list.rs        |  47 ++++--
4 files changed, 118 insertions(+), 101 deletions(-)

Detailed changes

crates/gpui/src/elements/flex.rs 🔗

@@ -1,11 +1,10 @@
-use std::{any::Any, f32::INFINITY, ops::Range};
+use std::{any::Any, cell::Cell, f32::INFINITY, ops::Range, rc::Rc};
 
 use crate::{
     json::{self, ToJson, Value},
     presenter::MeasurementContext,
     Axis, DebugContext, Element, ElementBox, ElementStateHandle, Event, EventContext,
-    LayoutContext, MouseMovedEvent, PaintContext, RenderContext, ScrollWheelEvent, SizeConstraint,
-    Vector2FExt, View,
+    LayoutContext, PaintContext, RenderContext, SizeConstraint, Vector2FExt, View,
 };
 use pathfinder_geometry::{
     rect::RectF,
@@ -15,14 +14,14 @@ use serde_json::json;
 
 #[derive(Default)]
 struct ScrollState {
-    scroll_to: Option<usize>,
-    scroll_position: f32,
+    scroll_to: Cell<Option<usize>>,
+    scroll_position: Cell<f32>,
 }
 
 pub struct Flex {
     axis: Axis,
     children: Vec<ElementBox>,
-    scroll_state: Option<ElementStateHandle<ScrollState>>,
+    scroll_state: Option<(ElementStateHandle<Rc<ScrollState>>, usize)>,
 }
 
 impl Flex {
@@ -52,9 +51,9 @@ impl Flex {
         Tag: 'static,
         V: View,
     {
-        let scroll_state = cx.default_element_state::<Tag, ScrollState>(element_id);
-        scroll_state.update(cx, |scroll_state, _| scroll_state.scroll_to = scroll_to);
-        self.scroll_state = Some(scroll_state);
+        let scroll_state = cx.default_element_state::<Tag, Rc<ScrollState>>(element_id);
+        scroll_state.read(cx).scroll_to.set(scroll_to);
+        self.scroll_state = Some((scroll_state, cx.handle().id()));
         self
     }
 
@@ -202,9 +201,9 @@ impl Element for Flex {
         }
 
         if let Some(scroll_state) = self.scroll_state.as_ref() {
-            scroll_state.update(cx, |scroll_state, _| {
+            scroll_state.0.update(cx, |scroll_state, _| {
                 if let Some(scroll_to) = scroll_state.scroll_to.take() {
-                    let visible_start = scroll_state.scroll_position;
+                    let visible_start = scroll_state.scroll_position.get();
                     let visible_end = visible_start + size.along(self.axis);
                     if let Some(child) = self.children.get(scroll_to) {
                         let child_start: f32 = self.children[..scroll_to]
@@ -213,15 +212,22 @@ impl Element for Flex {
                             .sum();
                         let child_end = child_start + child.size().along(self.axis);
                         if child_start < visible_start {
-                            scroll_state.scroll_position = child_start;
+                            scroll_state.scroll_position.set(child_start);
                         } else if child_end > visible_end {
-                            scroll_state.scroll_position = child_end - size.along(self.axis);
+                            scroll_state
+                                .scroll_position
+                                .set(child_end - size.along(self.axis));
                         }
                     }
                 }
 
-                scroll_state.scroll_position =
-                    scroll_state.scroll_position.min(-remaining_space).max(0.);
+                scroll_state.scroll_position.set(
+                    scroll_state
+                        .scroll_position
+                        .get()
+                        .min(-remaining_space)
+                        .max(0.),
+                );
             });
         }
 
@@ -242,9 +248,45 @@ impl Element for Flex {
             cx.scene.push_layer(Some(bounds));
         }
 
+        if let Some(scroll_state) = &self.scroll_state {
+            cx.scene.push_mouse_region(
+                crate::MouseRegion::new::<Self>(scroll_state.1, 0, bounds)
+                    .on_scroll({
+                        let scroll_state = scroll_state.0.read(cx).clone();
+                        let axis = self.axis;
+                        move |e, cx| {
+                            if remaining_space < 0. {
+                                let mut delta = match axis {
+                                    Axis::Horizontal => {
+                                        if e.delta.x() != 0. {
+                                            e.delta.x()
+                                        } else {
+                                            e.delta.y()
+                                        }
+                                    }
+                                    Axis::Vertical => e.delta.y(),
+                                };
+                                if !e.precise {
+                                    delta *= 20.;
+                                }
+
+                                scroll_state
+                                    .scroll_position
+                                    .set(scroll_state.scroll_position.get() - delta);
+
+                                cx.notify();
+                            } else {
+                                cx.propogate_event();
+                            }
+                        }
+                    })
+                    .on_move(|_, _| { /* Capture move events */ }),
+            )
+        }
+
         let mut child_origin = bounds.origin();
         if let Some(scroll_state) = self.scroll_state.as_ref() {
-            let scroll_position = scroll_state.read(cx).scroll_position;
+            let scroll_position = scroll_state.0.read(cx).scroll_position.get();
             match self.axis {
                 Axis::Horizontal => child_origin.set_x(child_origin.x() - scroll_position),
                 Axis::Vertical => child_origin.set_y(child_origin.y() - scroll_position),
@@ -278,9 +320,9 @@ impl Element for Flex {
     fn dispatch_event(
         &mut self,
         event: &Event,
-        bounds: RectF,
         _: RectF,
-        remaining_space: &mut Self::LayoutState,
+        _: RectF,
+        _: &mut Self::LayoutState,
         _: &mut Self::PaintState,
         cx: &mut EventContext,
     ) -> bool {
@@ -288,50 +330,6 @@ impl Element for Flex {
         for child in &mut self.children {
             handled = child.dispatch_event(event, cx) || handled;
         }
-        if !handled {
-            if let &Event::ScrollWheel(ScrollWheelEvent {
-                position,
-                delta,
-                precise,
-                ..
-            }) = event
-            {
-                if *remaining_space < 0. && bounds.contains_point(position) {
-                    if let Some(scroll_state) = self.scroll_state.as_ref() {
-                        scroll_state.update(cx, |scroll_state, cx| {
-                            let mut delta = match self.axis {
-                                Axis::Horizontal => {
-                                    if delta.x() != 0. {
-                                        delta.x()
-                                    } else {
-                                        delta.y()
-                                    }
-                                }
-                                Axis::Vertical => delta.y(),
-                            };
-                            if !precise {
-                                delta *= 20.;
-                            }
-
-                            scroll_state.scroll_position -= delta;
-
-                            handled = true;
-                            cx.notify();
-                        });
-                    }
-                }
-            }
-        }
-
-        if !handled {
-            if let &Event::MouseMoved(MouseMovedEvent { position, .. }) = event {
-                // If this is a scrollable flex, and the mouse is over it, eat the scroll event to prevent
-                // propogating it to the element below.
-                if self.scroll_state.is_some() && bounds.contains_point(position) {
-                    handled = true;
-                }
-            }
-        }
 
         handled
     }

crates/gpui/src/elements/list.rs 🔗

@@ -5,8 +5,8 @@ use crate::{
     },
     json::json,
     presenter::MeasurementContext,
-    DebugContext, Element, ElementBox, ElementRc, Event, EventContext, LayoutContext, PaintContext,
-    RenderContext, ScrollWheelEvent, SizeConstraint, View, ViewContext,
+    DebugContext, Element, ElementBox, ElementRc, Event, EventContext, LayoutContext, MouseRegion,
+    PaintContext, RenderContext, SizeConstraint, View, ViewContext,
 };
 use std::{cell::RefCell, collections::VecDeque, ops::Range, rc::Rc};
 use sum_tree::{Bias, SumTree};
@@ -263,6 +263,22 @@ impl Element for List {
     ) {
         cx.scene.push_layer(Some(bounds));
 
+        cx.scene
+            .push_mouse_region(MouseRegion::new::<Self>(10, 0, bounds).on_scroll({
+                let state = self.state.clone();
+                let height = bounds.height();
+                let scroll_top = scroll_top.clone();
+                move |e, cx| {
+                    state.0.borrow_mut().scroll(
+                        &scroll_top,
+                        height,
+                        e.platform_event.delta,
+                        e.platform_event.precise,
+                        cx,
+                    )
+                }
+            }));
+
         let state = &mut *self.state.0.borrow_mut();
         for (mut element, origin) in state.visible_elements(bounds, scroll_top) {
             element.paint(origin, visible_bounds, cx);
@@ -312,20 +328,6 @@ impl Element for List {
         drop(cursor);
         state.items = new_items;
 
-        if let Event::ScrollWheel(ScrollWheelEvent {
-            position,
-            delta,
-            precise,
-            ..
-        }) = event
-        {
-            if bounds.contains_point(*position)
-                && state.scroll(scroll_top, bounds.height(), *delta, *precise, cx)
-            {
-                handled = true;
-            }
-        }
-
         handled
     }
 
@@ -527,7 +529,7 @@ impl StateInner {
         mut delta: Vector2F,
         precise: bool,
         cx: &mut EventContext,
-    ) -> bool {
+    ) {
         if !precise {
             delta *= 20.;
         }
@@ -554,9 +556,6 @@ impl StateInner {
             let visible_range = self.visible_range(height, scroll_top);
             self.scroll_handler.as_mut().unwrap()(visible_range, cx);
         }
-        cx.notify();
-
-        true
     }
 
     fn scroll_top(&self, logical_scroll_top: &ListOffset) -> f32 {

crates/gpui/src/elements/mouse_event_handler.rs 🔗

@@ -7,7 +7,8 @@ use crate::{
     platform::CursorStyle,
     scene::{
         ClickRegionEvent, CursorRegion, DownOutRegionEvent, DownRegionEvent, DragRegionEvent,
-        HandlerSet, HoverRegionEvent, MoveRegionEvent, UpOutRegionEvent, UpRegionEvent,
+        HandlerSet, HoverRegionEvent, MoveRegionEvent, ScrollWheelRegionEvent, UpOutRegionEvent,
+        UpRegionEvent,
     },
     DebugContext, Element, ElementBox, Event, EventContext, LayoutContext, MeasurementContext,
     MouseButton, MouseRegion, MouseState, PaintContext, RenderContext, SizeConstraint, View,
@@ -122,6 +123,14 @@ impl<Tag> MouseEventHandler<Tag> {
         self
     }
 
+    pub fn on_scroll(
+        mut self,
+        handler: impl Fn(ScrollWheelRegionEvent, &mut EventContext) + 'static,
+    ) -> Self {
+        self.handlers = self.handlers.on_scroll(handler);
+        self
+    }
+
     pub fn with_hoverable(mut self, is_hoverable: bool) -> Self {
         self.hoverable = is_hoverable;
         self

crates/gpui/src/elements/uniform_list.rs 🔗

@@ -6,7 +6,8 @@ use crate::{
     },
     json::{self, json},
     presenter::MeasurementContext,
-    ElementBox, RenderContext, ScrollWheelEvent, View,
+    scene::ScrollWheelRegionEvent,
+    ElementBox, MouseRegion, RenderContext, ScrollWheelEvent, View,
 };
 use json::ToJson;
 use std::{cell::RefCell, cmp, ops::Range, rc::Rc};
@@ -50,6 +51,7 @@ pub struct UniformList {
     padding_top: f32,
     padding_bottom: f32,
     get_width_from_item: Option<usize>,
+    view_id: usize,
 }
 
 impl UniformList {
@@ -77,6 +79,7 @@ impl UniformList {
             padding_top: 0.,
             padding_bottom: 0.,
             get_width_from_item: None,
+            view_id: cx.handle().id(),
         }
     }
 
@@ -96,7 +99,7 @@ impl UniformList {
     }
 
     fn scroll(
-        &self,
+        state: UniformListState,
         _: Vector2F,
         mut delta: Vector2F,
         precise: bool,
@@ -107,7 +110,7 @@ impl UniformList {
             delta *= 20.;
         }
 
-        let mut state = self.state.0.borrow_mut();
+        let mut state = state.0.borrow_mut();
         state.scroll_top = (state.scroll_top - delta.y()).max(0.0).min(scroll_max);
         cx.notify();
 
@@ -283,6 +286,28 @@ impl Element for UniformList {
     ) -> Self::PaintState {
         cx.scene.push_layer(Some(bounds));
 
+        cx.scene.push_mouse_region(
+            MouseRegion::new::<Self>(self.view_id, 0, visible_bounds).on_scroll({
+                let scroll_max = layout.scroll_max;
+                let state = self.state.clone();
+                move |ScrollWheelRegionEvent {
+                          platform_event:
+                              ScrollWheelEvent {
+                                  position,
+                                  delta,
+                                  precise,
+                                  ..
+                              },
+                          ..
+                      },
+                      cx| {
+                    if !Self::scroll(state.clone(), position, delta, precise, scroll_max, cx) {
+                        cx.propogate_event();
+                    }
+                }
+            }),
+        );
+
         let mut item_origin = bounds.origin()
             - vec2f(
                 0.,
@@ -300,7 +325,7 @@ impl Element for UniformList {
     fn dispatch_event(
         &mut self,
         event: &Event,
-        bounds: RectF,
+        _: RectF,
         _: RectF,
         layout: &mut Self::LayoutState,
         _: &mut Self::PaintState,
@@ -311,20 +336,6 @@ impl Element for UniformList {
             handled = item.dispatch_event(event, cx) || handled;
         }
 
-        if let Event::ScrollWheel(ScrollWheelEvent {
-            position,
-            delta,
-            precise,
-            ..
-        }) = event
-        {
-            if bounds.contains_point(*position)
-                && self.scroll(*position, *delta, *precise, layout.scroll_max, cx)
-            {
-                handled = true;
-            }
-        }
-
         handled
     }