Drop scroll events if there's been a reset

co-authored-by: Nathan <nathan@zed.dev>
co-authored-by: Conrad <conrad@zed.dev>

crates/gpui/src/app/test_context.rs

@@ -643,7 +643,8 @@ impl<'a> VisualTestContext {
     /// Simulate an event from the platform, e.g. a SrollWheelEvent
     /// Make sure you've called [VisualTestContext::draw] first!
     pub fn simulate_event<E: InputEvent>(&mut self, event: E) {
-        self.update(|cx| cx.dispatch_event(event.to_platform_input()));
+        self.test_window(self.window)
+            .simulate_input(event.to_platform_input());
         self.background_executor.run_until_parked();
     }
 

crates/gpui/src/elements/list.rs

@@ -28,9 +28,9 @@ struct StateInner {
     render_item: Box<dyn FnMut(usize, &mut WindowContext) -> AnyElement>,
     items: SumTree<ListItem>,
     logical_scroll_top: Option<ListOffset>,
-    pending_scroll_delta: Pixels,
     alignment: ListAlignment,
     overdraw: Pixels,
+    reset: bool,
     #[allow(clippy::type_complexity)]
     scroll_handler: Option<Box<dyn FnMut(&ListScrollEvent, &mut WindowContext)>>,
 }
@@ -93,12 +93,17 @@ impl ListState {
             alignment: orientation,
             overdraw,
             scroll_handler: None,
-            pending_scroll_delta: px(0.),
+            reset: false,
         })))
     }
 
+    /// Reset this instantiation of the list state.
+    ///
+    /// Note that this will cause scroll events to be dropped until the next paint.
     pub fn reset(&self, element_count: usize) {
         let state = &mut *self.0.borrow_mut();
+        state.reset = true;
+
         state.logical_scroll_top = None;
         state.items = SumTree::new();
         state
@@ -154,11 +159,13 @@ impl ListState {
             scroll_top.item_ix = item_count;
             scroll_top.offset_in_item = px(0.);
         }
+
         state.logical_scroll_top = Some(scroll_top);
     }
 
     pub fn scroll_to_reveal_item(&self, ix: usize) {
         let state = &mut *self.0.borrow_mut();
+
         let mut scroll_top = state.logical_scroll_top();
         let height = state
             .last_layout_bounds
@@ -189,9 +196,9 @@ impl ListState {
     /// Get the bounds for the given item in window coordinates.
     pub fn bounds_for_item(&self, ix: usize) -> Option<Bounds<Pixels>> {
         let state = &*self.0.borrow();
+
         let bounds = state.last_layout_bounds.unwrap_or_default();
         let scroll_top = state.logical_scroll_top();
-
         if ix < scroll_top.item_ix {
             return None;
         }
@@ -232,7 +239,11 @@ impl StateInner {
         delta: Point<Pixels>,
         cx: &mut WindowContext,
     ) {
-        // self.pending_scroll_delta += delta.y;
+        // Drop scroll events after a reset, since we can't calculate
+        // the new logical scroll top without the item heights
+        if self.reset {
+            return;
+        }
 
         let scroll_max = (self.items.summary().height - height).max(px(0.));
         let new_scroll_top = (self.scroll_top(scroll_top) - delta.y)
@@ -329,6 +340,8 @@ impl Element for List {
     ) {
         let state = &mut *self.state.0.borrow_mut();
 
+        state.reset = false;
+
         // If the width of the list has changed, invalidate all cached item heights
         if state.last_layout_bounds.map_or(true, |last_bounds| {
             last_bounds.size.width != bounds.size.width
@@ -352,117 +365,104 @@ impl Element for List {
 
         let mut cursor = old_items.cursor::<Count>();
 
-        loop {
-            // Render items after the scroll top, including those in the trailing overdraw
-            cursor.seek(&Count(scroll_top.item_ix), Bias::Right, &());
-            for (ix, item) in cursor.by_ref().enumerate() {
-                let visible_height = rendered_height - scroll_top.offset_in_item;
-                if visible_height >= bounds.size.height + state.overdraw {
-                    break;
-                }
-
-                // Use the previously cached height if available
-                let mut height = if let ListItem::Rendered { height } = item {
-                    Some(*height)
-                } else {
-                    None
-                };
-
-                // If we're within the visible area or the height wasn't cached, render and measure the item's element
-                if visible_height < bounds.size.height || height.is_none() {
-                    let mut element = (state.render_item)(scroll_top.item_ix + ix, cx);
-                    let element_size = element.measure(available_item_space, cx);
-                    height = Some(element_size.height);
-                    if visible_height < bounds.size.height {
-                        item_elements.push_back(element);
-                    }
-                }
-
-                let height = height.unwrap();
-                rendered_height += height;
-                measured_items.push_back(ListItem::Rendered { height });
+        // Render items after the scroll top, including those in the trailing overdraw
+        cursor.seek(&Count(scroll_top.item_ix), Bias::Right, &());
+        for (ix, item) in cursor.by_ref().enumerate() {
+            let visible_height = rendered_height - scroll_top.offset_in_item;
+            if visible_height >= bounds.size.height + state.overdraw {
+                break;
             }
 
-            // Prepare to start walking upward from the item at the scroll top.
-            cursor.seek(&Count(scroll_top.item_ix), Bias::Right, &());
+            // Use the previously cached height if available
+            let mut height = if let ListItem::Rendered { height } = item {
+                Some(*height)
+            } else {
+                None
+            };
 
-            // If the rendered items do not fill the visible region, then adjust
-            // the scroll top upward.
-            if rendered_height - scroll_top.offset_in_item < bounds.size.height {
-                while rendered_height < bounds.size.height {
-                    cursor.prev(&());
-                    if cursor.item().is_some() {
-                        let mut element = (state.render_item)(cursor.start().0, cx);
-                        let element_size = element.measure(available_item_space, cx);
-
-                        rendered_height += element_size.height;
-                        measured_items.push_front(ListItem::Rendered {
-                            height: element_size.height,
-                        });
-                        item_elements.push_front(element)
-                    } else {
-                        break;
-                    }
-                }
-
-                scroll_top = ListOffset {
-                    item_ix: cursor.start().0,
-                    offset_in_item: rendered_height - bounds.size.height,
-                };
-
-                match state.alignment {
-                    ListAlignment::Top => {
-                        scroll_top.offset_in_item = scroll_top.offset_in_item.max(px(0.));
-                        state.logical_scroll_top = Some(scroll_top);
-                    }
-                    ListAlignment::Bottom => {
-                        scroll_top = ListOffset {
-                            item_ix: cursor.start().0,
-                            offset_in_item: rendered_height - bounds.size.height,
-                        };
-                        state.logical_scroll_top = None;
-                    }
-                };
-            }
-
-            // Measure items in the leading overdraw
-            let mut leading_overdraw = scroll_top.offset_in_item;
-            while leading_overdraw < state.overdraw {
-                cursor.prev(&());
-                if let Some(item) = cursor.item() {
-                    let height = if let ListItem::Rendered { height } = item {
-                        *height
-                    } else {
-                        let mut element = (state.render_item)(cursor.start().0, cx);
-                        element.measure(available_item_space, cx).height
-                    };
-
-                    leading_overdraw += height;
-                    measured_items.push_front(ListItem::Rendered { height });
-                } else {
-                    break;
+            // If we're within the visible area or the height wasn't cached, render and measure the item's element
+            if visible_height < bounds.size.height || height.is_none() {
+                let mut element = (state.render_item)(scroll_top.item_ix + ix, cx);
+                let element_size = element.measure(available_item_space, cx);
+                height = Some(element_size.height);
+                if visible_height < bounds.size.height {
+                    item_elements.push_back(element);
                 }
             }
 
-            let measured_range = cursor.start().0..(cursor.start().0 + measured_items.len());
-            let mut cursor = old_items.cursor::<Count>();
-            let mut new_items = cursor.slice(&Count(measured_range.start), Bias::Right, &());
-            new_items.extend(measured_items, &());
-            cursor.seek(&Count(measured_range.end), Bias::Right, &());
-            new_items.append(cursor.suffix(&()), &());
-
-            state.items = new_items;
-            state.last_layout_bounds = Some(bounds);
-
-            // if !state.pending_scroll_delta.is_zero() {
-            //     // Do scroll manipulation
-
-            //     state.pending_scroll_delta = px(0.);
-            // } else {
-            break;
-            // }
+            let height = height.unwrap();
+            rendered_height += height;
+            measured_items.push_back(ListItem::Rendered { height });
         }
 
+        // Prepare to start walking upward from the item at the scroll top.
+        cursor.seek(&Count(scroll_top.item_ix), Bias::Right, &());
+
+        // If the rendered items do not fill the visible region, then adjust
+        // the scroll top upward.
+        if rendered_height - scroll_top.offset_in_item < bounds.size.height {
+            while rendered_height < bounds.size.height {
+                cursor.prev(&());
+                if cursor.item().is_some() {
+                    let mut element = (state.render_item)(cursor.start().0, cx);
+                    let element_size = element.measure(available_item_space, cx);
+
+                    rendered_height += element_size.height;
+                    measured_items.push_front(ListItem::Rendered {
+                        height: element_size.height,
+                    });
+                    item_elements.push_front(element)
+                } else {
+                    break;
+                }
+            }
+
+            scroll_top = ListOffset {
+                item_ix: cursor.start().0,
+                offset_in_item: rendered_height - bounds.size.height,
+            };
+
+            match state.alignment {
+                ListAlignment::Top => {
+                    scroll_top.offset_in_item = scroll_top.offset_in_item.max(px(0.));
+                    state.logical_scroll_top = Some(scroll_top);
+                }
+                ListAlignment::Bottom => {
+                    scroll_top = ListOffset {
+                        item_ix: cursor.start().0,
+                        offset_in_item: rendered_height - bounds.size.height,
+                    };
+                    state.logical_scroll_top = None;
+                }
+            };
+        }
+
+        // Measure items in the leading overdraw
+        let mut leading_overdraw = scroll_top.offset_in_item;
+        while leading_overdraw < state.overdraw {
+            cursor.prev(&());
+            if let Some(item) = cursor.item() {
+                let height = if let ListItem::Rendered { height } = item {
+                    *height
+                } else {
+                    let mut element = (state.render_item)(cursor.start().0, cx);
+                    element.measure(available_item_space, cx).height
+                };
+
+                leading_overdraw += height;
+                measured_items.push_front(ListItem::Rendered { height });
+            } else {
+                break;
+            }
+        }
+
+        let measured_range = cursor.start().0..(cursor.start().0 + measured_items.len());
+        let mut cursor = old_items.cursor::<Count>();
+        let mut new_items = cursor.slice(&Count(measured_range.start), Bias::Right, &());
+        new_items.extend(measured_items, &());
+        cursor.seek(&Count(measured_range.end), Bias::Right, &());
+        new_items.append(cursor.suffix(&()), &());
+
         // Paint the visible items
         cx.with_content_mask(Some(ContentMask { bounds }), |cx| {
             let mut item_origin = bounds.origin;
@@ -474,12 +474,13 @@ impl Element for List {
             }
         });
 
+        state.items = new_items;
+        state.last_layout_bounds = Some(bounds);
+
         let list_state = self.state.clone();
         let height = bounds.size.height;
-        dbg!("scroll is being bound");
 
         cx.on_mouse_event(move |event: &ScrollWheelEvent, phase, cx| {
-            dbg!("scroll dispatched!");
             if phase == DispatchPhase::Bubble
                 && bounds.contains(&event.position)
                 && cx.was_top_layer(&event.position, cx.stacking_order())
@@ -624,7 +625,5 @@ mod test {
         // Scroll position should stay at the top of the list
         assert_eq!(state.logical_scroll_top().item_ix, 0);
         assert_eq!(state.logical_scroll_top().offset_in_item, px(0.));
-
-        panic!("We should not get here yet!")
     }
 }

crates/gpui/src/keymap/matcher.rs

@@ -209,7 +209,6 @@ mod tests {
         );
         assert!(!matcher.has_pending_keystrokes());
 
-        eprintln!("PROBLEM AREA");
         // If a is prefixed, C will not be dispatched because there
         // was a pending binding for it
         assert_eq!(

crates/gpui/src/window.rs

@@ -1717,7 +1717,6 @@ impl<'a> WindowContext<'a> {
             .mouse_listeners
             .remove(&event.type_id())
         {
-            dbg!(handlers.len());
             // Because handlers may add other handlers, we sort every time.
             handlers.sort_by(|(a, _, _), (b, _, _)| a.cmp(b));