yenru0/zed
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

gpui: Fix scrollbar drag position calculation in list (#53378)

Browse Source 
## Summary

Fixes scroll position inversion when content height changes during a
scrollbar drag (e.g. in the agent panel while streaming responses).

### Root cause

`set_offset_from_scrollbar` used the **live** `items.summary().height`
for `content_height` and computed a `drag_offset` correction (`live −
frozen`). However, `max_offset_for_scrollbar()` → `max_scroll_offset()`
already uses the **frozen** `scrollbar_drag_start_height` during drag.
This mismatch means:

1. The scrollbar computes thumb position using the frozen height range
2. `set_offset_from_scrollbar` converts that position using the live
height range
3. As content grows during drag, `drag_offset` increases
4. `(point.y - drag_offset).abs()` produces an incorrectly large value,
overshooting `scroll_max`

The `.abs()` call also masked the sign convention:
`compute_click_offset` (in `scrollbar.rs`) returns `-max_offset *
percentage` — a negative value — but `.abs()` converted it regardless of
sign, hiding the mismatch.

### Fix

- Use `scrollbar_drag_start_height` (frozen during drag) for
`content_height` in `set_offset_from_scrollbar`, matching
`max_scroll_offset()`. Both sides of the scrollbar mapping now use the
same height reference.
- Replace `(point.y - drag_offset).abs()` with `(-point.y)` to correctly
convert the negative scroll offset.
- Remove the symmetric `drag_offset` from
`scroll_px_offset_for_scrollbar`, which reported position back to the
scrollbar and suffered from the same inconsistency.
- Update existing tests to use negative offsets, matching the actual
values produced by the scrollbar component.
- Add a regression test that freezes height, grows content, drags the
scrollbar, and asserts the position is correct.

## Test plan

- [x] Existing tests updated to use correct sign convention (negative
offsets)
- [x] New regression test `test_scrollbar_drag_with_growing_content`:
verifies scroll position remains correct when content grows during a
scrollbar drag
- [ ] Manual: open agent panel, start a long generation, drag scrollbar
while content is streaming — position should track the thumb correctly

Release Notes:

- Fixed scrollbar position jumping or inverting when content height
changes during a scrollbar drag (e.g. in the agent panel while
streaming).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Smit Barmase <heysmitbarmase@gmail.com>
This commit is contained in:
KlausUllrichandSmit Barmase authored and GitHub committed 2026-05-12 19:17:38 +00:00
1 parent 7277654c40
commit cab449cdcf
1 file changed
+142 -15
Download Patch File Download Diff File Expand all files Collapse all files
crates/gpui/src/elements/list.rs
+142 -15
View File
@@ -716,7 +716,10 @@ impl ListState {
point(Pixels::ZERO, state.max_scroll_offset())
}
/// Returns the current scroll offset adjusted for the scrollbar
/// Returns the current scroll offset adjusted for the scrollbar.
///
/// The returned offset has a negative `y` component representing
/// how far the content has scrolled.
pub fn scroll_px_offset_for_scrollbar(&self) -> Point<Pixels> {
let state = &self.0.borrow();
@@ -729,11 +732,7 @@ impl ListState {
let mut cursor = state.items.cursor::<ListItemSummary>(());
let summary: ListItemSummary =
cursor.summary(&Count(logical_scroll_top.item_ix), Bias::Right);
let content_height = state.items.summary().height;
let drag_offset =
// if dragging the scrollbar, we want to offset the point if the height changed
content_height - state.scrollbar_drag_start_height.unwrap_or(content_height);
let offset = summary.height + logical_scroll_top.offset_in_item - drag_offset;
let offset = summary.height + logical_scroll_top.offset_in_item;
Point::new(px(0.), -offset)
}
@@ -1212,12 +1211,27 @@ impl StateInner {
let height = bounds.size.height;
let padding = self.last_padding.unwrap_or_default();
let content_height = self.items.summary().height;
// Scrollbar drag positions are computed from the content height
// captured at drag start, so map them back using the same height.
let content_height = self
.scrollbar_drag_start_height
.unwrap_or_else(|| self.items.summary().height);
let scroll_max = (content_height + padding.top + padding.bottom - height).max(px(0.));
let drag_offset =
// if dragging the scrollbar, we want to offset the point if the height changed
content_height - self.scrollbar_drag_start_height.unwrap_or(content_height);
let new_scroll_top = (point.y - drag_offset).abs().max(px(0.)).min(scroll_max);
let new_scroll_top = (-point.y).max(px(0.)).min(scroll_max);
// If content grew during the drag, the frozen bottom is below the
// live bottom. Treat dragging to the frozen end as resuming tail follow.
let dragged_to_end =
scroll_max > px(0.) && new_scroll_top >= (scroll_max - px(1.0)).max(px(0.));
if dragged_to_end && matches!(self.follow_state, FollowState::Tail { .. }) {
self.follow_state = FollowState::Tail { is_following: true };
let item_count = self.items.summary().count;
self.logical_scroll_top = Some(ListOffset {
item_ix: item_count,
offset_in_item: px(0.),
});
return;
}
self.follow_state.stop_following();
@@ -1923,8 +1937,7 @@ mod test {
assert!(state.is_following_tail());
// Simulate the scrollbar moving the viewport to the middle.
// `set_offset_from_scrollbar` accepts a positive distance from the start.
state.set_offset_from_scrollbar(point(px(0.), px(150.)));
state.set_offset_from_scrollbar(point(px(0.), px(-150.)));
let offset = state.logical_scroll_top();
assert_eq!(offset.item_ix, 3);
@@ -1945,6 +1958,71 @@ mod test {
assert_eq!(offset.offset_in_item, px(0.));
}
#[gpui::test]
fn test_scrollbar_drag_with_growing_content(cx: &mut TestAppContext) {
let cx = cx.add_empty_window();
let last_item_height = Rc::new(Cell::new(50usize));
let state = ListState::new(10, crate::ListAlignment::Top, px(0.)).measure_all();
struct TestView {
state: ListState,
last_item_height: Rc<Cell<usize>>,
}
impl Render for TestView {
fn render(&mut self, _: &mut Window, _: &mut Context<Self>) -> impl IntoElement {
let last_item_height = self.last_item_height.clone();
list(self.state.clone(), move |index, _, _| {
let height = if index == 9 {
last_item_height.get()
} else {
50
};
div().h(px(height as f32)).w_full().into_any()
})
.w_full()
.h_full()
}
}
let view = cx.update(|_, cx| {
cx.new(|_| TestView {
state: state.clone(),
last_item_height: last_item_height.clone(),
})
});
cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, _| {
view.clone().into_any_element()
});
state.scrollbar_drag_started();
state.set_offset_from_scrollbar(point(px(0.), px(-150.)));
let scrollbar_offset_before_growth = state.scroll_px_offset_for_scrollbar();
let offset = state.logical_scroll_top();
assert_eq!(offset.item_ix, 3);
assert_eq!(offset.offset_in_item, px(0.));
last_item_height.set(550);
state.remeasure_items(9..10);
cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, _| {
view.clone().into_any_element()
});
assert_eq!(state.max_offset_for_scrollbar().y, px(300.));
assert_eq!(
state.scroll_px_offset_for_scrollbar(),
scrollbar_offset_before_growth
);
state.set_offset_from_scrollbar(point(px(0.), px(-150.)));
let offset = state.logical_scroll_top();
assert_eq!(offset.item_ix, 3);
assert_eq!(offset.offset_in_item, px(0.));
}
#[gpui::test]
fn test_set_follow_tail_snaps_to_bottom(cx: &mut TestAppContext) {
let cx = cx.add_empty_window();
@@ -2182,12 +2260,12 @@ mod test {
assert!(state.is_following_tail());
// Drag the scrollbar up to the middle — follow_tail should suspend.
state.set_offset_from_scrollbar(point(px(0.), px(150.)));
state.set_offset_from_scrollbar(point(px(0.), px(-150.)));
assert!(!state.is_following_tail());
// Drag the scrollbar back to the bottom — follow_tail should re-engage
// on the next paint.
state.set_offset_from_scrollbar(point(px(0.), px(300.)));
state.set_offset_from_scrollbar(point(px(0.), px(-300.)));
cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, _| {
view.into_any_element()
});
@@ -2196,4 +2274,53 @@ mod test {
"follow_tail should re-engage after scrolling back to the bottom via the scrollbar"
);
}
#[gpui::test]
fn test_follow_tail_reengages_after_scrollbar_drag_to_bottom_while_growing(
cx: &mut TestAppContext,
) {
let cx = cx.add_empty_window();
let state = ListState::new(10, crate::ListAlignment::Top, px(0.)).measure_all();
struct TestView(ListState);
impl Render for TestView {
fn render(&mut self, _: &mut Window, _: &mut Context<Self>) -> impl IntoElement {
list(self.0.clone(), |_, _, _| {
div().h(px(50.)).w_full().into_any()
})
.w_full()
.h_full()
}
}
let view = cx.update(|_, cx| cx.new(|_| TestView(state.clone())));
state.set_follow_mode(FollowMode::Tail);
cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, _| {
view.clone().into_any_element()
});
assert!(state.is_following_tail());
state.scrollbar_drag_started();
state.splice(10..10, 10);
cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, _| {
view.clone().into_any_element()
});
state.set_offset_from_scrollbar(point(px(0.), px(-300.)));
state.scrollbar_drag_ended();
cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, _| {
view.into_any_element()
});
assert!(
state.is_following_tail(),
"follow_tail should re-engage when the user drags the scrollbar to \
the bottom of its track, even when content has grown during the drag \
(so frozen_bottom < live_bottom)"
);
}
}