From af258244c881cf537fde38f15784cee0d869c9c2 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 06:16:43 -0400 Subject: [PATCH] T23b round 1 fixes - source cards and web results with a handler are Tab stops with focus rings - the sources row and a passage's header shrink with an ellipsis, keeping the count, chevron and score in view - chat tests move into a folder, citations in their own file - gallery: a narrow column of citations --- TASKS.md | 1 + examples/gallery/pages/chat/cite.rs | 17 ++++ src/chat/cite.rs | 29 ++++-- src/chat/search.rs | 27 +++--- src/chat/tests/cite.rs | 137 ++++++++++++++++++++++++++++ src/chat/{tests.rs => tests/mod.rs} | 94 +------------------ 6 files changed, 197 insertions(+), 108 deletions(-) create mode 100644 src/chat/tests/cite.rs rename src/chat/{tests.rs => tests/mod.rs} (79%) diff --git a/TASKS.md b/TASKS.md index 9433e4e..4ca907d 100644 --- a/TASKS.md +++ b/TASKS.md @@ -258,3 +258,4 @@ Per-component lines live in `tasks/`. Tags there: | T22 | 2 | PASS | | | T23a | 1 | FAIL | a long message of yours overflowed a narrow column; pictures and video sized to the theme, not their column; a finished stream kept revealing; avatar pictures from the web were read as files; the list stayed away after it fit again; the reasoning toggle had no Tab stop; a link's thumbnail lost its corners | | T23a | 2 | PASS | | +| T23b | 1 | FAIL | source cards and web results with a handler were no Tab stops; in a narrow column the sources row and a passage's header ran past their edge | diff --git a/examples/gallery/pages/chat/cite.rs b/examples/gallery/pages/chat/cite.rs index 4348e50..bc4fade 100644 --- a/examples/gallery/pages/chat/cite.rs +++ b/examples/gallery/pages/chat/cite.rs @@ -118,6 +118,23 @@ pub fn search(_: &mut Window, cx: &mut App) -> impl IntoElement + use<> { vec![more..more + "more lift".len(), louder..louder + "looks louder".len()], ) .score(0.86), + ) + .child( + div() + .w(px(280.)) + .flex() + .flex_col() + .gap_3() + .child(SourceList::new("chat-sources-narrow", all.clone())) + .child( + DocumentChunkPreview::new( + "a-long-name-for-the-lift-notes.pdf", + "Page 1 · Dark is not inverted", + PASSAGE, + vec![more..more + "more lift".len()], + ) + .score(0.86), + ), ), ) } diff --git a/src/chat/cite.rs b/src/chat/cite.rs index db28d02..da203d0 100644 --- a/src/chat/cite.rs +++ b/src/chat/cite.rs @@ -82,7 +82,7 @@ fn lines(number: Option, source: &Source, cx: &App) -> Div { .size(IconSize::Xs) .color(colors.fg_subtle), ) - .child(source.site.clone()), + .child(div().min_w_0().child(Ellipsis::new(source.site.clone()))), ) .child( div() @@ -112,7 +112,9 @@ impl RenderOnce for SourceCard { .border_color(colors.border) .bg(colors.surface) .when_some(open, |card, open| { - card.cursor_pointer() + card.tab_index(0) + .focus_ring(cx) + .cursor_pointer() .hover(|card| card.bg(colors.hover)) .on_mouse_down(MouseButton::Left, |_, window, _| window.prevent_default()) .on_click(move |_, window, cx| { @@ -246,14 +248,25 @@ impl RenderOnce for SourceList { cx.notify(); }) }) - .child(div().font_weight(FontWeight::MEDIUM).child(match count { - 1 => "1 source".to_string(), - n => format!("{n} sources"), - })) - .child(div().text_color(colors.fg_subtle).child(sites.join(" · "))) .child( + div() + .flex_none() + .font_weight(FontWeight::MEDIUM) + .child(match count { + 1 => "1 source".to_string(), + n => format!("{n} sources"), + }), + ) + .child( + div() + .flex_1() + .min_w_0() + .text_color(colors.fg_subtle) + .child(Ellipsis::new(sites.join(" · "))), + ) + .child(div().flex_none().child( Disclosure::new((self.id.clone(), "chevron"), opened).size(IconSize::Sm), - ), + )), ) .when(opened, |list| { list.child(div().flex().flex_col().gap_2().children( diff --git a/src/chat/search.rs b/src/chat/search.rs index 094cfcb..1f53ece 100644 --- a/src/chat/search.rs +++ b/src/chat/search.rs @@ -3,14 +3,14 @@ use std::{ops::Range, rc::Rc}; use gpui::{ App, ElementId, FontWeight, HighlightStyle, InteractiveElement, IntoElement, MouseButton, ParentElement, RenderOnce, SharedString, StatefulInteractiveElement, Styled, StyledText, - Window, div, prelude::*, + Window, div, prelude::*, transparent_black, }; use super::cite::Source; use crate::{ forms::Run, motion::Spinner, - primitives::{Icon, IconName}, + primitives::{FocusRing, Icon, IconName}, theme::{ActiveTheme, IconSize, Radius, TextSize}, typography::{Ellipsis, tabular}, }; @@ -145,7 +145,13 @@ impl RenderOnce for WebResultCard { .gap_0p5() .py_2() .when_some(open, |card, open| { - card.cursor_pointer() + card.px_1() + .rounded(theme.radius(Radius::Md)) + .border_1() + .border_color(transparent_black()) + .tab_index(0) + .focus_ring(cx) + .cursor_pointer() .on_mouse_down(MouseButton::Left, |_, window, _| window.prevent_default()) .on_click(move |_, window, cx| open(window, cx)) }) @@ -167,7 +173,7 @@ impl RenderOnce for WebResultCard { .min_w_0() .child(Ellipsis::new(self.source.url.clone())), ) - .children(self.date.map(|date| div().child(date))), + .children(self.date.map(|date| div().flex_none().child(date))), ) .child( div() @@ -265,16 +271,15 @@ impl RenderOnce for DocumentChunkPreview { ) .child( div() + .min_w_0() .font_weight(FontWeight::MEDIUM) .text_color(colors.fg_muted) - .child(self.document), + .child(Ellipsis::new(self.document)), ) - .child(div().flex_1().child(self.place)) - .children( - self.score.map(|score| { - tabular(div()).child(format!("{:.0}% match", score * 100.0)) - }), - ), + .child(div().flex_1().min_w_0().child(Ellipsis::new(self.place))) + .children(self.score.map(|score| { + tabular(div().flex_none()).child(format!("{:.0}% match", score * 100.0)) + })), ) .child( div() diff --git a/src/chat/tests/cite.rs b/src/chat/tests/cite.rs new file mode 100644 index 0000000..89e5a7f --- /dev/null +++ b/src/chat/tests/cite.rs @@ -0,0 +1,137 @@ +use gpui::{ + Context, IntoElement, KeyUpEvent, Keystroke, ParentElement, Render, Styled, TestAppContext, + VisualTestContext, Window, canvas, div, px, +}; + +use super::{Measured, settle}; +use crate::{ + chat::{CitationBadge, Source, SourceCard, SourceList, WebResultCard}, + forms, + theme::Theme, +}; + +fn source(title: &str) -> Source { + Source { + site: "example.com".into(), + title: title.to_string().into(), + url: "https://example.com".into(), + snippet: None, + } +} + +/// Sources folded under their count, the list's box measured, and how often one opened. +struct Cited { + frame: Measured, + opened: usize, +} + +impl Render for Cited { + fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { + let (frame, me) = (self.frame.clone(), cx.entity()); + div() + .size_full() + .child( + CitationBadge::new("cite", 1, source("Gamma")) + .on_open(move |_, cx| me.update(cx, |cited, _| cited.opened += 1)), + ) + .child( + div() + .w(px(320.0)) + .relative() + .child(SourceList::new( + "sources", + [source("Gamma"), source("Lift")], + )) + .child( + canvas( + move |bounds, _, _| *frame.borrow_mut() = Some(bounds), + |_, _, _, _| {}, + ) + .absolute() + .top_0() + .left_0() + .size_full(), + ), + ) + } +} + +fn release(key: &str, cx: &mut VisualTestContext) { + cx.simulate_keystrokes(key); + cx.simulate_event(KeyUpEvent { + keystroke: Keystroke::parse(key).expect("the key parses"), + }); + settle(cx); +} + +#[gpui::test] +fn citations_and_sources_open_from_the_keyboard(cx: &mut TestAppContext) { + cx.update(|cx| { + Theme::init(cx); + forms::bind_keys(cx); + }); + let frame = Measured::default(); + let seen = frame.clone(); + let (cited, cx) = cx.add_window_view(move |_, _| Cited { + frame: seen, + opened: 0, + }); + cx.update(|window, _| window.activate_window()); + settle(cx); + cx.update(|window, _| window.focus_next()); + release("enter", cx); + assert_eq!( + cited.read_with(cx, |cited, _| cited.opened), + 1, + "the badge opens its source" + ); + let folded = frame.borrow().expect("the list is laid out").size.height; + cx.update(|window, _| window.focus_next()); + release("space", cx); + let open = frame.borrow().expect("the list is laid out").size.height; + assert!( + open > folded, + "Tab and Space open the sources: {folded:?} to {open:?}" + ); +} + +/// A source card and a web result, each to open, and which opened. +struct Cards { + opened: Vec<&'static str>, +} + +impl Render for Cards { + fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { + let (card, result) = (cx.entity(), cx.entity()); + div() + .size_full() + .child( + SourceCard::new("card", source("Gamma")) + .on_open(move |_, cx| card.update(cx, |cards, _| cards.opened.push("card"))), + ) + .child( + WebResultCard::new("result", source("Lift")).on_open(move |_, cx| { + result.update(cx, |cards, _| cards.opened.push("result")) + }), + ) + } +} + +#[gpui::test] +fn cards_open_from_the_keyboard(cx: &mut TestAppContext) { + cx.update(|cx| { + Theme::init(cx); + forms::bind_keys(cx); + }); + let (cards, cx) = cx.add_window_view(|_, _| Cards { opened: Vec::new() }); + cx.update(|window, _| window.activate_window()); + settle(cx); + cx.update(|window, _| window.focus_next()); + release("enter", cx); + cx.update(|window, _| window.focus_next()); + release("enter", cx); + assert_eq!( + cards.read_with(cx, |cards, _| cards.opened.clone()), + ["card", "result"] + ); +} diff --git a/src/chat/tests.rs b/src/chat/tests/mod.rs similarity index 79% rename from src/chat/tests.rs rename to src/chat/tests/mod.rs index aa6fa27..d8c794a 100644 --- a/src/chat/tests.rs +++ b/src/chat/tests/mod.rs @@ -7,9 +7,10 @@ use gpui::{ point, px, }; +mod cite; + use super::{ - CitationBadge, ImageMessage, MessageAvatar, MessageBubble, MessageEditor, MessageList, Role, - Source, SourceList, ThinkingBlock, + ImageMessage, MessageAvatar, MessageBubble, MessageEditor, MessageList, Role, ThinkingBlock, }; use crate::{forms, forms::TextInput, theme::Theme}; @@ -40,7 +41,7 @@ impl Render for Talk { } } -fn settle(cx: &mut VisualTestContext) { +pub(super) fn settle(cx: &mut VisualTestContext) { cx.run_until_parked(); cx.update(|window, _| window.refresh()); cx.run_until_parked(); @@ -174,7 +175,7 @@ fn the_editor_takes_focus_as_it_opens(cx: &mut TestAppContext) { ); } -type Measured = Rc>>>; +pub(super) type Measured = Rc>>>; /// A long message of yours in a narrow column, its body's box measured. struct Narrow { @@ -357,88 +358,3 @@ fn the_reasoning_opens_from_the_keyboard(cx: &mut TestAppContext) { "Tab and Space open it: {closed:?} to {open:?}" ); } - -fn source(title: &str) -> Source { - Source { - site: "example.com".into(), - title: title.to_string().into(), - url: "https://example.com".into(), - snippet: None, - } -} - -/// Sources folded under their count, the list's box measured, and how often one opened. -struct Cited { - frame: Measured, - opened: usize, -} - -impl Render for Cited { - fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { - let (frame, me) = (self.frame.clone(), cx.entity()); - div() - .size_full() - .child( - CitationBadge::new("cite", 1, source("Gamma")) - .on_open(move |_, cx| me.update(cx, |cited, _| cited.opened += 1)), - ) - .child( - div() - .w(px(320.0)) - .relative() - .child(SourceList::new( - "sources", - [source("Gamma"), source("Lift")], - )) - .child( - canvas( - move |bounds, _, _| *frame.borrow_mut() = Some(bounds), - |_, _, _, _| {}, - ) - .absolute() - .top_0() - .left_0() - .size_full(), - ), - ) - } -} - -fn release(key: &str, cx: &mut VisualTestContext) { - cx.simulate_keystrokes(key); - cx.simulate_event(KeyUpEvent { - keystroke: Keystroke::parse(key).expect("the key parses"), - }); - settle(cx); -} - -#[gpui::test] -fn citations_and_sources_open_from_the_keyboard(cx: &mut TestAppContext) { - cx.update(|cx| { - Theme::init(cx); - forms::bind_keys(cx); - }); - let frame = Measured::default(); - let seen = frame.clone(); - let (cited, cx) = cx.add_window_view(move |_, _| Cited { - frame: seen, - opened: 0, - }); - cx.update(|window, _| window.activate_window()); - settle(cx); - cx.update(|window, _| window.focus_next()); - release("enter", cx); - assert_eq!( - cited.read_with(cx, |cited, _| cited.opened), - 1, - "the badge opens its source" - ); - let folded = frame.borrow().expect("the list is laid out").size.height; - cx.update(|window, _| window.focus_next()); - release("space", cx); - let open = frame.borrow().expect("the list is laid out").size.height; - assert!( - open > folded, - "Tab and Space open the sources: {folded:?} to {open:?}" - ); -}