mirror of
https://github.com/zed-industries/zed.git
synced 2026-06-01 03:14:56 +07:00
gpui: Fix bottom-aligned scroll bar disappearing (#51223)
Closes #51198 This actually doesn't effect any of the scrollbars in Zed, as they have a separate handler that prevents this issue from occurring in `crates/ui/src/components/scrollbar.rs`, line 856 ```rust let current_offset = current_offset .along(axis) .clamp(-max_offset, Pixels::ZERO) .abs(); ``` so it is gpui specific. I still added a test case and I have a manual test script: <details><summary>Details</summary> <p> ```rust //! Reproduction of the scrollbar-offset bug in bottom-aligned `ListState`. //! //! Run with: cargo run -p gpui --example list_bottom_scrollbar_bug //! //! The list starts pinned to the bottom. Before the fix, the red scrollbar //! thumb was pushed off the bottom of the track (invisible). use gpui::{ App, Bounds, Context, ListAlignment, ListState, Window, WindowBounds, WindowOptions, div, list, prelude::*, px, rgb, size, }; use gpui_platform::application; const ITEM_COUNT: usize = 40; const COLORS: [u32; 4] = [0xE8F0FE, 0xFCE8E6, 0xE6F4EA, 0xFEF7E0]; struct BugRepro { list_state: ListState, } impl BugRepro { fn new() -> Self { let list_state = ListState::new(ITEM_COUNT, ListAlignment::Bottom, px(5000.)); Self { list_state } } } impl Render for BugRepro { fn render(&mut self, _window: &mut Window, _cx: &mut Context<Self>) -> impl IntoElement { let state = &self.list_state; let max_offset = state.max_offset_for_scrollbar().y; let raw_offset = -state.scroll_px_offset_for_scrollbar().y; let viewport_h = state.viewport_bounds().size.height; let content_h = max_offset + viewport_h; let thumb_h = if content_h > px(0.) { ((viewport_h / content_h) * viewport_h).max(px(20.)) } else { viewport_h }; let thumb_top = if max_offset > px(0.) { (raw_offset / max_offset) * (viewport_h - thumb_h) } else { px(0.) }; div() .size_full() .flex() .flex_row() .bg(rgb(0xffffff)) .text_color(rgb(0x333333)) .text_xl() .child( div().flex_1().h_full().child( list(state.clone(), |ix, _window, _cx| { let height = if ix % 4 == 0 { px(70.) } else { px(40.) }; let bg = COLORS[ix % COLORS.len()]; div() .h(height) .w_full() .bg(rgb(bg)) .border_b_1() .border_color(rgb(0xcccccc)) .px_2() .flex() .items_center() .child(format!("Item {ix}")) .into_any() }) .h_full() .w_full(), ), ) .child( div() .w(px(14.)) .h_full() .bg(rgb(0xe0e0e0)) .relative() .child( div() .absolute() .right(px(0.)) .top(thumb_top) .h(thumb_h) .w(px(14.)) .rounded_sm() .bg(rgb(0xff3333)), ), ) } } fn main() { application().run(|cx: &mut App| { let bounds = Bounds::centered(None, size(px(400.), px(500.)), cx); cx.open_window( WindowOptions { window_bounds: Some(WindowBounds::Windowed(bounds)), ..Default::default() }, |_, cx| cx.new(|_| BugRepro::new()), ) .unwrap(); cx.activate(true); }); } ``` </p> </details> where I was able to test it out, here is a video of the new working behavior. https://github.com/user-attachments/assets/02e26308-da18-418b-97fc-dd52a3325dab Release Notes: - gpui: fixed a bug where the scollbar would disappear when using a bottom aligned list. --------- Co-authored-by: Mikayla Maki <mikayla.c.maki@gmail.com>
This commit is contained in:
parent
0a4dfe327a
commit
ce0848af5c
3 changed files with 230 additions and 7 deletions
|
|
@ -235,6 +235,10 @@ path = "examples/window_shadow.rs"
|
|||
name = "grid_layout"
|
||||
path = "examples/grid_layout.rs"
|
||||
|
||||
[[example]]
|
||||
name = "list_example"
|
||||
path = "examples/list_example.rs"
|
||||
|
||||
[[example]]
|
||||
name = "mouse_pressure"
|
||||
path = "examples/mouse_pressure.rs"
|
||||
|
|
|
|||
170
crates/gpui/examples/list_example.rs
Normal file
170
crates/gpui/examples/list_example.rs
Normal file
|
|
@ -0,0 +1,170 @@
|
|||
#![cfg_attr(target_family = "wasm", no_main)]
|
||||
|
||||
use gpui::{
|
||||
App, Bounds, Context, ListAlignment, ListState, Render, Window, WindowBounds, WindowOptions,
|
||||
div, list, prelude::*, px, rgb, size,
|
||||
};
|
||||
use gpui_platform::application;
|
||||
|
||||
const ITEM_COUNT: usize = 40;
|
||||
const SCROLLBAR_WIDTH: f32 = 12.;
|
||||
|
||||
struct BottomListDemo {
|
||||
list_state: ListState,
|
||||
}
|
||||
|
||||
impl BottomListDemo {
|
||||
fn new() -> Self {
|
||||
Self {
|
||||
list_state: ListState::new(ITEM_COUNT, ListAlignment::Bottom, px(500.)).measure_all(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Render for BottomListDemo {
|
||||
fn render(&mut self, _window: &mut Window, _cx: &mut Context<Self>) -> impl IntoElement {
|
||||
let max_offset = self.list_state.max_offset_for_scrollbar().y;
|
||||
let current_offset = -self.list_state.scroll_px_offset_for_scrollbar().y;
|
||||
|
||||
let viewport_height = self.list_state.viewport_bounds().size.height;
|
||||
|
||||
let raw_fraction = if max_offset > px(0.) {
|
||||
current_offset / max_offset
|
||||
} else {
|
||||
0.
|
||||
};
|
||||
|
||||
let total_height = viewport_height + max_offset;
|
||||
let thumb_height = if total_height > px(0.) {
|
||||
px(viewport_height.as_f32() * viewport_height.as_f32() / total_height.as_f32())
|
||||
.max(px(30.))
|
||||
} else {
|
||||
px(30.)
|
||||
};
|
||||
|
||||
let track_space = viewport_height - thumb_height;
|
||||
let thumb_top = track_space * raw_fraction;
|
||||
|
||||
let bug_detected = raw_fraction > 1.0;
|
||||
|
||||
div()
|
||||
.size_full()
|
||||
.bg(rgb(0xFFFFFF))
|
||||
.flex()
|
||||
.flex_col()
|
||||
.p_4()
|
||||
.gap_2()
|
||||
.child(
|
||||
div()
|
||||
.text_sm()
|
||||
.flex()
|
||||
.flex_col()
|
||||
.gap_1()
|
||||
.child(format!(
|
||||
"offset: {:.0} / max: {:.0} | fraction: {:.3}",
|
||||
current_offset.as_f32(),
|
||||
max_offset.as_f32(),
|
||||
raw_fraction,
|
||||
))
|
||||
.child(
|
||||
div()
|
||||
.text_color(if bug_detected {
|
||||
rgb(0xCC0000)
|
||||
} else {
|
||||
rgb(0x008800)
|
||||
})
|
||||
.child(if bug_detected {
|
||||
format!(
|
||||
"BUG: fraction is {:.3} (> 1.0) — thumb is off-track!",
|
||||
raw_fraction
|
||||
)
|
||||
} else {
|
||||
"OK: fraction <= 1.0 — thumb is within track.".to_string()
|
||||
}),
|
||||
),
|
||||
)
|
||||
.child(
|
||||
div()
|
||||
.flex_1()
|
||||
.flex()
|
||||
.flex_row()
|
||||
.overflow_hidden()
|
||||
.border_1()
|
||||
.border_color(rgb(0xCCCCCC))
|
||||
.rounded_sm()
|
||||
.child(
|
||||
list(self.list_state.clone(), |index, _window, _cx| {
|
||||
let height = px(30. + (index % 5) as f32 * 10.);
|
||||
div()
|
||||
.h(height)
|
||||
.w_full()
|
||||
.flex()
|
||||
.items_center()
|
||||
.px_3()
|
||||
.border_b_1()
|
||||
.border_color(rgb(0xEEEEEE))
|
||||
.bg(if index % 2 == 0 {
|
||||
rgb(0xFAFAFA)
|
||||
} else {
|
||||
rgb(0xFFFFFF)
|
||||
})
|
||||
.text_sm()
|
||||
.child(format!("Item {index}"))
|
||||
.into_any()
|
||||
})
|
||||
.flex_1(),
|
||||
)
|
||||
// Scrollbar track
|
||||
.child(
|
||||
div()
|
||||
.w(px(SCROLLBAR_WIDTH))
|
||||
.h_full()
|
||||
.flex_shrink_0()
|
||||
.bg(rgb(0xE0E0E0))
|
||||
.relative()
|
||||
.child(
|
||||
// Thumb — position is unclamped to expose the bug
|
||||
div()
|
||||
.absolute()
|
||||
.top(thumb_top)
|
||||
.w_full()
|
||||
.h(thumb_height)
|
||||
.bg(if bug_detected {
|
||||
rgb(0xCC0000)
|
||||
} else {
|
||||
rgb(0x888888)
|
||||
})
|
||||
.rounded_sm(),
|
||||
),
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
fn run_example() {
|
||||
application().run(|cx: &mut App| {
|
||||
let bounds = Bounds::centered(None, size(px(400.), px(500.)), cx);
|
||||
cx.open_window(
|
||||
WindowOptions {
|
||||
focus: true,
|
||||
window_bounds: Some(WindowBounds::Windowed(bounds)),
|
||||
..Default::default()
|
||||
},
|
||||
|_, cx| cx.new(|_| BottomListDemo::new()),
|
||||
)
|
||||
.unwrap();
|
||||
cx.activate(true);
|
||||
});
|
||||
}
|
||||
|
||||
#[cfg(not(target_family = "wasm"))]
|
||||
fn main() {
|
||||
run_example();
|
||||
}
|
||||
|
||||
#[cfg(target_family = "wasm")]
|
||||
#[wasm_bindgen::prelude::wasm_bindgen(start)]
|
||||
pub fn start() {
|
||||
gpui_platform::web_init();
|
||||
run_example();
|
||||
}
|
||||
|
|
@ -493,18 +493,17 @@ impl ListState {
|
|||
/// This value remains constant while dragging to prevent the scrollbar from moving away unexpectedly.
|
||||
pub fn max_offset_for_scrollbar(&self) -> Point<Pixels> {
|
||||
let state = self.0.borrow();
|
||||
let bounds = state.last_layout_bounds.unwrap_or_default();
|
||||
|
||||
let height = state
|
||||
.scrollbar_drag_start_height
|
||||
.unwrap_or_else(|| state.items.summary().height);
|
||||
|
||||
point(Pixels::ZERO, Pixels::ZERO.max(height - bounds.size.height))
|
||||
point(Pixels::ZERO, state.max_scroll_offset())
|
||||
}
|
||||
|
||||
/// Returns the current scroll offset adjusted for the scrollbar
|
||||
pub fn scroll_px_offset_for_scrollbar(&self) -> Point<Pixels> {
|
||||
let state = &self.0.borrow();
|
||||
|
||||
if state.logical_scroll_top.is_none() && state.alignment == ListAlignment::Bottom {
|
||||
return Point::new(px(0.), -state.max_scroll_offset());
|
||||
}
|
||||
|
||||
let logical_scroll_top = state.logical_scroll_top();
|
||||
|
||||
let mut cursor = state.items.cursor::<ListItemSummary>(());
|
||||
|
|
@ -526,6 +525,14 @@ impl ListState {
|
|||
}
|
||||
|
||||
impl StateInner {
|
||||
fn max_scroll_offset(&self) -> Pixels {
|
||||
let bounds = self.last_layout_bounds.unwrap_or_default();
|
||||
let height = self
|
||||
.scrollbar_drag_start_height
|
||||
.unwrap_or_else(|| self.items.summary().height);
|
||||
(height - bounds.size.height).max(px(0.))
|
||||
}
|
||||
|
||||
fn visible_range(
|
||||
items: &SumTree<ListItem>,
|
||||
height: Pixels,
|
||||
|
|
@ -1449,4 +1456,46 @@ mod test {
|
|||
assert_eq!(offset.item_ix, 2);
|
||||
assert_eq!(offset.offset_in_item, px(20.));
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
fn test_bottom_aligned_scrollbar_offset_at_end(cx: &mut TestAppContext) {
|
||||
let cx = cx.add_empty_window();
|
||||
|
||||
const ITEMS: usize = 10;
|
||||
const ITEM_SIZE: f32 = 50.0;
|
||||
|
||||
let state = ListState::new(
|
||||
ITEMS,
|
||||
crate::ListAlignment::Bottom,
|
||||
px(ITEMS as f32 * ITEM_SIZE),
|
||||
);
|
||||
|
||||
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(ITEM_SIZE)).w_full().into_any()
|
||||
})
|
||||
.w_full()
|
||||
.h_full()
|
||||
}
|
||||
}
|
||||
|
||||
cx.draw(point(px(0.), px(0.)), size(px(100.), px(100.)), |_, cx| {
|
||||
cx.new(|_| TestView(state.clone())).into_any_element()
|
||||
});
|
||||
|
||||
// Bottom-aligned lists start pinned to the end: logical_scroll_top returns
|
||||
// item_ix == item_count, meaning no explicit scroll position has been set.
|
||||
assert_eq!(state.logical_scroll_top().item_ix, ITEMS);
|
||||
|
||||
let max_offset = state.max_offset_for_scrollbar();
|
||||
let scroll_offset = state.scroll_px_offset_for_scrollbar();
|
||||
|
||||
assert_eq!(
|
||||
-scroll_offset.y, max_offset.y,
|
||||
"scrollbar offset ({}) should equal max offset ({}) when list is pinned to bottom",
|
||||
-scroll_offset.y, max_offset.y,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue