Onboarding · Flows and highlights: focus that holds as the flow turns, a tip that closes

navigation::Wizard keeps one focus handle for Next and Finish, so a
second press on the step before last finishes; Back that reaches the
first step, and a finished step picked by key, hand focus to Next.
Popover::with_opener hands its content a close that gives focus back:
Got it closes the hotspot's tip before the owner hides it, and the
event details close before Edit and Delete run, so focus never lands
on a panel that left the tree. The wizard's count and the setup
checklist's tally name their selectors after their words, and the bar
takes the count's own value.

src/onboarding/checklist.rs

@@ -82,6 +82,7 @@ impl RenderOnce for SetupChecklist {
             self.tasks.iter().filter(|task| task.done).count(),
             self.tasks.len(),
         );
+        let tally = format!("{done} of {all} done");
         let turns: Vec<usize> = self
             .tasks
             .iter()
@@ -203,11 +204,14 @@ impl RenderOnce for SetupChecklist {
                     )
                     .child(
                         tabular(div())
-                            .debug_selector(move || format!("setup-{done}-of-{all}"))
+                            .debug_selector({
+                                let tally = tally.clone();
+                                move || format!("setup-{tally}")
+                            })
                             .flex_none()
                             .text_size(theme.text_size(TextSize::Sm))
                             .text_color(theme.colors.fg_muted)
-                            .child(format!("{done} of {all} done")),
+                            .child(tally),
                     ),
             )
             .child(

src/onboarding/hotspot.rs

@@ -73,7 +73,7 @@ impl Hotspot {
         }
     }
 
-    /// Runs on Got it; the owner stops showing the hotspot.
+    /// Runs on Got it, once the tip has closed and handed focus back; the owner stops showing the hotspot.
     pub fn on_dismiss(mut self, handler: impl Fn(&mut Window, &mut App) + 'static) -> Self {
         self.on_dismiss = Some(Rc::new(handler));
         self
@@ -94,7 +94,7 @@ impl RenderOnce for Hotspot {
                 id: dot.into(),
                 toggle,
             },
-            move |_, cx| {
+            move |close: Run, _: &mut Window, cx: &mut App| {
                 let theme = cx.theme();
                 let prose =
                     |text: SharedString| div().flex().child(div().flex_1().min_w_0().child(text));
@@ -114,6 +114,7 @@ impl RenderOnce for Hotspot {
                                 .size(ControlSize::Sm)
                                 .on_click(move |_, window, cx| {
                                     log::info!("hotspot: dismissed");
+                                    close(window, cx);
                                     dismiss(window, cx)
                                 }),
                         ),

src/onboarding/tests.rs

@@ -1,23 +1,26 @@
 use gpui::{
-    AnyElement, App, Context, Entity, IntoElement, KeyBinding, ParentElement, Render, Styled,
-    TestAppContext, VisualTestContext, Window, div, px,
+    AnyElement, App, Context, Entity, FocusHandle, IntoElement, KeyBinding, ParentElement, Render,
+    Styled, TestAppContext, VisualTestContext, Window, div, px,
 };
 
 use super::{
     FeatureHighlight, Hotspot, OnboardingStep, OnboardingWizard, SetupChecklist, SetupTask,
 };
 use crate::{
+    buttons::Button,
     forms,
-    primitives::{FocusNext, IconName},
+    primitives::{FocusNext, FocusScope, IconName},
     theme::Theme,
 };
 
 type Part = fn(&Bench, Entity<Bench>) -> AnyElement;
 
-/// A view that shows one onboarding part, keeps what it heard, and holds the owner's step and readiness.
+/// A view that shows one onboarding part, keeps what it heard, and holds the owner's step, readiness and what it hid, with the root focus an app keeps.
 struct Bench {
     part: Part,
     said: Vec<String>,
+    root: FocusHandle,
+    hidden: bool,
     step: usize,
     ready: bool,
     all_done: bool,
@@ -39,9 +42,11 @@ fn bench(part: Part, cx: &mut TestAppContext) -> (Entity<Bench>, &mut VisualTest
         forms::bind_keys(cx);
         cx.bind_keys([KeyBinding::new("tab", FocusNext, None)]);
     });
-    let (host, cx) = cx.add_window_view(|_, _| Bench {
+    let (host, cx) = cx.add_window_view(|_, cx| Bench {
         part,
         said: Vec::new(),
+        root: cx.focus_handle(),
+        hidden: false,
         step: 0,
         ready: true,
         all_done: false,
@@ -120,13 +125,23 @@ fn wizard(bench: &Bench, owner: Entity<Bench>) -> AnyElement {
 #[gpui::test]
 fn the_wizard_walks_on_and_back_and_finishes(cx: &mut TestAppContext) {
     let (host, cx) = bench(wizard, cx);
-    assert!(cx.debug_bounds("onboarding-step-1-of-3").is_some());
+    assert!(cx.debug_bounds("onboarding-Step 1 of 3").is_some());
+    let (first, second) = (
+        cx.debug_bounds("progress-fill-0")
+            .expect("the bar's first part"),
+        cx.debug_bounds("progress-fill-1")
+            .expect("the bar's second part"),
+    );
+    assert!(
+        first.size.width > px(0.0) && second.size.width == px(0.0),
+        "the first of three parts is full: {first:?}, {second:?}"
+    );
     tab(2, cx);
     tap("space", cx);
     tab(2, cx);
     tap("space", cx);
     edit(&host, cx, |bench| bench.step = 2);
-    assert!(cx.debug_bounds("onboarding-step-3-of-3").is_some());
+    assert!(cx.debug_bounds("onboarding-Step 3 of 3").is_some());
     tab(3, cx);
     tap("space", cx);
     tab(1, cx);
@@ -144,6 +159,28 @@ fn next_rests_until_the_step_is_ready(cx: &mut TestAppContext) {
     assert_eq!(said(&host, cx), ["skip"]);
 }
 
+/// On the step before last, the stops are Skip, Back and Next: Next turns to Finish under focus, so a second press finishes.
+#[gpui::test]
+fn next_keeps_focus_as_it_turns_to_finish(cx: &mut TestAppContext) {
+    let (host, cx) = bench(wizard, cx);
+    edit(&host, cx, |bench| bench.step = 1);
+    tab(3, cx);
+    tap("space", cx);
+    tap("space", cx);
+    assert_eq!(said(&host, cx), ["step 2", "finish"]);
+}
+
+/// Back from the second step leaves Back at rest on the first, so focus moves to Next, and a press walks on again.
+#[gpui::test]
+fn back_to_the_first_step_hands_focus_to_next(cx: &mut TestAppContext) {
+    let (host, cx) = bench(wizard, cx);
+    edit(&host, cx, |bench| bench.step = 1);
+    tab(2, cx);
+    tap("space", cx);
+    tap("space", cx);
+    assert_eq!(said(&host, cx), ["step 0", "step 1"]);
+}
+
 fn hotspot(_: &Bench, owner: Entity<Bench>) -> AnyElement {
     Hotspot::new("pins", "Pin a view", "Keep it one press away.")
         .on_dismiss(move |_, cx| say(&owner, "seen".into(), cx))
@@ -172,6 +209,40 @@ fn a_hotspot_opens_its_tip_from_the_keyboard_and_is_dismissed(cx: &mut TestAppCo
     );
 }
 
+fn hidden_hotspot(bench: &Bench, owner: Entity<Bench>) -> AnyElement {
+    let after = owner.clone();
+    let spot = (!bench.hidden).then(|| {
+        Hotspot::new("pins", "Pin a view", "Keep it one press away.").on_dismiss(move |_, cx| {
+            owner.update(cx, |bench, cx| {
+                bench.said.push("seen".into());
+                bench.hidden = true;
+                cx.notify();
+            })
+        })
+    });
+    FocusScope::new(&bench.root)
+        .root()
+        .children(spot)
+        .child(
+            Button::new("after", "After").on_click(move |_, _, cx| say(&after, "after".into(), cx)),
+        )
+        .into_any_element()
+}
+
+/// Got it closes the tip before the owner hides the dot, so the root keeps focus and Tab reaches the next stop.
+#[gpui::test]
+fn after_got_it_tab_reaches_the_next_stop(cx: &mut TestAppContext) {
+    let (host, cx) = bench(hidden_hotspot, cx);
+    cx.update(|window, _| window.activate_window());
+    tab(1, cx);
+    tap("enter", cx);
+    tap("tab", cx);
+    tap("space", cx);
+    tap("tab", cx);
+    tap("space", cx);
+    assert_eq!(said(&host, cx), ["seen", "after"]);
+}
+
 fn highlight(_: &Bench, owner: Entity<Bench>) -> AnyElement {
     let later = owner.clone();
     FeatureHighlight::new(
@@ -222,13 +293,13 @@ fn checklist(bench: &Bench, owner: Entity<Bench>) -> AnyElement {
 #[gpui::test]
 fn a_setup_checklist_starts_what_is_left_and_hides_when_done(cx: &mut TestAppContext) {
     let (host, cx) = bench(checklist, cx);
-    assert!(cx.debug_bounds("setup-1-of-3").is_some());
+    assert!(cx.debug_bounds("setup-1 of 3 done").is_some());
     tab(2, cx);
     tap("space", cx);
     tab(3, cx);
     tap("space", cx);
     edit(&host, cx, |bench| bench.all_done = true);
-    assert!(cx.debug_bounds("setup-3-of-3").is_some());
+    assert!(cx.debug_bounds("setup-3 of 3 done").is_some());
     tab(1, cx);
     tap("space", cx);
     assert_eq!(said(&host, cx), ["start connect", "start invite", "hide"]);

src/onboarding/wizard.rs

@@ -106,6 +106,8 @@ impl RenderOnce for OnboardingWizard {
             .unwrap_or_else(|| panic!("onboarding wizard {id:?} has no on_finish"));
         let theme = cx.theme();
         let (current, count) = (self.current, self.steps.len());
+        let reached = current + 1;
+        let place = format!("Step {reached} of {count}");
         let skip = self.on_skip.map(|run| {
             Button::new((id.clone(), "skip"), "Skip for now")
                 .variant(ButtonVariant::Ghost)
@@ -123,10 +125,13 @@ impl RenderOnce for OnboardingWizard {
             .gap_2()
             .child(
                 div()
-                    .debug_selector(move || format!("onboarding-step-{}-of-{count}", current + 1))
+                    .debug_selector({
+                        let place = place.clone();
+                        move || format!("onboarding-{place}")
+                    })
                     .text_size(theme.text_size(TextSize::Sm))
                     .text_color(theme.colors.fg_muted)
-                    .child(format!("Step {} of {count}", current + 1)),
+                    .child(place),
             )
             .children(skip.map(|skip| div().flex_none().child(skip)));
         let step = &self.steps[current];
@@ -172,11 +177,8 @@ impl RenderOnce for OnboardingWizard {
             .gap_6()
             .child(
                 div().flex().flex_col().gap_2().child(head).child(
-                    ProgressBar::new(
-                        (id.clone(), "progress"),
-                        (current + 1) as f32 / count as f32,
-                    )
-                    .segments(count),
+                    ProgressBar::new((id.clone(), "progress"), reached as f32 / count as f32)
+                        .segments(count),
                 ),
             )
             .child(wizard)