From dfd8328f7b87a959c46d51bf964a58158a5019ab Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Wed, 6 May 2026 16:05:53 -0400 Subject: [PATCH] gpui: Fix material list unstable scrollbar position (#55808) Before this PR, `ListState` always used a proportional pending scroll fraction to preserve the relative scroll position when list items were resized. This caused text to creep in the agent panel when the scrollbar was in the middle of the list while the agent was generating text, because streaming updates changed the logical scroll top slightly. This PR fixes that behavior by using an absolute offset when only a subset of items is remeasured, added, or deleted, keeping the logical top position stable. 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 is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Closes #ISSUE Release Notes: - N/A --------- Co-authored-by: Remco Smits --- crates/gpui/src/elements/list.rs | 162 +++++++++++++++++++++++++------ 1 file changed, 134 insertions(+), 28 deletions(-) diff --git a/crates/gpui/src/elements/list.rs b/crates/gpui/src/elements/list.rs index 558e89dd83e752af4d97fe828c8aac5789376373..d417c14e49bed69f4e5dce929b2f839216795549 100644 --- a/crates/gpui/src/elements/list.rs +++ b/crates/gpui/src/elements/list.rs @@ -71,12 +71,27 @@ struct StateInner { scroll_handler: Option>, scrollbar_drag_start_height: Option, measuring_behavior: ListMeasuringBehavior, - pending_scroll: Option, + pending_scroll: Option, follow_state: FollowState, } +/// Deferred scroll adjustment applied after the scroll-top item has been remeasured. +/// +/// An absolute pending scroll preserves the same pixel offset into the item, which keeps +/// visible text stable while content is appended to or removed from that item. A +/// proportional pending scroll preserves the same fractional position within the item, +/// which is useful when the whole list is being resized and each item scales similarly. +#[derive(Clone)] +enum PendingScroll { + /// Preserve the same pixel offset into the item after it is remeasured. + Absolute { item_ix: usize, offset: Pixels }, + /// Preserve the same fractional offset into the item after it is remeasured. + Proportional(PendingScrollFraction), +} + /// Keeps track of a fractional scroll position within an item for restoration /// after remeasurement. +#[derive(Clone)] struct PendingScrollFraction { /// The index of the item to scroll within. item_ix: usize, @@ -84,6 +99,15 @@ struct PendingScrollFraction { fraction: f32, } +/// Determines how remeasurement preserves the scroll position when the scroll-top item +/// changes height. +enum ScrollAnchor { + /// Preserve the same pixel offset into the scroll-top item. + Absolute, + /// Preserve the same fractional position within the scroll-top item. + Proportional, +} + /// Controls whether the list automatically follows new content at the end. #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] pub enum FollowMode { @@ -336,7 +360,7 @@ impl ListState { /// but the number and identity of items remains the same. pub fn remeasure(&self) { let count = self.item_count(); - self.remeasure_items(0..count); + self.remeasure_items_with_scroll_anchor(0..count, ScrollAnchor::Proportional); } /// Mark items in `range` as needing remeasurement while preserving @@ -347,31 +371,47 @@ impl ListState { /// height may be different (e.g., streaming text, tool results /// loading), but the item itself still exists at the same index. pub fn remeasure_items(&self, range: Range) { + self.remeasure_items_with_scroll_anchor(range, ScrollAnchor::Absolute); + } + + fn remeasure_items_with_scroll_anchor(&self, range: Range, scroll_anchor: ScrollAnchor) { let state = &mut *self.0.borrow_mut(); - // If the scroll-top item falls within the remeasured range, - // store a fractional offset so the layout can restore the - // proportional scroll position after the item is re-rendered - // at its new height. if let Some(scroll_top) = state.logical_scroll_top { if range.contains(&scroll_top.item_ix) { - let mut cursor = state.items.cursor::(()); - cursor.seek(&Count(scroll_top.item_ix), Bias::Right); - - if let Some(item) = cursor.item() { - if let Some(size) = item.size() { - let fraction = if size.height.0 > 0.0 { - (scroll_top.offset_in_item.0 / size.height.0).clamp(0.0, 1.0) - } else { - 0.0 - }; - - state.pending_scroll = Some(PendingScrollFraction { - item_ix: scroll_top.item_ix, - fraction, - }); + state.pending_scroll = match scroll_anchor { + ScrollAnchor::Absolute => Some(PendingScroll::Absolute { + item_ix: scroll_top.item_ix, + offset: scroll_top.offset_in_item, + }), + ScrollAnchor::Proportional => { + // If the scroll-top item falls within the remeasured range, + // store a fractional offset so the layout can restore the + // proportional scroll position after the item is re-rendered + // at its new height. + let mut cursor = state.items.cursor::(()); + cursor.seek(&Count(scroll_top.item_ix), Bias::Right); + + cursor + .item() + .and_then(|item| { + item.size().map(|size| { + let fraction = if size.height.0 > 0.0 { + (scroll_top.offset_in_item.0 / size.height.0) + .clamp(0.0, 1.0) + } else { + 0.0 + }; + + PendingScroll::Proportional(PendingScrollFraction { + item_ix: scroll_top.item_ix, + fraction, + }) + }) + }) + .or_else(|| state.pending_scroll.clone()) } - } + }; } } @@ -894,14 +934,26 @@ impl StateInner { size = Some(element_size); // If there's a pending scroll adjustment for the scroll-top - // item, apply it, ensuring proportional scroll position is - // maintained after re-measuring. + // item, apply it. if ix == 0 { if let Some(pending_scroll) = self.pending_scroll.take() { - if pending_scroll.item_ix == scroll_top.item_ix { - scroll_top.offset_in_item = - Pixels(pending_scroll.fraction * element_size.height.0); - self.logical_scroll_top = Some(scroll_top); + match pending_scroll { + PendingScroll::Absolute { item_ix, offset } + if item_ix == scroll_top.item_ix => + { + scroll_top.offset_in_item = offset.min(element_size.height); + self.logical_scroll_top = Some(scroll_top); + } + PendingScroll::Proportional(pending_scroll) + if pending_scroll.item_ix == scroll_top.item_ix => + { + // Ensuring proportional scroll position is + // maintained after re-measuring. + scroll_top.offset_in_item = + Pixels(pending_scroll.fraction * element_size.height.0); + self.logical_scroll_top = Some(scroll_top); + } + _ => {} } } } @@ -1669,6 +1721,60 @@ mod test { assert_eq!(offset.offset_in_item, px(20.)); } + #[gpui::test] + fn test_remeasure_item_preserves_scroll_offset(cx: &mut TestAppContext) { + let cx = cx.add_empty_window(); + + let item_height = Rc::new(Cell::new(100usize)); + let state = ListState::new(20, crate::ListAlignment::Top, px(10.)); + + struct TestView { + state: ListState, + item_height: Rc>, + } + + impl Render for TestView { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + let height = self.item_height.get(); + list(self.state.clone(), move |index, _, _| { + let height = if index == 5 { height } else { 100 }; + div().h(px(height as f32)).w_full().into_any() + }) + .w_full() + .h_full() + } + } + + let state_clone = state.clone(); + let item_height_clone = item_height.clone(); + let view = cx.update(|_, cx| { + cx.new(|_| TestView { + state: state_clone, + item_height: item_height_clone, + }) + }); + + state.scroll_to(gpui::ListOffset { + item_ix: 5, + offset_in_item: px(40.), + }); + + cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, _| { + view.clone().into_any_element() + }); + + item_height.set(200); + state.remeasure_items(5..6); + + cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, _| { + view.into_any_element() + }); + + let offset = state.logical_scroll_top(); + assert_eq!(offset.item_ix, 5); + assert_eq!(offset.offset_in_item, px(40.)); + } + #[gpui::test] fn test_follow_tail_stays_at_bottom_as_items_grow(cx: &mut TestAppContext) { let cx = cx.add_empty_window();