From 4f9e9eb7564d79923d515247af73d8800821c0f7 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 07:42:45 -0400 Subject: [PATCH] T23d round 1 fix - the conversation list reads the zone through format::system_zone, which fails loud; the gallery expects the system zone - Combobox opens with its cursor on its value, MultiSelect on the first row ticked; tests for all three lists open r25 of thirty in view - AGENTS: the zone rule; lists open on their choice --- AGENTS.md | 2 + TASKS.md | 1 + examples/gallery/pages/chat/history.rs | 4 +- examples/gallery/pages/chat/talk.rs | 4 +- src/chat/conversations.rs | 4 +- src/forms/combobox.rs | 12 ++++ src/forms/multi.rs | 5 +- src/forms/tests/choices.rs | 81 +++++++++++++++++++------- 8 files changed, 87 insertions(+), 26 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 965e779..57c9c61 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -131,6 +131,8 @@ Ely GPUI Component. A component library for GPUI, in light and dark. - Editor rows read their text through `CodeEditor::row_text`: a multi-line ghost cuts its line, and the rest follows the ghost's last line. - gpui styles a drag-over only on an element with its own hitbox. A veil that shows while files hover takes the drop itself (`chat::DragDropOverlay`). - gpui 0.2.2 never clears `debug_bounds`: a test sees an element appear, not leave. +- 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. ## Decisions diff --git a/TASKS.md b/TASKS.md index 6cfec90..0d31128 100644 --- a/TASKS.md +++ b/TASKS.md @@ -264,3 +264,4 @@ Per-component lines live in `tasks/`. Tags there: | T23c | 1 | FAIL | suggestion arrows landed on disabled rows and left the cursor out of view; a dot ended a file mention; the drop veil never showed, since gpui styles a drag-over only on an element with a hitbox; a parameter's places came from its step alone; the gallery's prompt menu and Reset only logged | | T23c | 2 | FAIL | a slash inside a path read as a new trigger, closing the file suggestions; a list whose first rows were disabled opened with its cursor out of view | | T23c | 3 | PASS | | +| T23d | 1 | FAIL | the list grouped days in a silent UTC fallback when the system zone was unknown; Combobox and MultiSelect opened a long list at its top, away from the choice | diff --git a/examples/gallery/pages/chat/history.rs b/examples/gallery/pages/chat/history.rs index 3edc103..47136d3 100644 --- a/examples/gallery/pages/chat/history.rs +++ b/examples/gallery/pages/chat/history.rs @@ -79,7 +79,9 @@ pub fn conversations_section( window: &mut Window, cx: &mut App, ) -> impl IntoElement + use<> { - let today = Timestamp::now().to_zoned(TimeZone::system()).date(); + let today = Timestamp::now() + .to_zoned(TimeZone::try_system().expect("the gallery reads the system time zone")) + .date(); let query = window.use_keyed_state("chat-find-conversation", cx, |window, cx| { TextInput::new(window, cx).placeholder("Search conversations") }); diff --git a/examples/gallery/pages/chat/talk.rs b/examples/gallery/pages/chat/talk.rs index 5743759..0d93e11 100644 --- a/examples/gallery/pages/chat/talk.rs +++ b/examples/gallery/pages/chat/talk.rs @@ -115,7 +115,9 @@ fn row(ix: usize, row: &Row, today: Date, now: Timestamp) -> gpui::AnyElement { } pub fn conversation(_: &mut Window, cx: &mut App) -> impl IntoElement + use<> { - let today = Timestamp::now().to_zoned(TimeZone::system()).date(); + let today = Timestamp::now() + .to_zoned(TimeZone::try_system().expect("the gallery reads the system time zone")) + .date(); let now = Timestamp::now(); let all = rows(today); let count = all.len(); diff --git a/src/chat/conversations.rs b/src/chat/conversations.rs index 5d66c53..b8cb0e5 100644 --- a/src/chat/conversations.rs +++ b/src/chat/conversations.rs @@ -13,7 +13,7 @@ use crate::{ menus::{Menu, MenuItem, OverflowMenu}, primitives::{FocusRing, Icon, IconName, tab_stop}, theme::{ActiveTheme, ControlSize, IconSize, Radius, TextSize}, - typography::Ellipsis, + typography::{Ellipsis, format::system_zone}, }; /// A conversation in a list: its key, its title, when it last moved, and whether it is pinned. @@ -302,7 +302,7 @@ impl RenderOnce for ConversationList { &self.conversations, &self.query, self.today, - &TimeZone::system(), + &system_zone("conversation list"), ); let empty = groups.is_empty(); div() diff --git a/src/forms/combobox.rs b/src/forms/combobox.rs index fc00206..03b1b19 100644 --- a/src/forms/combobox.rs +++ b/src/forms/combobox.rs @@ -158,6 +158,18 @@ impl RenderOnce for Combobox { }; let rows = Rc::new(matching(&self.choices, query)); let open = focused && !typing.read(cx).dismissed && !rows.is_empty(); + if open != picker.read(cx).open { + let chosen = self.selected.as_ref().and_then(|value| { + rows.iter() + .position(|row| !row.disabled && row.value == *value) + }); + picker.update(cx, |picker, _| { + picker.open = open; + if let Some(at) = chosen.filter(|_| open) { + picker.highlighted = at; + } + }); + } let kept = picker.read(cx).highlighted; let highlighted = if rows.get(kept).is_some_and(|row| !row.disabled) { kept diff --git a/src/forms/multi.rs b/src/forms/multi.rs index 59e9dff..414a86a 100644 --- a/src/forms/multi.rs +++ b/src/forms/multi.rs @@ -79,7 +79,10 @@ impl RenderOnce for MultiSelect { picker.update(cx, |picker, _| picker.open = false); } let (choices, selected) = (Rc::new(self.choices), Rc::new(self.selected)); - let start = step(&choices, choices.len() - 1, 1); + let start = choices + .iter() + .position(|choice| !choice.disabled && selected.contains(&choice.value)) + .unwrap_or_else(|| step(&choices, choices.len() - 1, 1)); if picker.read(cx).highlighted >= choices.len() { picker.update(cx, |picker, _| picker.highlighted = start); } diff --git a/src/forms/tests/choices.rs b/src/forms/tests/choices.rs index a8029d1..8a2a0db 100644 --- a/src/forms/tests/choices.rs +++ b/src/forms/tests/choices.rs @@ -87,38 +87,59 @@ fn select_opens_moves_and_picks_from_the_keyboard(cx: &mut TestAppContext) { assert_eq!(view.read_with(cx, |view, _| view.chosen.clone()), "b"); } -/// A select of thirty rows, low in the window, and the rows it picked. +/// Which list of thirty rows a test opens. +#[derive(Clone, Copy)] +enum Kind { + Select, + Combobox, + Multi, +} + +/// A list of thirty rows with r25 chosen, low in the window, and what each press picked. struct Long { - chosen: SharedString, - picks: Vec, + kind: Kind, + field: Entity, + picks: Vec, } impl Render for Long { fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { - let view = cx.entity(); let rows = (0..30).map(|ix| Choice::new(format!("r{ix:02}"), format!("Row {ix:02}"))); - div().pt(px(200.0)).w(px(240.0)).child( - Select::new("long", rows) - .selected(self.chosen.clone()) - .on_change(move |value, _, cx| { - view.update(cx, |view, cx| { - view.chosen = value.clone(); - view.picks.push(value.clone()); - cx.notify(); - }) - }), - ) + let view = cx.entity(); + let note = move |pick: String, cx: &mut gpui::App| { + view.update(cx, |view, cx| { + view.picks.push(pick); + cx.notify(); + }) + }; + let list = match self.kind { + Kind::Select => Select::new("long", rows) + .selected("r25") + .on_change(move |value, _, cx| note(value.to_string(), cx)) + .into_any_element(), + Kind::Combobox => Combobox::new("long", &self.field, rows) + .selected("r25") + .on_change(move |value, _, cx| note(value.to_string(), cx)) + .into_any_element(), + Kind::Multi => MultiSelect::new("long", rows) + .selected([SharedString::from("r25")]) + .on_change(move |next, _, cx| note(next.join(","), cx)) + .into_any_element(), + }; + div().pt(px(200.0)).w(px(240.0)).child(list) } } -#[gpui::test] -fn a_long_list_opens_with_its_choice_in_view(cx: &mut TestAppContext) { +/// Opens the list with a press on its field, then presses the row at its foot. +fn press_the_foot(kind: Kind, cx: &mut TestAppContext) -> Vec { setup(cx); cx.update(|cx| Theme::update(cx, |theme| theme.reduced_motion = true)); - let (view, cx) = cx.add_window_view(|_, _| Long { - chosen: "r25".into(), + let (view, cx) = cx.add_window_view(|window, cx| Long { + kind, + field: cx.new(|cx| TextInput::new(window, cx)), picks: Vec::new(), }); + cx.update(|window, _| window.activate_window()); let (trigger, list) = cx.update(|window, cx| { let theme = cx.theme(); ( @@ -139,8 +160,26 @@ fn a_long_list_opens_with_its_choice_in_view(cx: &mut TestAppContext) { cx.simulate_mouse_move(foot, None, Modifiers::none()); cx.simulate_click(foot, Modifiers::none()); frames(cx); - let picks = view.read_with(cx, |view, _| view.picks.clone()); - assert_eq!(picks, ["r25"], "the list opens with its choice at the foot"); + view.read_with(cx, |view, _| view.picks.clone()) +} + +#[gpui::test] +fn a_long_select_opens_with_its_choice_in_view(cx: &mut TestAppContext) { + assert_eq!(press_the_foot(Kind::Select, cx), ["r25"]); +} + +#[gpui::test] +fn a_long_combobox_opens_with_its_choice_in_view(cx: &mut TestAppContext) { + assert_eq!(press_the_foot(Kind::Combobox, cx), ["r25"]); +} + +#[gpui::test] +fn a_long_multi_select_opens_with_its_first_choice_in_view(cx: &mut TestAppContext) { + assert_eq!( + press_the_foot(Kind::Multi, cx), + [""], + "the press unticks r25" + ); } /// Frames 2ms apart, past reduced motion's 1ms entrances.