Closes #51417 I noticed this bug in the settings menu where when I opened the settings menu, I could not scroll down through all the available options, eg. on the initial page I wasn't able to scroll down to privacy. When I saw that no one else had reported this issue, I figured it may be due to my setup, and it turns out that using Aerospace, the window manager I use, was what made this bug visible to me. Because aerospace resizes the window right after it launches, the originally computed heights for the list are incorrect, meaning the scroll bar is the wrong size as well. in the relevant code there was a comment that says "If the width of the list has changed, invalidate all cached item heights" which wasn't incorrect per-se, but it just invalidated them without triggering any re-computation, causing incorrect scroll bars. My intuition is that window resizes/events that change the width of the list bounds are fairly rare, so there shouldn't be a large performance hit from the change. Also implemented a test that directly showcases the behavior, if you run the test without the change it fails, as the max_offset_for_scrollbar will be wrong. Videos: Before https://github.com/user-attachments/assets/2b680222-7071-4098-863f-519361f0756a After: https://github.com/user-attachments/assets/1222a299-23d7-4007-8e88-55d2daccce64 [x] Tests [x] Video of behavior Release Notes: - gpui: fixed list height re-computation when the list width changes.
Unified · Split
| @@ -1103,6 +1103,7 @@ impl Element for List { · 1102 unmodified lines | |||
| 1103 | ); | 1103 | ); |
| 1104 | 1104 | ||
| 1105 | state.items = new_items; | 1105 | state.items = new_items; |
| 1106 | state.measuring_behavior.reset(); | ||
| 1106 | } | 1107 | } |
| 1107 | 1108 | ||
| 1108 | let padding = style | 1109 | let padding = style |
| @@ -1348,6 +1349,41 @@ mod test { · 239 unmodified lines | |||
| 1348 | assert_eq!(offset.offset_in_item, px(0.)); | 1349 | assert_eq!(offset.offset_in_item, px(0.)); |
| 1349 | } | 1350 | } |
| 1350 | 1351 | ||
| 1352 | #[gpui::test] | ||
| 1353 | fn test_measure_all_after_width_change(cx: &mut TestAppContext) { | ||
| 1354 | let cx = cx.add_empty_window(); | ||
| 1355 | |||
| 1356 | let state = ListState::new(10, crate::ListAlignment::Top, px(0.)).measure_all(); | ||
| 1357 | |||
| 1358 | struct TestView(ListState); | ||
| 1359 | impl Render for TestView { | ||
| 1360 | fn render(&mut self, _: &mut Window, _: &mut Context<Self>) -> impl IntoElement { | ||
| 1361 | list(self.0.clone(), |_, _, _| { | ||
| 1362 | div().h(px(50.)).w_full().into_any() | ||
| 1363 | }) | ||
| 1364 | .w_full() | ||
| 1365 | .h_full() | ||
| 1366 | } | ||
| 1367 | } | ||
| 1368 | |||
| 1369 | let view = cx.update(|_, cx| cx.new(|_| TestView(state.clone()))); | ||
| 1370 | |||
| 1371 | // First draw at width 100: all 10 items measured (total 500px). | ||
| 1372 | // Viewport is 200px, so max scroll offset should be 300px. | ||
| 1373 | cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, _| { | ||
| 1374 | view.clone().into_any_element() | ||
| 1375 | }); | ||
| 1376 | assert_eq!(state.max_offset_for_scrollbar().y, px(300.)); | ||
| 1377 | |||
| 1378 | // Second draw at a different width: items get invalidated. | ||
| 1379 | // Without the fix, max_offset would drop because unmeasured items | ||
| 1380 | // contribute 0 height. | ||
| 1381 | cx.draw(point(px(0.), px(0.)), size(px(200.), px(200.)), |_, _| { | ||
| 1382 | view.into_any_element() | ||
| 1383 | }); | ||
| 1384 | assert_eq!(state.max_offset_for_scrollbar().y, px(300.)); | ||
| 1385 | } | ||
| 1386 | |||
| 1351 | #[gpui::test] | 1387 | #[gpui::test] |
| 1352 | fn test_remeasure(cx: &mut TestAppContext) { | 1388 | fn test_remeasure(cx: &mut TestAppContext) { |
| 1353 | let cx = cx.add_empty_window(); | 1389 | let cx = cx.add_empty_window(); |