diff --git a/AGENTS.md b/AGENTS.md index 02a277e..4a27b74 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -137,6 +137,8 @@ Ely GPUI Component. A component library for GPUI, in light and dark. - 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. +- The focus color belongs to the focus ring. A chosen or shown item takes accent, as `forms::glyphs` and `lists::ListItem` do. +- A box that shows a host's picture takes the picture's shape from the host (`ratio`), so nothing is cropped and a point given as a share lands on the picture. ## Decisions diff --git a/TASKS.md b/TASKS.md index 6a91cc3..2fb740f 100644 --- a/TASKS.md +++ b/TASKS.md @@ -1,7 +1,7 @@ # Tasks Progress truth. Work runs top to bottom. No item is skipped. -Each `T` item ends with a codex review (`gpt-6-astra`, effort `max`, herdr split pane). Three rounds at most. +Each `T` item ends with a review in a herdr split pane, three rounds at most: codex (`gpt-6-astra`, effort `max`) through T24d round 2; from T24d round 3, after codex ran out of quota on 2026-09-26, a Claude session on Fable 5.1 at max effort, at the user's word. Per-component lines live in `tasks/`. Tags there: @@ -80,7 +80,7 @@ Per-component lines live in `tasks/`. Tags there: - [x] T24a Agent · Tool calls - [x] T24b Agent · Progress - [x] T24c Agent · Changes -- [ ] T24d Agent · Previews +- [x] T24d Agent · Previews - [ ] T24e Agent · Environment - [ ] T24f Agent · Control and cost - [ ] T25 Generative @@ -281,3 +281,4 @@ Per-component lines live in `tasks/`. Tags there: | 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 | | T24d | 2 | FAIL | Tab to a later thumbnail left the strip where it was, the focused frame and its ring clipped | +| T24d | 3 | FAIL | reviewed by Claude Fable 5.1 max after codex ran out of quota: the shown thumbnail wore the focus color, so focus and choice looked alike; frames were forced to 16:10 and cropped, and a pointer's share missed its target. Fixed after the cap: the shown frame takes accent, and BrowserPreview and ComputerUseViewer take the host's ratio (test a_screen_takes_its_pictures_shape); no fourth review | diff --git a/src/agent/previews.rs b/src/agent/previews.rs index bbfb4b1..a757903 100644 --- a/src/agent/previews.rs +++ b/src/agent/previews.rs @@ -16,17 +16,22 @@ use crate::{ typography::Ellipsis, }; -/// Width over height of a frame from the host. +/// Width over height of a frame from the host, unless it says. const FRAME: f32 = 16.0 / 10.0; +fn checked(ratio: f32) -> f32 { + assert!(ratio.is_finite() && ratio > 0.0, "a frame ratio of {ratio}"); + ratio +} + /// A box that keeps a frame's shape at the width it is given. -fn framed(cx: &App) -> Div { +fn framed(ratio: f32, cx: &App) -> Div { let mut frame = div() .relative() .w_full() .overflow_hidden() .bg(cx.theme().colors.sunken); - frame.style().aspect_ratio = Some(FRAME); + frame.style().aspect_ratio = Some(ratio); frame } @@ -82,6 +87,7 @@ pub struct BrowserPreview { url: SharedString, frames: Vec, shown: usize, + ratio: f32, loading: bool, on_show: Option, } @@ -101,11 +107,18 @@ impl BrowserPreview { url: url.into(), frames, shown, + ratio: FRAME, loading: false, on_show: None, } } + /// The frames' width over their height, 16:10 unless set, so the boxes take the pictures' shape. + pub fn ratio(mut self, width_over_height: f32) -> Self { + self.ratio = checked(width_over_height); + self + } + pub fn loading(mut self, loading: bool) -> Self { self.loading = loading; self @@ -174,7 +187,7 @@ impl RenderOnce for BrowserPreview { .rounded(theme.radius(Radius::Sm)) .border_1() .border_color(if ix == shown { - colors.focus + colors.accent } else { colors.border }) @@ -183,7 +196,7 @@ impl RenderOnce for BrowserPreview { .rounded(theme.radius(Radius::Sm)) .size_full(), ); - tile.style().aspect_ratio = Some(FRAME); + tile.style().aspect_ratio = Some(self.ratio); tile.when_some(show, |tile, show| { tile.track_focus(&handles[ix]) .focus_ring(cx) @@ -204,7 +217,7 @@ impl RenderOnce for BrowserPreview { card(cx) .child(address_bar(&self.id, self.url, self.loading, None, cx)) .child( - framed(cx) + framed(self.ratio, cx) .when(strip.is_none(), |frame| { frame.rounded_b(theme.radius(Radius::Lg)) }) @@ -228,6 +241,7 @@ impl RenderOnce for BrowserPreview { pub struct ComputerUseViewer { id: ElementId, frame: SharedString, + ratio: f32, pointer: Option>, action: Option, } @@ -238,11 +252,18 @@ impl ComputerUseViewer { Self { id: id.into(), frame: frame.into(), + ratio: FRAME, pointer: None, action: None, } } + /// The frame's width over its height, 16:10 unless set, so the box takes the picture's shape. + pub fn ratio(mut self, width_over_height: f32) -> Self { + self.ratio = checked(width_over_height); + self + } + /// Where the agent points, each side a share of the frame from its top left. pub fn pointer(mut self, at: Point) -> Self { assert!( @@ -302,7 +323,7 @@ impl RenderOnce for ComputerUseViewer { }); card(cx) .child( - framed(cx) + framed(self.ratio, cx) .rounded_t(theme.radius(Radius::Lg)) .when(self.action.is_none(), |frame| { frame.rounded_b(theme.radius(Radius::Lg)) diff --git a/src/agent/tests/previews.rs b/src/agent/tests/previews.rs index 81a2761..aa22321 100644 --- a/src/agent/tests/previews.rs +++ b/src/agent/tests/previews.rs @@ -244,3 +244,47 @@ fn tab_brings_a_hidden_frame_into_view(cx: &mut TestAppContext) { "the eighth frame sits in view" ); } + +/// A 4:3 screen in a 480px column, and its height as drawn. +struct Screen { + height: Rc>, +} + +impl Render for Screen { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + let height = self.height.clone(); + div() + .relative() + .w(px(480.0)) + .child( + ComputerUseViewer::new("screen", "a.png") + .ratio(4.0 / 3.0) + .pointer(point(0.5, 0.95)), + ) + .child( + canvas( + move |bounds, _, _| height.set(bounds.size.height), + |_, _, _, _| {}, + ) + .absolute() + .top_0() + .left_0() + .size_full(), + ) + } +} + +#[gpui::test] +fn a_screen_takes_its_pictures_shape(cx: &mut TestAppContext) { + setup(cx); + let height = Rc::new(Cell::new(Pixels::ZERO)); + let seen = height.clone(); + let (_, cx) = cx.add_window_view(|_, _| Screen { height: seen }); + settle(cx); + let expected = px(478.0 * 3.0 / 4.0 + 2.0); + assert!( + (height.get() - expected).abs() < px(1.0), + "478px inside the border at 4:3, plus the border: {:?} vs {expected:?}", + height.get() + ); +}