T24c round 1 fix
- FileChangeCard's header wraps: the path keeps label_width, counts and verdict drop below as one group; MultiFileDiffReview's header wraps its counts and its all-buttons inside the box, parted by justify_between - git::DiffViewer's fold rows are Tab stops with a focus ring when the owner opens folds; every row keeps a transparent border so heights match; an inert fold no longer shows a pointer - tests: a_narrow_review_wraps_its_rows_inside_its_box, git::a_fold_opens_from_the_keyboard; the gallery adds a 280px review - AGENTS: taffy drops gaps beside an auto margin; wrapping headers
This commit is contained in:
1 parent
0a44a2d905
commit
b9bff970d0
8 files changed
+172
-28
No files matched your search
@@ -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
|
||||
|
||||
|
||||
@@ -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 |
|
||||
@@ -65,22 +65,31 @@ fn decided(mut changes: Vec<FileChange>, decision: Decision) -> Vec<FileChange>
|
||||
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",
|
||||
|
||||
+32
-16
@@ -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)| {
|
||||
|
||||
@@ -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<bool>,
|
||||
height: Rc<Cell<Pixels>>,
|
||||
@@ -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<Cell<Pixels>>,
|
||||
}
|
||||
|
||||
impl Render for Narrow {
|
||||
fn render(&mut self, _: &mut Window, _: &mut Context<Self>) -> 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()
|
||||
);
|
||||
}
|
||||
@@ -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};
|
||||
|
||||
@@ -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<usize>,
|
||||
}
|
||||
|
||||
impl Render for Folded {
|
||||
fn render(&mut self, _: &mut Window, cx: &mut Context<Self>) -> 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]);
|
||||
}
|
||||
+14
-7
@@ -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!(
|
||||
"⋯ {}",
|
||||
|
||||
Reference in new issue
Block a user