Onboarding · Help: articles in a list the keyboard walks, a long address cut

HelpPanel lists its articles in lists::SelectableList: the list is one
stop whose cursor scrolls into view, and Enter or a double press opens
an article, focus going to All articles as before. SelectableList now
activates on Enter's release, armed by its own press, as the palette
and menus pick: an activation that moves focus left the release to
press what took it, here the way back. ContactSupport cuts a long
address in the middle and puts a Copy beside it. The lists tests move
into a folder, the release tests on their own.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

src/lists/select.rs

@@ -47,13 +47,14 @@ struct Cursor {
     scroll: ScrollHandle,
     seen: Option<Vec<SharedString>>,
     sent: Option<Vec<SharedString>>,
+    armed: bool,
 }
 
 type OnSelect = Rc<dyn Fn(&[SharedString], &mut Window, &mut App)>;
 type OnActivate = Rc<dyn Fn(&SharedString, &mut Window, &mut App)>;
 type Picker = Rc<dyn Fn(usize, Pick, &mut Window, &mut App)>;
 
-/// Rows you select. A press picks one; with `multiple`, Cmd-press adds or drops one and Shift-press takes the range from the last pick. Up and Down move, Shift with them extends, Space toggles, Cmd-A takes all. Keys pass over disabled rows, and no pick adds one.
+/// Rows you select. A press picks one; with `multiple`, Cmd-press adds or drops one and Shift-press takes the range from the last pick. Up and Down move, Shift with them extends, Space toggles, Cmd-A takes all, and Enter activates the row on its release, so an activation that moves focus leaves the release nothing to press. Keys pass over disabled rows, and no pick adds one.
 #[derive(IntoElement)]
 pub struct SelectableList {
     id: ElementId,
@@ -235,6 +236,8 @@ impl RenderOnce for SelectableList {
                     })
             })
             .collect();
+        let (lifted, released, barred_rows, keyed) =
+            (cursor.clone(), activate.clone(), off.clone(), keys.clone());
         self.base
             .id(self.id)
             .track_focus(&focus)
@@ -243,6 +246,22 @@ impl RenderOnce for SelectableList {
             .flex()
             .flex_col()
             .gap_0p5()
+            .on_key_up(move |event, window, cx| {
+                if event.keystroke.key != "enter" || count == 0 {
+                    return;
+                }
+                let (armed, at) = lifted.update(cx, |cursor, _| {
+                    (std::mem::take(&mut cursor.armed), cursor.at.min(count - 1))
+                });
+                if !armed {
+                    return;
+                }
+                cx.stop_propagation();
+                if barred_rows[at] {
+                    return;
+                }
+                released(&keyed[at], window, cx);
+            })
             .on_key_down(move |event, window, cx| {
                 if count == 0 {
                     return;
@@ -259,9 +278,7 @@ impl RenderOnce for SelectableList {
                     "space" => (Some(at).filter(open), Pick::Toggle),
                     "enter" => {
                         cx.stop_propagation();
-                        if open(&at) {
-                            activate(&keys[at], window, cx);
-                        }
+                        cursor.update(cx, |cursor, _| cursor.armed = !held.modified());
                         return;
                     }
                     "a" if held.platform => (Some(at), Pick::All),

src/lists/tests/mod.rs

@@ -18,6 +18,8 @@ use crate::{
     theme::Theme,
 };
 
+mod release;
+
 fn setup(cx: &mut TestAppContext) {
     cx.update(|cx| {
         Theme::init(cx);
@@ -322,6 +324,14 @@ fn a_long_list_builds_only_what_is_near_the_view(cx: &mut TestAppContext) {
     assert!((9..60).contains(&most), "built up to row {most} of 1000");
 }
 
+/// Enter, pressed and let go: a row opens on the release.
+fn enter(cx: &mut VisualTestContext) {
+    cx.simulate_keystrokes("enter");
+    cx.simulate_event(gpui::KeyUpEvent {
+        keystroke: gpui::Keystroke::parse("enter").expect("a key"),
+    });
+}
+
 #[gpui::test]
 fn enter_or_a_double_press_opens_a_row(cx: &mut TestAppContext) {
     setup(cx);
@@ -329,7 +339,7 @@ fn enter_or_a_double_press_opens_a_row(cx: &mut TestAppContext) {
     settle(cx);
     cx.simulate_click(row(1), Modifiers::none());
     settle(cx);
-    cx.simulate_keystrokes("enter");
+    enter(cx);
     settle(cx);
     for count in [1, 2] {
         cx.simulate_event(gpui::MouseDownEvent {
@@ -363,7 +373,9 @@ fn keys_pass_over_a_disabled_row_and_never_pick_or_open_it(cx: &mut TestAppConte
     cx.simulate_keystrokes("down");
     settle(cx);
     assert_eq!(picks(&view, cx), ["c"], "down steps over the disabled row");
-    cx.simulate_keystrokes("up enter cmd-a");
+    cx.simulate_keystrokes("up");
+    enter(cx);
+    cx.simulate_keystrokes("cmd-a");
     settle(cx);
     assert_eq!(
         picks(&view, cx),
@@ -392,7 +404,8 @@ fn the_cursor_stays_on_its_row_when_the_rows_reorder(cx: &mut TestAppContext) {
         cx.notify();
     });
     settle(cx);
-    cx.simulate_keystrokes("enter up");
+    enter(cx);
+    cx.simulate_keystrokes("up");
     settle(cx);
     let opened: Vec<String> = view.read_with(cx, |picks, _| {
         picks.1.iter().map(|key| key.to_string()).collect()

src/lists/tests/release.rs

@@ -0,0 +1,94 @@
+use gpui::{
+    AppContext, Context, Entity, FocusHandle, InteractiveElement, IntoElement, ParentElement,
+    Render, TestAppContext, Window, div,
+};
+
+use super::{enter, settle, setup};
+use crate::{
+    buttons::Button,
+    forms::{Enter, Input, TextInput},
+    lists::{ListItem, SelectableList},
+};
+
+/// A list whose open hands focus to a button beside it, and what each heard.
+struct Handoff {
+    button: FocusHandle,
+    heard: Vec<&'static str>,
+}
+
+impl Render for Handoff {
+    fn render(&mut self, _: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
+        let (to, opened, clicked) = (self.button.clone(), cx.entity(), cx.entity());
+        div()
+            .child(
+                SelectableList::new("handoff")
+                    .row("a", ListItem::new("handoff-a", "A"))
+                    .on_activate(move |_, window, cx| {
+                        window.focus(&to);
+                        opened.update(cx, |view, _| view.heard.push("open"));
+                    }),
+            )
+            .child(
+                Button::new("after", "After")
+                    .focus_handle(&self.button)
+                    .on_click(move |_, _, cx| {
+                        clicked.update(cx, |view, _| view.heard.push("click"))
+                    }),
+            )
+    }
+}
+
+/// Enter opens on its release, so an open that moves focus to a button leaves the button unpressed.
+#[gpui::test]
+fn an_open_that_moves_focus_leaves_the_release_alone(cx: &mut TestAppContext) {
+    setup(cx);
+    let (view, cx) = cx.add_window_view(|_, cx| Handoff {
+        button: cx.focus_handle().tab_stop(true),
+        heard: Vec::new(),
+    });
+    settle(cx);
+    cx.update(|window, _| window.focus_next());
+    settle(cx);
+    enter(cx);
+    settle(cx);
+    assert_eq!(view.read_with(cx, |view, _| view.heard.clone()), ["open"]);
+}
+
+/// A field whose Enter moves focus on to a list, and what the list opened.
+struct Relay {
+    field: Entity<TextInput>,
+    heard: Vec<&'static str>,
+}
+
+impl Render for Relay {
+    fn render(&mut self, _: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
+        let opened = cx.entity();
+        div()
+            .capture_action(|_: &Enter, window, _| window.focus_next())
+            .child(Input::new(&self.field))
+            .child(
+                SelectableList::new("relay")
+                    .row("a", ListItem::new("relay-a", "A"))
+                    .on_activate(move |_, _, cx| {
+                        opened.update(cx, |view, _| view.heard.push("open"))
+                    }),
+            )
+    }
+}
+
+/// A release whose press fell elsewhere opens nothing.
+#[gpui::test]
+fn a_release_that_began_elsewhere_opens_nothing(cx: &mut TestAppContext) {
+    setup(cx);
+    cx.update(crate::forms::bind_keys);
+    let (view, cx) = cx.add_window_view(|window, cx| Relay {
+        field: cx.new(|cx| TextInput::new(window, cx)),
+        heard: Vec::new(),
+    });
+    settle(cx);
+    cx.update(|window, _| window.focus_next());
+    settle(cx);
+    enter(cx);
+    settle(cx);
+    assert!(view.read_with(cx, |view, _| view.heard.is_empty()));
+}

src/onboarding/help.rs

@@ -10,9 +10,9 @@ use crate::{
     documents::MarkdownRenderer,
     forms::{Input, Run, TextInput},
     layout::on_axis,
-    primitives::{FocusRing, Icon, IconName, tab_stop},
-    theme::{ActiveTheme, ControlSize, IconSize, Radius, TextSize},
-    typography::Ellipsis,
+    lists::{ListItem, SelectableList},
+    primitives::{Icon, IconName, tab_stop},
+    theme::{ActiveTheme, ControlSize, IconSize, TextSize},
 };
 
 /// A help article: its key, its title, a line on it, and its body in Markdown. A link to `help:<key>` in a body opens that article.
@@ -53,7 +53,7 @@ fn link(url: &str) -> Link<'_> {
 
 type OnOpen = Rc<dyn Fn(Option<&SharedString>, &mut Window, &mut App)>;
 
-/// Help beside the work: a search over the articles and their list, or one article with the way back to all. Focus follows: an article opened takes it to All articles, and the way back takes it to the search. Contact support and Keyboard shortcuts wait at its foot when the owner handles them. It fills its box, which the host sizes.
+/// Help beside the work: a search over the articles and their list, where Enter or a double press opens one, or one article with the way back to all. Focus follows: an article opened takes it to All articles, and the way back takes it to the search. Contact support and Keyboard shortcuts wait at its foot when the owner handles them. It fills its box, which the host sizes.
 #[derive(IntoElement)]
 pub struct HelpPanel {
     id: ElementId,
@@ -182,7 +182,7 @@ impl RenderOnce for HelpPanel {
         let body = match shown {
             Some(article) => {
                 let (back, follow) = (on_open.clone(), on_open.clone());
-                div()
+                let page = div()
                     .flex()
                     .flex_col()
                     .gap_3()
@@ -219,57 +219,52 @@ impl RenderOnce for HelpPanel {
                             }
                             Link::Web(url) => cx.open_url(url),
                         }),
-                    )
-                    .into_any_element()
+                    );
+                on_axis(
+                    div()
+                        .id((id.clone(), "body"))
+                        .flex_1()
+                        .min_h_0()
+                        .overflow_y_scroll()
+                        .p_4()
+                        .child(page),
+                )
+                .into_any_element()
             }
             None => {
                 let query = self.search.read(cx).text().trim().to_string();
                 let listed = found(&self.articles, &query);
                 let none = listed.is_empty();
-                let rows = listed.iter().map(|article| {
-                    let (open, key) = (on_open.clone(), article.key.clone());
-                    div()
-                        .id((id.clone(), format!("row-{}", article.key)))
-                        .flex()
-                        .child(
-                            div()
-                                .flex_1()
-                                .min_w_0()
-                                .child(
-                                    div()
-                                        .font_weight(FontWeight::MEDIUM)
-                                        .child(Ellipsis::new(article.title.clone())),
-                                )
-                                .child(
-                                    div()
-                                        .text_size(theme.text_size(TextSize::Sm))
-                                        .text_color(theme.colors.fg_muted)
-                                        .child(article.summary.clone()),
-                                ),
+                let list = listed.iter().fold(
+                    SelectableList::new((id.clone(), "articles")),
+                    |list, article| {
+                        list.row(
+                            article.key.clone(),
+                            ListItem::new(
+                                (id.clone(), format!("row-{}", article.key)),
+                                article.title.clone(),
+                            )
+                            .description(article.summary.clone()),
                         )
-                        .px_2()
-                        .py_1p5()
-                        .rounded(theme.radius(Radius::Md))
-                        .border_1()
-                        .border_color(gpui::transparent_black())
-                        .cursor_pointer()
-                        .hover(|style| style.bg(theme.colors.hover))
-                        .tab_index(0)
-                        .focus_ring(cx)
-                        .on_click(move |_, window, cx| {
-                            log::info!("help panel: open {key}");
-                            open(Some(&key), window, cx)
-                        })
-                });
+                    },
+                );
                 div()
+                    .flex_1()
+                    .min_h_0()
                     .flex()
                     .flex_col()
                     .gap_3()
+                    .p_4()
                     .child(
                         Input::new(&self.search)
                             .prefix(Icon::new(IconName::Search).size(IconSize::Sm)),
                     )
-                    .child(div().flex().flex_col().gap_0p5().children(rows))
+                    .children((!none).then(|| {
+                        list.on_activate(move |key, window, cx| {
+                            log::info!("help panel: open {key}");
+                            on_open(Some(key), window, cx)
+                        })
+                    }))
                     .children(none.then(|| {
                         div()
                             .text_size(theme.text_size(TextSize::Sm))
@@ -299,6 +294,7 @@ impl RenderOnce for HelpPanel {
         });
         let foot = (contact.is_some() || shortcuts.is_some()).then(|| {
             div()
+                .debug_selector(|| "help-foot".into())
                 .flex()
                 .flex_none()
                 .flex_wrap()
@@ -315,15 +311,7 @@ impl RenderOnce for HelpPanel {
             .flex()
             .flex_col()
             .child(head)
-            .child(on_axis(
-                div()
-                    .id((id.clone(), "body"))
-                    .flex_1()
-                    .min_h_0()
-                    .overflow_y_scroll()
-                    .p_4()
-                    .child(body),
-            ))
+            .child(body)
             .children(foot)
     }
 }

src/onboarding/support.rs

@@ -1,20 +1,21 @@
 use std::rc::Rc;
 
 use gpui::{
-    App, ElementId, IntoElement, ParentElement, RenderOnce, SharedString, Styled, Window, div,
+    App, ElementId, InteractiveElement, IntoElement, ParentElement, RenderOnce, SharedString,
+    Styled, Window, div,
 };
 
 use crate::{
-    buttons::{Button, ButtonVariant},
+    buttons::{Button, ButtonVariant, CopyButton},
     forms::{Choice, FormField, Input, Run, Select, TextInput},
     primitives::{Icon, IconName},
     theme::{ActiveTheme, IconSize, TextSize},
-    typography::literal,
+    typography::{MiddleEllipsis, literal},
 };
 
 type OnMessage = Rc<dyn Fn(&SharedString, &str, &mut Window, &mut App)>;
 
-/// A way to reach people: a topic and a message, and Send once both are given. It rests while the owner sends, and empties the message once sent, with focus back in it. With `address`, the address to write to instead shows under it.
+/// A way to reach people: a topic and a message, and Send once both are given. It rests while the owner sends, and empties the message once sent, with focus back in it. With `address`, the address to write to instead shows under it, cut in the middle when narrow, with a Copy.
 #[derive(IntoElement)]
 pub struct ContactSupport {
     id: ElementId,
@@ -116,13 +117,20 @@ impl RenderOnce for ContactSupport {
             .children(self.address.map(|address| {
                 div()
                     .flex()
-                    .flex_wrap()
                     .items_center()
-                    .gap_x_1()
+                    .gap_1()
                     .text_size(theme.text_size(TextSize::Sm))
                     .text_color(theme.colors.fg_muted)
-                    .child("Or write to")
-                    .child(literal(div()).text_color(theme.colors.fg).child(address))
+                    .child(div().flex_none().child("Or write to"))
+                    .child(
+                        literal(div())
+                            .debug_selector(|| "support-address".into())
+                            .flex_1()
+                            .min_w_0()
+                            .text_color(theme.colors.fg)
+                            .child(MiddleEllipsis::new(address.clone())),
+                    )
+                    .child(CopyButton::new((id.clone(), "copy-address"), address))
             }))
     }
 }

src/onboarding/tests/help.rs

@@ -48,16 +48,16 @@ fn panel(bench: &Bench, owner: Entity<Bench>) -> AnyElement {
         .into_any_element()
 }
 
-/// Stops on the list: close, the search, each article, then the foot; on an article: close, All articles, then the foot. An article opened by key takes focus to All articles, and the way back takes it to the search, where words narrow the list.
+/// Stops on the list: close, the search, the list, then the foot; on an article: close, All articles, then the foot. Enter on the list's cursor, on the first row at first, opens an article and takes focus to All articles, which the release leaves alone; the way back takes it to the search, where words narrow the list.
 #[gpui::test]
 fn an_article_opens_from_the_list_and_focus_follows(cx: &mut TestAppContext) {
     let (host, cx) = bench(panel, cx);
     tab(3, cx);
-    tap("space", cx);
+    tap("enter", cx);
     tap("space", cx);
     write("email", cx);
     tab(3, cx);
-    tap("space", cx);
+    tap("enter", cx);
     tab(3, cx);
     tap("space", cx);
     tab(4, cx);
@@ -77,6 +77,47 @@ fn an_article_opens_from_the_list_and_focus_follows(cx: &mut TestAppContext) {
     );
 }
 
+fn many(bench: &Bench, owner: Entity<Bench>) -> AnyElement {
+    let articles = (1..=12).map(|n| article(&format!("a{n}"), &format!("Article {n}"), "Words."));
+    let (opener, contact) = (owner.clone(), owner);
+    div()
+        .w(px(280.0))
+        .h(px(300.0))
+        .child(
+            HelpPanel::new("help", articles, &bench.search, bench.open.clone())
+                .on_open(move |key, _, cx| {
+                    let key = key.cloned();
+                    opener.update(cx, |bench, cx| {
+                        bench.said.push(format!(
+                            "open {}",
+                            key.as_ref().map_or("none", |key| key.as_ref())
+                        ));
+                        bench.open = key;
+                        cx.notify();
+                    })
+                })
+                .on_contact(move |_, cx| say(&contact, "contact".into(), cx)),
+        )
+        .into_any_element()
+}
+
+/// Twelve articles in a 300px panel scroll inside the list, so the foot stays in the box, and the cursor walks down to the eighth.
+#[gpui::test]
+fn a_long_list_scrolls_inside_the_panel(cx: &mut TestAppContext) {
+    let (host, cx) = bench(many, cx);
+    let foot = cx.debug_bounds("help-foot").expect("the foot");
+    assert!(
+        foot.bottom() <= px(316.0),
+        "the foot ends inside the 300px panel, which starts 16px down: {foot:?}"
+    );
+    tab(2, cx);
+    for _ in 0..7 {
+        tap("down", cx);
+    }
+    tap("enter", cx);
+    assert_eq!(said(&host, cx), ["open a8"]);
+}
+
 fn cheatsheet(bench: &Bench, owner: Entity<Bench>) -> AnyElement {
     let shortcut = |name: &str, keys: &str| Shortcut {
         group: "General".into(),
@@ -216,6 +257,30 @@ fn a_message_waits_while_busy(cx: &mut TestAppContext) {
     assert_eq!(said(&host, cx), ["bug: It froze"]);
 }
 
+fn long_address(_: &Bench, owner: Entity<Bench>) -> AnyElement {
+    div()
+        .w(px(280.0))
+        .child(
+            ContactSupport::new("support", [Choice::new("bug", "Something broke")])
+                .address("support-and-billing-questions@example-company.com")
+                .on_send(move |topic, message, _, cx| {
+                    say(&owner, format!("{topic}: {message}"), cx)
+                }),
+        )
+        .into_any_element()
+}
+
+/// A long address gives way in the middle, so it ends inside a 280px box, which starts 16px in.
+#[gpui::test]
+fn a_long_address_is_cut_inside_a_narrow_box(cx: &mut TestAppContext) {
+    let (_, cx) = bench(long_address, cx);
+    let address = cx.debug_bounds("support-address").expect("the address");
+    assert!(
+        address.right() <= px(296.0),
+        "the address ends inside the box: {address:?}"
+    );
+}
+
 fn inline(_: &Bench, owner: Entity<Bench>) -> AnyElement {
     InlineHelp::new("hint", "Only admins can change billing.")
         .on_more(move |_, cx| say(&owner, "more".into(), cx))