From f04bc081eb2de5cb865f9533f62a193ca302b632 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sat, 26 Sep 2026 08:17:18 -0400 Subject: [PATCH] T24a round 2 fix - a dialog's body scroll box holds one inner column, so the body keeps its height and scrolls instead of shrinking to the capped card - test a_tall_dialog_body_keeps_its_height_and_scrolls: 2000px stays 2000px and a 300px wheel moves it 300px --- TASKS.md | 1 + src/overlays/dialog.rs | 5 +---- src/overlays/tests/mod.rs | 47 ++++++++++++++++++++++++++++++++++----- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/TASKS.md b/TASKS.md index aa52671..b2373ca 100644 --- a/TASKS.md +++ b/TASKS.md @@ -273,3 +273,4 @@ Per-component lines live in `tasks/`. Tags there: | T23d | 2 | PASS | | | T23e | 1 | PASS | | | T24a | 1 | FAIL | tall arguments pushed the approval dialog's title and buttons off screen; a long tool name in a narrow column pushed the time and chevron out of the card; the dark capture closed the card the light one opened | +| T24a | 2 | FAIL | a dialog body's children shrank to the capped column instead of scrolling: a 2000px body drew at 860px and a scroll moved nothing | diff --git a/src/overlays/dialog.rs b/src/overlays/dialog.rs index 0fd95d4..0dd378a 100644 --- a/src/overlays/dialog.rs +++ b/src/overlays/dialog.rs @@ -210,10 +210,7 @@ impl RenderOnce for Dialog { .id((self.id.clone(), "body")) .min_h_0() .overflow_y_scroll() - .flex() - .flex_col() - .gap_5() - .children(self.body) + .child(div().flex().flex_col().gap_5().children(self.body)) }); div() .id(self.id.clone()) diff --git a/src/overlays/tests/mod.rs b/src/overlays/tests/mod.rs index 9a02a8b..ce9b0a1 100644 --- a/src/overlays/tests/mod.rs +++ b/src/overlays/tests/mod.rs @@ -1,9 +1,10 @@ -use std::time::Duration; +use std::{cell::Cell, rc::Rc, time::Duration}; use gpui::{ - AppContext as _, Context, Entity, FocusHandle, InteractiveElement, IntoElement, KeyBinding, - KeyUpEvent, Keystroke, Modifiers, ParentElement, Render, Styled, TestAppContext, - VisualTestContext, Window, div, point, px, + AppContext as _, Bounds, Context, Entity, FocusHandle, InteractiveElement, IntoElement, + KeyBinding, KeyUpEvent, Keystroke, Modifiers, ParentElement, Pixels, Render, ScrollDelta, + ScrollWheelEvent, Styled, TestAppContext, TouchPhase, VisualTestContext, Window, canvas, div, + point, px, }; mod guides; @@ -32,6 +33,7 @@ struct Stage { field: Entity, open: Option, log: Vec, + tall: Rc>>, } impl Stage { @@ -120,7 +122,14 @@ impl Render for Stage { }) .into_any_element(), Open::Tall => Dialog::new("tall", "Tall", shut) - .child(div().h(px(2000.0))) + .child(div().relative().h(px(2000.0)).child({ + let tall = self.tall.clone(); + canvas(move |bounds, _, _| tall.set(bounds), |_, _, _, _| {}) + .absolute() + .top_0() + .left_0() + .size_full() + })) .action(move |_| { Button::new("tall-done", "Done").on_click(move |_, window, cx| done(window, cx)) }) @@ -177,6 +186,7 @@ fn stage(cx: &mut TestAppContext) -> (Entity, &mut VisualTestContext) { field: cx.new(|cx| TextInput::new(window, cx)), open: None, log: Vec::new(), + tall: Rc::new(Cell::new(Bounds::default())), }); settle(cx); (view, cx) @@ -389,3 +399,30 @@ fn a_tall_dialog_keeps_its_actions_in_the_window(cx: &mut TestAppContext) { "the button sits at the card's foot, inside the window" ); } + +#[gpui::test] +fn a_tall_dialog_body_keeps_its_height_and_scrolls(cx: &mut TestAppContext) { + let (view, cx) = stage(cx); + cx.update(|_, cx| Theme::update(cx, |theme| theme.reduced_motion = true)); + show(&view, Open::Tall, cx); + std::thread::sleep(Duration::from_millis(2)); + settle(cx); + let tall = view.read_with(cx, |stage, _| stage.tall.clone()); + let before = tall.get(); + assert_eq!(before.size.height, px(2000.0), "the body keeps its height"); + let middle = cx + .update(|window, _| window.viewport_size()) + .map(|side| side / 2.0); + cx.simulate_event(ScrollWheelEvent { + position: point(middle.width, middle.height), + delta: ScrollDelta::Pixels(point(px(0.0), px(-300.0))), + modifiers: Modifiers::none(), + touch_phase: TouchPhase::Moved, + }); + settle(cx); + assert_eq!( + tall.get().top(), + before.top() - px(300.0), + "a scroll moves the body" + ); +}