diff --git a/AGENTS.md b/AGENTS.md index 0ae31d2..6d5f4e4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -134,6 +134,8 @@ Ely GPUI Component. A component library for GPUI, in light and dark. - The system time zone comes from `typography::format::system_zone`, which fails loud; jiff's `TimeZone::system()` falls back to UTC in silence. - A list opens with its cursor on the current choice: Select on the chosen row, Combobox on its value, MultiSelect on the first one ticked. - 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. ## Decisions diff --git a/TASKS.md b/TASKS.md index d184536..ad6f4d6 100644 --- a/TASKS.md +++ b/TASKS.md @@ -277,3 +277,4 @@ Per-component lines live in `tasks/`. Tags there: | T24a | 3 | PASS | | | T24b | 1 | FAIL | a task of one step crashed at render: its bar asked for one segment, and a segmented bar needs two | | 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 | diff --git a/examples/gallery/pages/agent/changes.rs b/examples/gallery/pages/agent/changes.rs index 605574a..ee56d9b 100644 --- a/examples/gallery/pages/agent/changes.rs +++ b/examples/gallery/pages/agent/changes.rs @@ -65,22 +65,31 @@ fn decided(mut changes: Vec, decision: Decision) -> Vec pub fn changes_section(window: &mut Window, cx: &mut App) -> impl IntoElement + use<> { let files = keep("agent-changes", changes, window, cx); let now = files.read(cx).clone(); + let narrow = files.clone(); section( "FileChangeCard / MultiFileDiff Review / CommandExecutionCard", - "What an agent would change, file by file: how much moves, the diff a press away, and a verdict each or for all. A command it ran is terminal::CommandBlock.", + "What an agent would change, file by file: how much moves, the diff a press away, and a verdict each or for all; in a side panel the counts and verdicts wrap below. A command it ran is terminal::CommandBlock.", cx, ) .child( probe( "agent-changes", div().w(px(640.)).child( - MultiFileDiffReview::new("agent-review", now).on_decide(move |decision, _, cx| { + MultiFileDiffReview::new("agent-review", now.clone()).on_decide(move |decision, _, cx| { let next = decided(files.read(cx).clone(), decision); set(&files, next, cx) }), ), ), ) + .child( + div().w(px(280.)).child( + MultiFileDiffReview::new("agent-review-narrow", now).on_decide(move |decision, _, cx| { + let next = decided(narrow.read(cx).clone(), decision); + set(&narrow, next, cx) + }), + ), + ) .child( div().w(px(640.)).child(CommandBlock::new( "agent-command", diff --git a/src/agent/changes.rs b/src/agent/changes.rs index 30c45fe..4acfe06 100644 --- a/src/agent/changes.rs +++ b/src/agent/changes.rs @@ -135,6 +135,7 @@ impl RenderOnce for FileChangeCard { div() .id((self.id.clone(), "header")) .flex() + .flex_wrap() .items_center() .gap_2() .px_2() @@ -162,17 +163,20 @@ impl RenderOnce for FileChangeCard { .child( div() .flex_1() - .min_w_0() + .min_w(theme.label_width()) .font_family(theme.mono_family.clone()) .child(Ellipsis::new(self.change.path.clone())), ) - .child(div().flex_none().child(DiffStat::new(added, removed))) - .child(div().flex_none().child(verdict( - &self.id, - self.change.state, - decide, - cx, - ))), + .child( + div() + .flex_none() + .ml_auto() + .flex() + .items_center() + .gap_2() + .child(DiffStat::new(added, removed)) + .child(verdict(&self.id, self.change.state, decide, cx)), + ), ) .when(open, |card| { card.child( @@ -248,6 +252,7 @@ impl RenderOnce for MultiFileDiffReview { let all = self.on_decide.clone().filter(|_| open > 0).map(|decide| { let (reject, accept) = (decide.clone(), decide); div() + .flex_none() .flex() .items_center() .gap_1() @@ -279,18 +284,29 @@ impl RenderOnce for MultiFileDiffReview { .child( div() .flex() + .flex_wrap() .items_center() - .gap_3() + .justify_between() + .gap_y_2() .child( div() - .flex_none() - .font_weight(FontWeight::MEDIUM) - .child(format!("{count} files changed")), + .flex() + .flex_wrap() + .items_center() + .gap_x_3() + .gap_y_1() + .child( + div() + .font_weight(FontWeight::MEDIUM) + .child(format!("{count} files changed")), + ) + .child(DiffStat::new(added, removed)) + .child( + div() + .text_color(colors.fg_subtle) + .child(format!("{} of {count} reviewed", count - open)), + ), ) - .child(div().flex_none().child(DiffStat::new(added, removed))) - .child(div().flex_1().min_w_0().text_color(colors.fg_subtle).child( - Ellipsis::new(format!("{} of {count} reviewed", count - open)), - )) .children(all), ) .children(self.changes.into_iter().enumerate().map(|(ix, change)| { diff --git a/src/agent/tests/changes.rs b/src/agent/tests/changes.rs index 92d9fa9..ea079ff 100644 --- a/src/agent/tests/changes.rs +++ b/src/agent/tests/changes.rs @@ -31,7 +31,7 @@ fn setup(cx: &mut TestAppContext) { }); } -/// One change card in a 400px column, its verdicts and the column's height. +/// One change card in a 640px column, its verdicts and the column's height. struct Card { verdicts: Vec, height: Rc>, @@ -42,7 +42,7 @@ impl Render for Card { let (view, height) = (cx.entity(), self.height.clone()); div() .relative() - .w(px(400.0)) + .w(px(640.0)) .child( FileChangeCard::new("card", change("src/theme/lift.rs")).on_decide( move |accepted, _, cx| view.update(cx, |card, _| card.verdicts.push(accepted)), @@ -72,7 +72,7 @@ fn a_press_on_accept_decides_and_leaves_the_card_shut(cx: &mut TestAppContext) { }); settle(cx); let shut = height.get(); - let accept = point(px(400.0 - 16.0), shut / 2.0); + let accept = point(px(640.0 - 16.0), shut / 2.0); cx.simulate_mouse_move(accept, None, Modifiers::none()); cx.simulate_click(accept, Modifiers::none()); settle(cx); @@ -135,3 +135,58 @@ fn a_cards_verdict_names_its_file(cx: &mut TestAppContext) { [Decision::Accept(1)] ); } + +/// A review at `width`, and its height as drawn. +struct Narrow { + width: Pixels, + height: Rc>, +} + +impl Render for Narrow { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + let height = self.height.clone(); + div() + .relative() + .w(self.width) + .child( + MultiFileDiffReview::new( + "narrow", + ["src/theme/lift.rs", "src/theme/tokens.rs"].map(change), + ) + .on_decide(|_, _, _| {}), + ) + .child( + canvas( + move |bounds, _, _| height.set(bounds.size.height), + |_, _, _, _| {}, + ) + .absolute() + .top_0() + .left_0() + .size_full(), + ) + } +} + +#[gpui::test] +fn a_narrow_review_wraps_its_rows_inside_its_box(cx: &mut TestAppContext) { + setup(cx); + let height = Rc::new(Cell::new(Pixels::ZERO)); + let seen = height.clone(); + let (view, cx) = cx.add_window_view(|_, _| Narrow { + width: px(640.0), + height: seen, + }); + settle(cx); + let wide = height.get(); + view.update(cx, |narrow, cx| { + narrow.width = px(280.0); + cx.notify(); + }); + settle(cx); + assert!( + height.get() > wide, + "at 280px the counts and verdicts wrap below: {:?} vs {wide:?}", + height.get() + ); +} diff --git a/src/git/mod.rs b/src/git/mod.rs index aa41610..2b6034a 100644 --- a/src/git/mod.rs +++ b/src/git/mod.rs @@ -10,6 +10,8 @@ mod merging; mod refs; mod review; mod tags; +#[cfg(all(test, feature = "test-support"))] +mod tests; mod viewer; pub use badges::{DiffStat, GitStatusBadge}; diff --git a/src/git/tests.rs b/src/git/tests.rs new file mode 100644 index 0000000..0305259 --- /dev/null +++ b/src/git/tests.rs @@ -0,0 +1,52 @@ +use gpui::{ + Context, IntoElement, KeyBinding, KeyUpEvent, Keystroke, ParentElement, Render, Styled, + TestAppContext, VisualTestContext, Window, div, px, +}; + +use super::DiffViewer; +use crate::{forms, primitives::FocusNext, theme::Theme}; + +/// A diff whose first stretch folds, and the folds asked open. +struct Folded { + opened: Vec, +} + +impl Render for Folded { + fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { + let view = cx.entity(); + let old: String = (0..20).map(|ix| format!("line {ix}\n")).collect(); + let new = old.replace("line 18", "line eighteen"); + div().w(px(480.0)).h(px(320.0)).child( + DiffViewer::new("diff", "a.rs", &old, &new) + .headless() + .on_open(move |ix, _, cx| view.update(cx, |folded, _| folded.opened.push(ix))), + ) + } +} + +fn settle(cx: &mut VisualTestContext) { + cx.run_until_parked(); + cx.update(|window, _| window.refresh()); + cx.run_until_parked(); +} + +#[gpui::test] +fn a_fold_opens_from_the_keyboard(cx: &mut TestAppContext) { + cx.update(|cx| { + Theme::init(cx); + forms::bind_keys(cx); + cx.bind_keys([KeyBinding::new("tab", FocusNext, None)]); + }); + let (view, cx) = cx.add_window_view(|_, _| Folded { opened: Vec::new() }); + cx.update(|window, _| window.activate_window()); + settle(cx); + cx.update(|window, _| window.focus_next()); + settle(cx); + cx.simulate_keystrokes("enter"); + settle(cx); + cx.simulate_event(KeyUpEvent { + keystroke: Keystroke::parse("enter").expect("a key"), + }); + settle(cx); + assert_eq!(view.read_with(cx, |folded, _| folded.opened.clone()), [0]); +} diff --git a/src/git/viewer.rs b/src/git/viewer.rs index 9a1b22a..b4246f6 100644 --- a/src/git/viewer.rs +++ b/src/git/viewer.rs @@ -3,7 +3,7 @@ use std::{ops::Range, rc::Rc}; use gpui::{ AnyElement, App, ElementId, FontWeight, HighlightStyle, Hsla, InteractiveElement, IntoElement, MouseButton, ParentElement, RenderOnce, SharedString, StatefulInteractiveElement, Styled, - StyledText, Window, div, prelude::*, relative, uniform_list, + StyledText, Window, div, prelude::*, relative, transparent_black, uniform_list, }; use super::{ @@ -13,6 +13,7 @@ use super::{ use crate::{ buttons::SegmentedControl, editor::{code_colors, stack}, + primitives::FocusRing, theme::{ActiveTheme, ControlSize, Palette, TextSize}, typography::{LEADING, format}, }; @@ -243,6 +244,8 @@ impl RenderOnce for DiffViewer { .w_full() .flex() .px_2() + .border_1() + .border_color(transparent_black()) .whitespace_nowrap() .line_height(relative(LEADING)); match &rows[ix] { @@ -287,13 +290,17 @@ impl RenderOnce for DiffViewer { .justify_center() .bg(colors.hover.opacity(0.5)) .text_color(colors.fg_subtle) - .cursor_pointer() - .hover(|row| row.text_color(colors.fg)) - .on_mouse_down(MouseButton::Left, |_, window, _| { - window.prevent_default() - }) .when_some(open, |row, open| { - row.on_click(move |_, window, cx| open(stretch, window, cx)) + row.tab_index(0) + .focus_ring(cx) + .cursor_pointer() + .hover(|row| row.text_color(colors.fg)) + .on_mouse_down(MouseButton::Left, |_, window, _| { + window.prevent_default() + }) + .on_click(move |_, window, cx| { + open(stretch, window, cx) + }) }) .child(format!( "⋯ {}",