From a47d0b058b3833b26a7cd897d7e853b7b72a757b 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:09:46 -0400 Subject: [PATCH] T24a round 1 fix - overlays::Dialog caps its card at the window, a title bar's height clear at each end, and scrolls its body while heading and actions stay; the fullscreen body scrolls too; test a_tall_dialog_keeps_its_actions_in_the_window - ToolCallCard's tool name gives way with an Ellipsis, so time and chevron stay in a narrow column; the gallery shows one at 280px - the Agent script shuts the card it opened, so each theme's shot opens it - AGENTS: dialogs fit the window --- AGENTS.md | 2 +- TASKS.md | 1 + examples/gallery/pages/agent/mod.rs | 3 ++ examples/gallery/pages/agent/tools.rs | 8 +++++ src/agent/tools.rs | 4 +-- src/overlays/dialog.rs | 35 +++++++++++++++++++--- src/overlays/tests/mod.rs | 42 +++++++++++++++++++++++++-- 7 files changed, 86 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 4ea3be0..0ae31d2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -69,7 +69,7 @@ Ely GPUI Component. A component library for GPUI, in light and dark. - gpui focuses the nearest focusable ancestor on mouse down, and apps wrap everything in a `FocusScope`. A press that must leave focus alone calls `prevent_default`, as buttons and context regions do. - gpui draws a window's first frame inside `open_window`. State the first frame reads, such as `cx.set_menus`, is set before it opens. - gpui clicks a focused element when Enter or Space is released. An overlay that hands focus back picks on release too; a pick on press returns focus first, and the release clicks the opener again. -- A dialog goes through `overlays::Dialog`: scrim, focus trap, Escape and focus return live there. AlertDialog, ConfirmDialog and PromptDialog build on it; a fullscreen dialog fills the window below its title bar. +- A dialog goes through `overlays::Dialog`: scrim, focus trap, Escape and focus return live there. AlertDialog, ConfirmDialog and PromptDialog build on it; a fullscreen dialog fills the window below its title bar. A dialog's card fits the window, a title bar's height clear at each end; its body scrolls and its actions stay. - `primitives::Severity` names info, success, warning and danger with their colors and icons, for alerts and feedback alike. - Rows that move are keyed. `motion::Flip` places them absolutely from last frame's heights, so no frame shows a row at its new place before it glides there, and drives the glide from elapsed time: an animation id that changes per move would rebuild the row's keyed state. `Reorder` holds a row by key, drags with an empty ghost and moves the held row itself. - A marker that slides between items (segment thumb, tab line) measures them with `motion::slide` and eases with `glide`. diff --git a/TASKS.md b/TASKS.md index f403362..aa52671 100644 --- a/TASKS.md +++ b/TASKS.md @@ -272,3 +272,4 @@ Per-component lines live in `tasks/`. Tags there: | T23d | 1 | FAIL | the list grouped days in a silent UTC fallback when the system zone was unknown; Combobox and MultiSelect opened a long list at its top, away from the choice | | 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 | diff --git a/examples/gallery/pages/agent/mod.rs b/examples/gallery/pages/agent/mod.rs index e3ef183..11d392d 100644 --- a/examples/gallery/pages/agent/mod.rs +++ b/examples/gallery/pages/agent/mod.rs @@ -24,6 +24,9 @@ const SCRIPT: &[Step] = &[ Step::Shot("approval"), Step::Key("escape"), Step::Wait(200), + Step::DownAt("agent-call", 60.0, 18.0), + Step::UpAt("agent-call", 60.0, 18.0), + Step::Wait(200), ]; fn render(window: &mut Window, cx: &mut App) -> AnyElement { diff --git a/examples/gallery/pages/agent/tools.rs b/examples/gallery/pages/agent/tools.rs index 0249fd2..8b983af 100644 --- a/examples/gallery/pages/agent/tools.rs +++ b/examples/gallery/pages/agent/tools.rs @@ -59,6 +59,14 @@ pub fn calls(_: &mut Window, cx: &mut App) -> impl IntoElement + use<> { .summary("src/theme/lift.rs") .arguments(WRITE), ) + .child( + div().w(px(280.)).child( + ToolCallCard::new("agent-narrow", "mcp__filesystem__read_multiple_files", StepState::Done) + .summary("3 files") + .took(Duration::from_millis(1_100)) + .arguments(READ), + ), + ) .child( ToolCallGroup::new("agent-group", "Read 3 files") .child(ToolCallCard::new("agent-group-1", "read_file", StepState::Done).summary("src/theme/mod.rs").took(Duration::from_millis(120))) diff --git a/src/agent/tools.rs b/src/agent/tools.rs index 97ca5c6..c568115 100644 --- a/src/agent/tools.rs +++ b/src/agent/tools.rs @@ -123,10 +123,10 @@ impl RenderOnce for ToolCallCard { .child(step_mark((self.id.clone(), "mark").into(), self.status, cx)) .child( div() - .flex_none() + .min_w_0() .font_family(theme.mono_family.clone()) .text_color(colors.fg) - .child(self.name), + .child(Ellipsis::new(self.name)), ) .child( div() diff --git a/src/overlays/dialog.rs b/src/overlays/dialog.rs index 5273fb3..0fd95d4 100644 --- a/src/overlays/dialog.rs +++ b/src/overlays/dialog.rs @@ -120,6 +120,7 @@ impl RenderOnce for Dialog { let theme = cx.theme(); let colors = &theme.colors; let heading = div() + .flex_none() .flex() .flex_col() .gap_1() @@ -143,8 +144,14 @@ impl RenderOnce for Dialog { .into_iter() .map(|build| build(close.clone())) .collect(); - let actions = - (!buttons.is_empty()).then(|| div().flex().justify_end().gap_2().children(buttons)); + let actions = (!buttons.is_empty()).then(|| { + div() + .flex_none() + .flex() + .justify_end() + .gap_2() + .children(buttons) + }); let enter = self.enter.map(|build| { let field = self .first @@ -179,7 +186,15 @@ impl RenderOnce for Dialog { .on_click(move |_, window, cx| button(window, cx)), ), ) - .child(div().flex_1().min_h_0().p_6().children(self.body)) + .child( + div() + .id((self.id.clone(), "body")) + .flex_1() + .min_h_0() + .overflow_y_scroll() + .p_6() + .children(self.body), + ) .children(actions.map(|row| { div() .px_6() @@ -189,12 +204,24 @@ impl RenderOnce for Dialog { .child(row) })) } else { + let margin = theme.titlebar_height().to_pixels(window.rem_size()); + let body = (!self.body.is_empty()).then(|| { + div() + .id((self.id.clone(), "body")) + .min_h_0() + .overflow_y_scroll() + .flex() + .flex_col() + .gap_5() + .children(self.body) + }); div() .id(self.id.clone()) .flex() .flex_col() .gap_5() .w(theme.dialog_width()) + .max_h(window.viewport_size().height - margin * 2.0) .p_6() .rounded(theme.radius(Radius::Xl)) .bg(colors.overlay) @@ -202,7 +229,7 @@ impl RenderOnce for Dialog { .border_color(colors.border) .shadow(theme.elevation(Elevation::Modal)) .child(heading) - .children(self.body) + .children(body) .children(actions) }; let rise = if fullscreen { diff --git a/src/overlays/tests/mod.rs b/src/overlays/tests/mod.rs index d2ddd6f..9a02a8b 100644 --- a/src/overlays/tests/mod.rs +++ b/src/overlays/tests/mod.rs @@ -13,7 +13,7 @@ use crate::{ buttons::Button, forms::TextInput, primitives::{FocusNext, FocusScope, Severity}, - theme::Theme, + theme::{ActiveTheme, ControlSize, Theme}, }; #[derive(Clone, Copy, PartialEq)] @@ -22,6 +22,7 @@ enum Open { Alert, Confirm, Prompt, + Tall, } /// A popover, a hover card and one dialog at a time, recording what they do. @@ -90,7 +91,11 @@ impl Render for Stage { Button::new("follow", "Follow").on_click(move |_, window, cx| follow(window, cx)) }, ); - let (shut, confirmed) = (self.shut(cx), self.note(cx, "confirmed")); + let (shut, confirmed, done) = ( + self.shut(cx), + self.note(cx, "confirmed"), + self.note(cx, "done"), + ); let sent = cx.entity(); let overlay = self.open.map(|open| match open { Open::Plain => Dialog::new("plain", "Plain", shut) @@ -114,6 +119,12 @@ impl Render for Stage { sent.update(cx, |stage, _| stage.log.push(text)); }) .into_any_element(), + Open::Tall => Dialog::new("tall", "Tall", shut) + .child(div().h(px(2000.0))) + .action(move |_| { + Button::new("tall-done", "Done").on_click(move |_, window, cx| done(window, cx)) + }) + .into_any_element(), }); FocusScope::new(&self.root) .size_full() @@ -351,3 +362,30 @@ fn enter_on_a_prompt_button_runs_that_button_alone(cx: &mut TestAppContext) { press("enter", cx); assert_eq!(log(&view, cx), ["sent notes"]); } + +#[gpui::test] +fn a_tall_dialog_keeps_its_actions_in_the_window(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 (size, margin, width, button) = cx.update(|window, cx| { + let (theme, rem) = (cx.theme(), window.rem_size()); + ( + window.viewport_size(), + theme.titlebar_height().to_pixels(rem), + theme.dialog_width().to_pixels(rem), + theme.control_height(ControlSize::Md).to_pixels(rem), + ) + }); + let padding = px(24.0); + let x = size.width / 2.0 + width / 2.0 - padding - px(12.0); + let y = size.height - margin - padding - button / 2.0; + click(f32::from(x), f32::from(y), cx); + assert_eq!( + log(&view, cx), + ["done"], + "the button sits at the card's foot, inside the window" + ); +}