From e99a86bed55ceda628903ba9e16e64f353e82aa0 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 09:07:14 -0400 Subject: [PATCH] T24d round 1 fix - BrowserPreview's frame strip scrolls sideways, so every frame stays in reach in a narrow card - ArtifactPanel's header wraps: the title keeps label_width, the controls ride in one group that wraps inside the box - tests: a_narrow_strip_scrolls_to_its_last_frame, a_narrow_panel_wraps_its_header - AGENTS: everything works at 280px --- AGENTS.md | 1 + TASKS.md | 1 + src/agent/artifact.rs | 74 +++++++++++++++-------- src/agent/previews.rs | 2 + src/agent/tests/previews.rs | 117 +++++++++++++++++++++++++++++++++++- 5 files changed, 166 insertions(+), 29 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6d5f4e4..cf30d73 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -136,6 +136,7 @@ Ely GPUI Component. A component library for GPUI, in light and dark. - A search step, a tool call or an agent's step says where it stands with `chat::StepState` and draws it with `chat::step_mark`. - A flex line whose free space goes to an auto margin loses its gaps in taffy 0.9. A row that parts left and right groups its sides and uses `justify_between`; `ml_auto` is safe only beside a `flex_1` item that leaves no free space. - A header that holds a name beside counts and actions wraps: `flex_wrap`, the name `flex_1` with `min_w(label_width)`, the rest one `flex_none` group that drops below. +- Every header, row and strip works at 280px: a name keeps its minimum width, the rest wraps below or scrolls, and nothing draws past its box. ## Decisions diff --git a/TASKS.md b/TASKS.md index b1433fc..17c10a3 100644 --- a/TASKS.md +++ b/TASKS.md @@ -279,3 +279,4 @@ Per-component lines live in `tasks/`. Tags there: | T24b | 2 | PASS | | | T24c | 1 | FAIL | at 280px the path vanished and Accept all drew outside the box; the diff's fold rows took presses but no Tab | | T24c | 2 | PASS | | +| T24d | 1 | FAIL | at 280px eight thumbnails ran past the browser card; the artifact panel lost its title and its controls ran out of the box | diff --git a/src/agent/artifact.rs b/src/agent/artifact.rs index c801f6c..55a9ff7 100644 --- a/src/agent/artifact.rs +++ b/src/agent/artifact.rs @@ -2,7 +2,7 @@ use std::rc::Rc; use gpui::{ AnyElement, App, ElementId, FontWeight, InteractiveElement, IntoElement, ParentElement, - RenderOnce, SharedString, StatefulInteractiveElement, Styled, Window, div, + RenderOnce, SharedString, StatefulInteractiveElement, Styled, Window, div, prelude::*, }; use crate::{ @@ -114,6 +114,25 @@ impl RenderOnce for ArtifactPanel { }) }) }); + let version = self.version.map(|(current, total, pick)| { + BranchNavigator::new( + (self.id.clone(), "versions"), + current, + total, + move |to, window, cx| pick(to, window, cx), + ) + .into_any_element() + }); + let copy = (self.on_copy) + .map(|copy| action("copy", IconName::Copy, "Copy", copy).into_any_element()); + let save = (self.on_download).map(|save| { + action("download", IconName::Download, "Download", save).into_any_element() + }); + let controls: Vec = + [version, view.map(IntoElement::into_any_element), copy, save] + .into_iter() + .flatten() + .collect(); let body = if showing_source { self.source } else { @@ -131,42 +150,45 @@ impl RenderOnce for ArtifactPanel { div() .flex_none() .flex() + .flex_wrap() .items_center() - .gap_2() + .gap_x_2() + .gap_y_1() .px_3() .py_2() .border_b_1() .border_color(colors.border) .text_size(theme.text_size(TextSize::Sm)) - .child( - Icon::new(self.icon) - .size(IconSize::Sm) - .color(colors.fg_muted), - ) .child( div() .flex_1() - .min_w_0() - .font_weight(FontWeight::MEDIUM) - .child(Ellipsis::new(self.title)), + .min_w(theme.label_width()) + .flex() + .items_center() + .gap_2() + .child( + Icon::new(self.icon) + .size(IconSize::Sm) + .color(colors.fg_muted), + ) + .child( + div() + .flex_1() + .min_w_0() + .font_weight(FontWeight::MEDIUM) + .child(Ellipsis::new(self.title)), + ), ) - .children(self.version.map(|(current, total, pick)| { - BranchNavigator::new( - (self.id.clone(), "versions"), - current, - total, - move |to, window, cx| pick(to, window, cx), + .when(!controls.is_empty(), |header| { + header.child( + div() + .flex() + .flex_wrap() + .items_center() + .gap_1() + .children(controls), ) - })) - .children(view) - .children( - self.on_copy - .map(|copy| action("copy", IconName::Copy, "Copy", copy)), - ) - .children( - self.on_download - .map(|save| action("download", IconName::Download, "Download", save)), - ), + }), ) .child( div() diff --git a/src/agent/previews.rs b/src/agent/previews.rs index c0803ba..71dd75b 100644 --- a/src/agent/previews.rs +++ b/src/agent/previews.rs @@ -126,6 +126,8 @@ impl RenderOnce for BrowserPreview { let shown = self.shown; let strip = (self.frames.len() > 1).then(|| { div() + .id((self.id.clone(), "strip")) + .overflow_x_scroll() .flex() .gap_1p5() .p_2() diff --git a/src/agent/tests/previews.rs b/src/agent/tests/previews.rs index 7fa8e71..f5423e6 100644 --- a/src/agent/tests/previews.rs +++ b/src/agent/tests/previews.rs @@ -1,8 +1,8 @@ use std::{cell::Cell, rc::Rc}; use gpui::{ - Context, IntoElement, KeyBinding, ParentElement, Render, Styled, TestAppContext, Window, - canvas, div, point, px, + Context, IntoElement, KeyBinding, Modifiers, ParentElement, Pixels, Render, ScrollDelta, + ScrollWheelEvent, Styled, TestAppContext, TouchPhase, Window, canvas, div, point, px, }; use super::{press, settle}; @@ -10,7 +10,7 @@ use crate::{ agent::{ArtifactPanel, BrowserPreview, ComputerUseViewer}, forms, primitives::{FocusNext, IconName}, - theme::Theme, + theme::{ActiveTheme, AvatarSize, Theme}, }; fn setup(cx: &mut TestAppContext) { @@ -101,3 +101,114 @@ fn the_source_shows_when_asked(cx: &mut TestAppContext) { press("enter", cx); assert!(drew.get(), "the source shows once asked"); } + +/// A narrow browser with eight frames, its height as drawn, and the frames asked for. +struct Strip { + height: Rc>, + asked: Vec, +} + +impl Render for Strip { + fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { + let (view, height) = (cx.entity(), self.height.clone()); + let frames = (0..8).map(|ix| format!("{ix}.png")); + div() + .relative() + .w(px(280.0)) + .child( + BrowserPreview::new("strip", "localhost", frames, 0) + .on_show(move |ix, _, cx| view.update(cx, |strip, _| strip.asked.push(ix))), + ) + .child( + canvas( + move |bounds, _, _| height.set(bounds.size.height), + |_, _, _, _| {}, + ) + .absolute() + .top_0() + .left_0() + .size_full(), + ) + } +} + +#[gpui::test] +fn a_narrow_strip_scrolls_to_its_last_frame(cx: &mut TestAppContext) { + setup(cx); + let height = Rc::new(Cell::new(Pixels::ZERO)); + let seen = height.clone(); + let (view, cx) = cx.add_window_view(|_, _| Strip { + height: seen, + asked: Vec::new(), + }); + settle(cx); + let thumb = cx.update(|window, cx| { + cx.theme() + .avatar_size(AvatarSize::Lg) + .to_pixels(window.rem_size()) + }); + let row = height.get() - px(9.0) - thumb / 2.0; + cx.simulate_event(ScrollWheelEvent { + position: point(px(140.0), row), + delta: ScrollDelta::Pixels(point(px(-2000.0), px(0.0))), + modifiers: Modifiers::none(), + touch_phase: TouchPhase::Moved, + }); + settle(cx); + let last = point(px(280.0 - 24.0), row); + cx.simulate_mouse_move(last, None, Modifiers::none()); + cx.simulate_click(last, Modifiers::none()); + settle(cx); + assert_eq!(view.read_with(cx, |strip, _| strip.asked.clone()), [7]); +} + +/// A panel with every control at `width`, noting where its body starts. +struct Crowded { + width: Pixels, + top: Rc>, +} + +impl Render for Crowded { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + let top = self.top.clone(); + div().w(self.width).h(px(320.0)).child( + ArtifactPanel::new("crowded", "Notes on lift", IconName::FileText) + .version(1, 3, |_, _, _| {}) + .preview( + div().relative().h(px(40.0)).child( + canvas(move |bounds, _, _| top.set(bounds.top()), |_, _, _, _| {}) + .absolute() + .top_0() + .left_0() + .size_full(), + ), + ) + .source(div().h(px(40.0))) + .on_copy(|_, _| {}) + .on_download(|_, _| {}), + ) + } +} + +#[gpui::test] +fn a_narrow_panel_wraps_its_header(cx: &mut TestAppContext) { + setup(cx); + let top = Rc::new(Cell::new(Pixels::ZERO)); + let seen = top.clone(); + let (view, cx) = cx.add_window_view(|_, _| Crowded { + width: px(640.0), + top: seen, + }); + settle(cx); + let wide = top.get(); + view.update(cx, |crowded, cx| { + crowded.width = px(280.0); + cx.notify(); + }); + settle(cx); + assert!( + top.get() > wide, + "the header wraps below at 280px: {:?} vs {wide:?}", + top.get() + ); +}