virtual_list: Fix scroll to item not align to paddings. (#1122)

- Fixed List scroll to item at first time rendering.

crates/story/src/dropdown_story.rs

@@ -89,7 +89,7 @@ impl DropdownStory {
         let country_dropdown = cx.new(|cx| {
             DropdownState::new(
                 grouped_countries,
-                Some(IndexPath::default().row(1)),
+                Some(IndexPath::default().row(8).section(2)),
                 window,
                 cx,
             )

crates/ui/src/dropdown.rs

@@ -556,7 +556,7 @@ where
         cx: &mut Context<Self>,
     ) {
         self.list.update(cx, |list, cx| {
-            list.set_selected_index(selected_index, window, cx);
+            list._set_selected_index(selected_index, window, cx);
         });
         self.update_selected_value(window, cx);
     }

crates/ui/src/list/list.rs

@@ -60,6 +60,7 @@ pub struct List<D: ListDelegate> {
     pub(crate) size: Size,
     rows_cache: RowsCache,
     selected_index: Option<IndexPath>,
+    deferred_scroll_to_index: Option<IndexPath>,
     mouse_right_clicked_index: Option<IndexPath>,
     reset_on_cancel: bool,
     _search_task: Task<()>,
@@ -85,6 +86,7 @@ where
             query_input: Some(query_input),
             last_query: None,
             selected_index: None,
+            deferred_scroll_to_index: None,
             mouse_right_clicked_index: None,
             scroll_handle: VirtualListScrollHandle::new(),
             scroll_state: ScrollbarState::default(),
@@ -158,7 +160,7 @@ where
 
     /// Set the selected index of the list,
     /// this will also scroll to the selected item.
-    fn _set_selected_index(
+    pub(crate) fn _set_selected_index(
         &mut self,
         ix: Option<IndexPath>,
         window: &mut Window,
@@ -213,10 +215,8 @@ where
             return;
         }
 
-        if let Some(ix) = self.rows_cache.position_of(&ix) {
+        self.deferred_scroll_to_index = Some(ix);
-            self.scroll_handle.scroll_to_item(ix, ScrollStrategy::Top);
+        cx.notify();
-            cx.notify();
-        }
     }
 
     /// Get scroll handle
@@ -226,11 +226,8 @@ where
 
     pub fn scroll_to_selected_item(&mut self, _window: &mut Window, cx: &mut Context<Self>) {
         if let Some(ix) = self.selected_index {
-            if let Some(item_ix) = self.rows_cache.position_of(&ix) {
+            self.deferred_scroll_to_index = Some(ix);
-                self.scroll_handle
+            cx.notify();
-                    .scroll_to_item(item_ix, ScrollStrategy::Top);
-                cx.notify();
-            }
         }
     }
 
@@ -575,6 +572,14 @@ where
     fn render(&mut self, window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
         self.prepare_items_if_needed(window, cx);
 
+        // Scroll to the selected item if it is set.
+        if let Some(ix) = self.deferred_scroll_to_index.take() {
+            if let Some(item_ix) = self.rows_cache.position_of(&ix) {
+                self.scroll_handle
+                    .scroll_to_item(item_ix, ScrollStrategy::Top);
+            }
+        }
+
         let items_count = self.rows_cache.items_count();
         let entities_count = self.rows_cache.len();
         let loading = self.delegate.loading(cx);

crates/ui/src/virtual_list.rs

@@ -19,9 +19,10 @@ use std::{
 
 use gpui::{
     div, point, px, size, Along, AnyElement, App, AvailableSpace, Axis, Bounds, ContentMask,
-    Context, Div, Element, ElementId, Entity, GlobalElementId, Half, Hitbox, InteractiveElement,
+    Context, DeferredScrollToItem, Div, Element, ElementId, Entity, GlobalElementId, Half, Hitbox,
-    IntoElement, IsZero as _, ListSizingBehavior, Pixels, Point, Render, ScrollHandle,
+    InteractiveElement, IntoElement, IsZero as _, ListSizingBehavior, Pixels, Point, Render,
-    ScrollStrategy, Size, Stateful, StatefulInteractiveElement, StyleRefinement, Styled, Window,
+    ScrollHandle, ScrollStrategy, Size, Stateful, StatefulInteractiveElement, StyleRefinement,
+    Styled, Window,
 };
 use smallvec::SmallVec;
 
@@ -29,7 +30,8 @@ use crate::{scroll::ScrollHandleOffsetable, AxisExt};
 
 struct VirtualListScrollHandleState {
     axis: Axis,
-    items_bounds: Vec<Bounds<Pixels>>,
+    items_count: usize,
+    pub deferred_scroll_to_item: Option<DeferredScrollToItem>,
 }
 
 #[derive(Clone)]
@@ -79,7 +81,8 @@ impl VirtualListScrollHandle {
         VirtualListScrollHandle {
             state: Rc::new(RefCell::new(VirtualListScrollHandleState {
                 axis: Axis::Vertical,
-                items_bounds: vec![],
+                items_count: 0,
+                deferred_scroll_to_item: None,
             })),
             base_handle: ScrollHandle::default(),
         }
@@ -89,65 +92,25 @@ impl VirtualListScrollHandle {
         &self.base_handle
     }
 
-    fn set_items_bounds(&self, items_bounds: Vec<Bounds<Pixels>>) {
-        self.state.borrow_mut().items_bounds = items_bounds;
-    }
-
-    fn set_axis(&self, axis: Axis) {
-        self.state.borrow_mut().axis = axis;
-    }
-
     /// Scroll to the item at the given index.
     pub fn scroll_to_item(&self, ix: usize, strategy: ScrollStrategy) {
-        let state = self.state.borrow();
+        self.scroll_to_item_with_offset(ix, strategy, 0);
-        let Some(bounds) = state.items_bounds.get(ix) else {
+    }
-            return;
-        };
 
-        let axis = state.axis;
+    /// Scroll to the item at the given index, with an additional offset items.
-        let mut scroll_offset = self.base_handle.offset();
+    fn scroll_to_item_with_offset(&self, ix: usize, strategy: ScrollStrategy, offset: usize) {
-        let container_bounds = self.base_handle().bounds();
+        let mut state = self.state.borrow_mut();
-
+        state.deferred_scroll_to_item = Some(DeferredScrollToItem {
-        match strategy {
+            item_index: ix,
-            ScrollStrategy::Center => {
+            strategy,
-                if axis.is_vertical() {
+            offset,
-                    scroll_offset.y = container_bounds.top() + container_bounds.size.height.half()
+        });
-                        - bounds.top()
-                        - bounds.size.height.half()
-                } else {
-                    scroll_offset.x = container_bounds.left() + container_bounds.size.width.half()
-                        - bounds.left()
-                        - bounds.size.width.half()
-                }
-            }
-            _ => {
-                // Ref: https://github.com/zed-industries/zed/blob/0d145289e0867a8d5d63e5e1397a5ca69c9d49c3/crates/gpui/src/elements/div.rs#L3026
-                if axis.is_vertical() {
-                    if bounds.top() + scroll_offset.y < container_bounds.top() {
-                        scroll_offset.y = container_bounds.top() - bounds.top();
-                    } else if bounds.bottom() + scroll_offset.y > container_bounds.bottom() {
-                        scroll_offset.y = container_bounds.bottom() - bounds.bottom();
-                    }
-                } else {
-                    if bounds.left() + scroll_offset.x < container_bounds.left() {
-                        scroll_offset.x = container_bounds.left() - bounds.left();
-                    } else if bounds.right() + scroll_offset.x > container_bounds.right() {
-                        scroll_offset.x = container_bounds.right() - bounds.right();
-                    }
-                }
-            }
-        }
-
-        self.base_handle.set_offset(scroll_offset);
     }
 
     /// Scrolls to the bottom of the list.
     pub fn scroll_to_bottom(&self) {
-        let state = self.state.borrow();
+        let items_count = self.state.borrow().items_count;
-        self.scroll_to_item(
+        self.scroll_to_item(items_count.saturating_sub(1), ScrollStrategy::Top);
-            state.items_bounds.len().saturating_sub(1),
-            ScrollStrategy::Top,
-        );
     }
 }
 
@@ -268,6 +231,54 @@ impl VirtualList {
         self.scroll_handle = scroll_handle.clone();
         self
     }
+
+    fn scroll_to_deferred_item(
+        &self,
+        scroll_offset: Point<Pixels>,
+        items_bounds: &[Bounds<Pixels>],
+        content_bounds: &Bounds<Pixels>,
+        scroll_to_item: DeferredScrollToItem,
+    ) -> Point<Pixels> {
+        let Some(bounds) = items_bounds
+            .get(scroll_to_item.item_index + scroll_to_item.offset)
+            .cloned()
+        else {
+            return scroll_offset;
+        };
+
+        let mut scroll_offset = scroll_offset;
+        match scroll_to_item.strategy {
+            ScrollStrategy::Center => {
+                if self.axis.is_vertical() {
+                    scroll_offset.y = content_bounds.top() + content_bounds.size.height.half()
+                        - bounds.top()
+                        - bounds.size.height.half()
+                } else {
+                    scroll_offset.x = content_bounds.left() + content_bounds.size.width.half()
+                        - bounds.left()
+                        - bounds.size.width.half()
+                }
+            }
+            _ => {
+                // Ref: https://github.com/zed-industries/zed/blob/0d145289e0867a8d5d63e5e1397a5ca69c9d49c3/crates/gpui/src/elements/div.rs#L3026
+                if self.axis.is_vertical() {
+                    if bounds.top() + scroll_offset.y < content_bounds.top() {
+                        scroll_offset.y = content_bounds.top() - bounds.top()
+                    } else if bounds.bottom() + scroll_offset.y > content_bounds.bottom() {
+                        scroll_offset.y = content_bounds.bottom() - bounds.bottom();
+                    }
+                } else {
+                    if bounds.left() + scroll_offset.x < content_bounds.left() {
+                        scroll_offset.x = content_bounds.left() - bounds.left();
+                    } else if bounds.right() + scroll_offset.x > content_bounds.right() {
+                        scroll_offset.x = content_bounds.right() - bounds.right();
+                    }
+                }
+            }
+        }
+        self.scroll_handle.set_offset(scroll_offset);
+        scroll_offset
+    }
 }
 
 /// Frame state used by the [VirtualItem].
@@ -489,6 +500,19 @@ impl Element for VirtualList {
         let item_sizes = &layout.size_layout.sizes;
         let item_origins = &layout.size_layout.origins;
 
+        let content_bounds = Bounds::from_corners(
+            bounds.origin
+                + point(
+                    border_widths.left + paddings.left,
+                    border_widths.top + paddings.top,
+                ),
+            bounds.bottom_right()
+                - point(
+                    border_widths.right + paddings.right,
+                    border_widths.bottom + paddings.bottom,
+                ),
+        );
+
         // Update scroll_handle with the item bounds
         let items_bounds = item_origins
             .iter()
@@ -498,18 +522,32 @@ impl Element for VirtualList {
 
                 Bounds {
                     origin: match self.axis {
-                        Axis::Horizontal => point(bounds.left() + origin, px(0.)),
+                        Axis::Horizontal => point(content_bounds.left() + origin, px(0.)),
-                        Axis::Vertical => point(px(0.), bounds.top() + origin),
+                        Axis::Vertical => point(px(0.), content_bounds.top() + origin),
                     },
                     size: match self.axis {
-                        Axis::Horizontal => size(item_size, bounds.size.height),
+                        Axis::Horizontal => size(item_size, content_bounds.size.height),
-                        Axis::Vertical => size(bounds.size.width, item_size),
+                        Axis::Vertical => size(content_bounds.size.width, item_size),
                     },
                 }
             })
             .collect::<Vec<_>>();
-        self.scroll_handle.set_axis(self.axis);
+
-        self.scroll_handle.set_items_bounds(items_bounds);
+        let axis = self.axis;
+
+        let mut scroll_state = self.scroll_handle.state.borrow_mut();
+        scroll_state.axis = axis;
+        scroll_state.items_count = self.items_count;
+
+        let mut scroll_offset = self.scroll_handle.offset();
+        if let Some(scroll_to_item) = scroll_state.deferred_scroll_to_item.take() {
+            scroll_offset = self.scroll_to_deferred_item(
+                scroll_offset,
+                &items_bounds,
+                &content_bounds,
+                scroll_to_item,
+            );
+        }
 
         self.base.interactivity().prepaint(
             global_id,
@@ -519,24 +557,9 @@ impl Element for VirtualList {
             window,
             cx,
             |_style, _, hitbox, window, cx| {
-                let mut scroll_offset = self.scroll_handle.offset();
-
-                let padded_bounds = Bounds::from_corners(
-                    bounds.origin
-                        + point(
-                            border_widths.left + paddings.left,
-                            border_widths.top + paddings.top,
-                        ),
-                    bounds.bottom_right()
-                        - point(
-                            border_widths.right + paddings.right,
-                            border_widths.bottom + paddings.bottom,
-                        ),
-                );
-
                 if self.items_count > 0 {
                     let is_scrolled = !scroll_offset.along(self.axis).is_zero();
-                    let min_scroll_offset = padded_bounds.size.along(self.axis)
+                    let min_scroll_offset = content_bounds.size.along(self.axis)
                         - layout.size_layout.content_size.along(self.axis);
 
                     if is_scrolled {
@@ -567,7 +590,8 @@ impl Element for VirtualList {
                             let mut last_visible_element_ix = 0;
                             for (i, &size) in item_sizes.iter().enumerate() {
                                 cumulative_size += size;
-                                if cumulative_size > (-scroll_offset.x + padded_bounds.size.width) {
+                                if cumulative_size > (-scroll_offset.x + content_bounds.size.width)
+                                {
                                     last_visible_element_ix = i + 1;
                                     break;
                                 }
@@ -594,7 +618,7 @@ impl Element for VirtualList {
                             let mut last_visible_element_ix = 0;
                             for (i, &size) in item_sizes.iter().enumerate() {
                                 cumulative_size += size;
-                                if cumulative_size > (-scroll_offset.y + padded_bounds.size.height)
+                                if cumulative_size > (-scroll_offset.y + content_bounds.size.height)
                                 {
                                     last_visible_element_ix = i + 1;
                                     break;
@@ -619,11 +643,11 @@ impl Element for VirtualList {
                         for (mut item, ix) in items.into_iter().zip(visible_range.clone()) {
                             let item_origin = match self.axis {
                                 Axis::Horizontal => {
-                                    padded_bounds.origin
+                                    content_bounds.origin
                                         + point(item_origins[ix] + scroll_offset.x, scroll_offset.y)
                                 }
                                 Axis::Vertical => {
-                                    padded_bounds.origin
+                                    content_bounds.origin
                                         + point(scroll_offset.x, item_origins[ix] + scroll_offset.y)
                                 }
                             };
@@ -631,10 +655,10 @@ impl Element for VirtualList {
                             let available_space = match self.axis {
                                 Axis::Horizontal => size(
                                     AvailableSpace::Definite(item_sizes[ix]),
-                                    AvailableSpace::Definite(padded_bounds.size.height),
+                                    AvailableSpace::Definite(content_bounds.size.height),
                                 ),
                                 Axis::Vertical => size(
-                                    AvailableSpace::Definite(padded_bounds.size.width),
+                                    AvailableSpace::Definite(content_bounds.size.width),
                                     AvailableSpace::Definite(item_sizes[ix]),
                                 ),
                             };