Panics · P37 Onboarding: handlers in new; gone articles, steps and topics hold

examples/gallery/pages/onboarding/flows.rs

@@ -88,13 +88,17 @@ pub fn wizard(window: &mut Window, cx: &mut App) -> impl IntoElement + use<> {
                     }),
             )
             .into_any_element(),
-        false => OnboardingWizard::new("onboarding-wizard", steps, step)
-            .content(content)
-            .ready(ready)
-            .on_step(move |to, _, cx| change(&walk, cx, |welcome| welcome.step = to))
-            .on_finish(move |_, cx| change(&finish, cx, |welcome| welcome.finished = true))
-            .on_skip(move |_, cx| change(&skip, cx, |welcome| welcome.finished = true))
-            .into_any_element(),
+        false => OnboardingWizard::new(
+            "onboarding-wizard",
+            steps,
+            step,
+            move |to, _, cx| change(&walk, cx, |welcome| welcome.step = to),
+            move |_, cx| change(&finish, cx, |welcome| welcome.finished = true),
+        )
+        .content(content)
+        .ready(ready)
+        .on_skip(move |_, cx| change(&skip, cx, |welcome| welcome.finished = true))
+        .into_any_element(),
     };
     section(
         "OnboardingWizard · WelcomeTour",
@@ -122,17 +126,15 @@ pub fn highlights(window: &mut Window, cx: &mut App) -> impl IntoElement + use<>
         .relative()
         .child(tool("onboarding-pin", IconName::Pin))
         .when(!spot_seen, |pin| {
-            pin.child(
-                div().absolute().top_neg_1p5().right_neg_1p5().child(probe(
+            pin.child(div().absolute().top_neg_1p5().right_neg_1p5().child(probe(
+                "onboarding-hotspot",
+                Hotspot::new(
                     "onboarding-hotspot",
-                    Hotspot::new(
-                        "onboarding-hotspot",
-                        "Pin a view",
-                        "Keep the views you use most one press away.",
-                    )
-                    .on_dismiss(move |_, cx| change(&spot, cx, |seen| seen.hotspot = true)),
-                )),
-            )
+                    "Pin a view",
+                    "Keep the views you use most one press away.",
+                    move |_, cx| change(&spot, cx, |seen| seen.hotspot = true),
+                ),
+            )))
         });
     let toolbar = div()
         .flex()
@@ -151,9 +153,9 @@ pub fn highlights(window: &mut Window, cx: &mut App) -> impl IntoElement + use<>
             IconName::Columns2,
             "Split view",
             "Work on two files side by side, each with its own history.",
+            |_, _| log::info!("gallery: split view tried"),
+            move |_, cx| change(&card, cx, |seen| seen.highlight = true),
         )
-        .on_try(|_, _| log::info!("gallery: split view tried"))
-        .on_dismiss(move |_, cx| change(&card, cx, |seen| seen.highlight = true))
     });
     let menu = List::new()
         .child(
@@ -247,16 +249,20 @@ pub fn checklist(window: &mut Window, cx: &mut App) -> impl IntoElement + use<>
                 })
             })
             .into_any_element(),
-        false => SetupChecklist::new("onboarding-setup", "Get started", tasks)
-            .on_start(move |key, _, cx| {
+        false => SetupChecklist::new(
+            "onboarding-setup",
+            "Get started",
+            tasks,
+            move |key, _, cx| {
                 let at = TASKS
                     .iter()
                     .position(|(each, _, _)| *each == key.as_ref())
                     .expect("a setup task");
                 change(&start, cx, |setup| setup.done[at] = true)
-            })
-            .on_dismiss(move |_, cx| change(&hide, cx, |setup| setup.hidden = true))
-            .into_any_element(),
+            },
+        )
+        .on_dismiss(move |_, cx| change(&hide, cx, |setup| setup.hidden = true))
+        .into_any_element(),
     };
     section(
         "Checklist (first-run tasks)",

examples/gallery/pages/onboarding/help.rs

@@ -176,16 +176,21 @@ pub fn panel(window: &mut Window, cx: &mut App) -> impl IntoElement + use<> {
             .border_color(theme.colors.border)
             .bg(theme.colors.surface)
             .child(
-                HelpPanel::new("onboarding-help-panel", articles(), &search, open)
-                    .on_open(move |key, _, cx| {
+                HelpPanel::new(
+                    "onboarding-help-panel",
+                    articles(),
+                    &search,
+                    open,
+                    move |key, _, cx| {
                         let key = key.cloned();
                         change(&opener, cx, |help| help.open = key)
-                    })
-                    .on_contact(|_, _| log::info!("gallery: contact support"))
-                    .on_shortcuts(move |_, cx| {
-                        change(&keys, cx, |help| help.dialog = Some(Dialog::Shortcuts))
-                    })
-                    .on_close(move |_, cx| change(&close, cx, |help| help.hidden = true)),
+                    },
+                )
+                .on_contact(|_, _| log::info!("gallery: contact support"))
+                .on_shortcuts(move |_, cx| {
+                    change(&keys, cx, |help| help.dialog = Some(Dialog::Shortcuts))
+                })
+                .on_close(move |_, cx| change(&close, cx, |help| help.hidden = true)),
             )
             .into_any_element(),
     };
@@ -290,8 +295,7 @@ pub fn feedback(window: &mut Window, cx: &mut App) -> impl IntoElement + use<> {
             .gap_6()
             .child(probe(
                 "onboarding-feedback",
-                FeedbackWidget::new("onboarding-feedback")
-                    .on_send(|sentiment, note, _, _| log::info!("gallery: feedback {} {note}", sentiment.key())),
+                FeedbackWidget::new("onboarding-feedback", |sentiment, note, _, _| log::info!("gallery: feedback {} {note}", sentiment.key())),
             ))
             .child(
                 div()
@@ -300,12 +304,11 @@ pub fn feedback(window: &mut Window, cx: &mut App) -> impl IntoElement + use<> {
                     .flex_col()
                     .gap_3()
                     .child(
-                        ContactSupport::new("onboarding-support", topics)
-                            .address("help@example.com")
-                            .on_send(move |topic, _, _, cx| {
+                        ContactSupport::new("onboarding-support", topics, move |topic, _, _, cx| {
                                 log::info!("gallery: support about {topic}");
                                 change(&done, cx, |help| help.sent = true)
-                            }),
+                            })
+                            .address("help@example.com"),
                     )
                     .when(sent, |column| column.child(div().text_sm().child("Sent. We answer within a day."))),
             ),

src/onboarding/checklist.rs

@@ -31,7 +31,7 @@ pub struct SetupChecklist {
     id: ElementId,
     title: SharedString,
     tasks: Vec<SetupTask>,
-    on_start: Option<OnKey>,
+    on_start: OnKey,
     on_dismiss: Option<Run>,
 }
 
@@ -40,6 +40,7 @@ impl SetupChecklist {
         id: impl Into<ElementId>,
         title: impl Into<SharedString>,
         tasks: impl IntoIterator<Item = SetupTask>,
+        on_start: impl Fn(&SharedString, &mut Window, &mut App) + 'static,
     ) -> Self {
         let tasks: Vec<SetupTask> = tasks.into_iter().collect();
         assert!(
@@ -51,20 +52,11 @@ impl SetupChecklist {
             id: id.into(),
             title: title.into(),
             tasks,
-            on_start: None,
+            on_start: Rc::new(on_start),
             on_dismiss: None,
         }
     }
 
-    /// Runs with the key of the task whose Start was pressed.
-    pub fn on_start(
-        mut self,
-        handler: impl Fn(&SharedString, &mut Window, &mut App) + 'static,
-    ) -> Self {
-        self.on_start = Some(Rc::new(handler));
-        self
-    }
-
     /// Shows Hide once every task is done.
     pub fn on_dismiss(mut self, handler: impl Fn(&mut Window, &mut App) + 'static) -> Self {
         self.on_dismiss = Some(Rc::new(handler));
@@ -75,9 +67,7 @@ impl SetupChecklist {
 impl RenderOnce for SetupChecklist {
     fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement {
         let id = self.id;
-        let on_start = self
-            .on_start
-            .unwrap_or_else(|| panic!("setup checklist {id:?} has no on_start"));
+        let on_start = self.on_start;
         let (done, all) = (
             self.tasks.iter().filter(|task| task.done).count(),
             self.tasks.len(),

src/onboarding/feedback.rs

@@ -73,33 +73,25 @@ struct Draft {
 #[derive(IntoElement)]
 pub struct FeedbackWidget {
     id: ElementId,
-    on_send: Option<OnFeedback>,
+    on_send: OnFeedback,
 }
 
 impl FeedbackWidget {
-    pub fn new(id: impl Into<ElementId>) -> Self {
+    pub fn new(
+        id: impl Into<ElementId>,
+        on_send: impl Fn(Sentiment, &str, &mut Window, &mut App) + 'static,
+    ) -> Self {
         Self {
             id: id.into(),
-            on_send: None,
+            on_send: Rc::new(on_send),
         }
     }
-
-    /// Runs with the face picked and the note, trimmed.
-    pub fn on_send(
-        mut self,
-        handler: impl Fn(Sentiment, &str, &mut Window, &mut App) + 'static,
-    ) -> Self {
-        self.on_send = Some(Rc::new(handler));
-        self
-    }
 }
 
 impl RenderOnce for FeedbackWidget {
     fn render(self, _: &mut Window, _: &mut App) -> impl IntoElement {
         let id = self.id;
-        let on_send = self
-            .on_send
-            .unwrap_or_else(|| panic!("feedback widget {id:?} has no on_send"));
+        let on_send = self.on_send;
         Popover::new(id.clone(), "Feedback", move |window, cx| {
             form(id, on_send, window, cx)
         })

src/onboarding/help.rs

@@ -60,7 +60,7 @@ pub struct HelpPanel {
     articles: Vec<HelpArticle>,
     search: Entity<TextInput>,
     open: Option<SharedString>,
-    on_open: Option<OnOpen>,
+    on_open: OnOpen,
     on_close: Option<Run>,
     on_contact: Option<Run>,
     on_shortcuts: Option<Run>,
@@ -73,35 +73,26 @@ impl HelpPanel {
         articles: impl IntoIterator<Item = HelpArticle>,
         search: &Entity<TextInput>,
         open: Option<SharedString>,
+        on_open: impl Fn(Option<&SharedString>, &mut Window, &mut App) + 'static,
     ) -> Self {
         let articles: Vec<HelpArticle> = articles.into_iter().collect();
-        if let Some(key) = &open {
-            assert!(
-                articles.iter().any(|article| &article.key == key),
-                "help panel: no article {key}"
-            );
+        if let Some(key) = &open
+            && !articles.iter().any(|article| &article.key == key)
+        {
+            log::error!("help panel: no article {key}; none open");
         }
         Self {
             id: id.into(),
             articles,
             search: search.clone(),
             open,
-            on_open: None,
+            on_open: Rc::new(on_open),
             on_close: None,
             on_contact: None,
             on_shortcuts: None,
         }
     }
 
-    /// Runs with the article to show, or none for the list.
-    pub fn on_open(
-        mut self,
-        handler: impl Fn(Option<&SharedString>, &mut Window, &mut App) + 'static,
-    ) -> Self {
-        self.on_open = Some(Rc::new(handler));
-        self
-    }
-
     /// Shows a close button.
     pub fn on_close(mut self, handler: impl Fn(&mut Window, &mut App) + 'static) -> Self {
         self.on_close = Some(Rc::new(handler));
@@ -142,9 +133,7 @@ fn foot_button(
 impl RenderOnce for HelpPanel {
     fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement {
         let id = self.id;
-        let on_open = self
-            .on_open
-            .unwrap_or_else(|| panic!("help panel {id:?} has no on_open"));
+        let on_open = self.on_open;
         let back_focus = tab_stop((id.clone(), "back").into(), true, window, cx);
         let back_handle = back_focus.clone();
         let search_focus = self.search.read(cx).focus().clone();

src/onboarding/highlight.rs

@@ -22,8 +22,8 @@ pub struct FeatureHighlight {
     icon: IconName,
     title: SharedString,
     body: SharedString,
-    on_try: Option<Run>,
-    on_dismiss: Option<Run>,
+    on_try: Run,
+    on_dismiss: Run,
 }
 
 impl FeatureHighlight {
@@ -32,38 +32,25 @@ impl FeatureHighlight {
         icon: IconName,
         title: impl Into<SharedString>,
         body: impl Into<SharedString>,
+        on_try: impl Fn(&mut Window, &mut App) + 'static,
+        on_dismiss: impl Fn(&mut Window, &mut App) + 'static,
     ) -> Self {
         Self {
             id: id.into(),
             icon,
             title: title.into(),
             body: body.into(),
-            on_try: None,
-            on_dismiss: None,
+            on_try: Rc::new(on_try),
+            on_dismiss: Rc::new(on_dismiss),
         }
     }
-
-    pub fn on_try(mut self, handler: impl Fn(&mut Window, &mut App) + 'static) -> Self {
-        self.on_try = Some(Rc::new(handler));
-        self
-    }
-
-    /// Runs on Not now; the owner stops showing it.
-    pub fn on_dismiss(mut self, handler: impl Fn(&mut Window, &mut App) + 'static) -> Self {
-        self.on_dismiss = Some(Rc::new(handler));
-        self
-    }
 }
 
 impl RenderOnce for FeatureHighlight {
     fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement {
         let id = self.id;
-        let try_it = self
-            .on_try
-            .unwrap_or_else(|| panic!("feature highlight {id:?} has no on_try"));
-        let dismiss = self
-            .on_dismiss
-            .unwrap_or_else(|| panic!("feature highlight {id:?} has no on_dismiss"));
+        let try_it = self.on_try;
+        let dismiss = self.on_dismiss;
         let theme = cx.theme();
         let title = self.title.clone();
         let actions = div()

src/onboarding/hotspot.rs

@@ -56,7 +56,7 @@ pub struct Hotspot {
     id: ElementId,
     title: SharedString,
     body: SharedString,
-    on_dismiss: Option<Run>,
+    on_dismiss: Run,
 }
 
 impl Hotspot {
@@ -64,28 +64,21 @@ impl Hotspot {
         id: impl Into<ElementId>,
         title: impl Into<SharedString>,
         body: impl Into<SharedString>,
+        on_dismiss: impl Fn(&mut Window, &mut App) + 'static,
     ) -> Self {
         Self {
             id: id.into(),
             title: title.into(),
             body: body.into(),
-            on_dismiss: None,
+            on_dismiss: Rc::new(on_dismiss),
         }
     }
-
-    /// 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
-    }
 }
 
 impl RenderOnce for Hotspot {
     fn render(self, _: &mut Window, _: &mut App) -> impl IntoElement {
         let id = self.id;
-        let dismiss = self
-            .on_dismiss
-            .unwrap_or_else(|| panic!("hotspot {id:?} has no on_dismiss"));
+        let dismiss = self.on_dismiss;
         let (dot, got_it) = ((id.clone(), "dot"), (id.clone(), "got-it"));
         let (title, body) = (self.title, self.body);
         Popover::with_opener(

src/onboarding/support.rs

@@ -22,11 +22,15 @@ pub struct ContactSupport {
     topics: Vec<Choice>,
     busy: bool,
     address: Option<SharedString>,
-    on_send: Option<OnMessage>,
+    on_send: OnMessage,
 }
 
 impl ContactSupport {
-    pub fn new(id: impl Into<ElementId>, topics: impl IntoIterator<Item = Choice>) -> Self {
+    pub fn new(
+        id: impl Into<ElementId>,
+        topics: impl IntoIterator<Item = Choice>,
+        on_send: impl Fn(&SharedString, &str, &mut Window, &mut App) + 'static,
+    ) -> Self {
         let topics: Vec<Choice> = topics.into_iter().collect();
         assert!(!topics.is_empty(), "contact support needs a topic");
         Self {
@@ -34,7 +38,7 @@ impl ContactSupport {
             topics,
             busy: false,
             address: None,
-            on_send: None,
+            on_send: Rc::new(on_send),
         }
     }
 
@@ -49,30 +53,27 @@ impl ContactSupport {
         self.address = Some(address.into());
         self
     }
-
-    /// Runs with the topic's key and the message, trimmed.
-    pub fn on_send(
-        mut self,
-        handler: impl Fn(&SharedString, &str, &mut Window, &mut App) + 'static,
-    ) -> Self {
-        self.on_send = Some(Rc::new(handler));
-        self
-    }
 }
 
 impl RenderOnce for ContactSupport {
     fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement {
         let id = self.id;
-        let on_send = self
-            .on_send
-            .unwrap_or_else(|| panic!("contact support {id:?} has no on_send"));
+        let on_send = self.on_send;
         let topic = window.use_keyed_state((id.clone(), "topic"), cx, |_, _| None::<SharedString>);
         let message = window.use_keyed_state((id.clone(), "message"), cx, |window, cx| {
             TextInput::new(window, cx)
                 .multi_line(4, 8)
                 .placeholder("What happened, and what you expected")
         });
-        let chosen = topic.read(cx).clone();
+        let chosen = topic.read(cx).clone().filter(|key| {
+            let offered = self.topics.iter().any(|choice| choice.value == *key);
+            if !offered {
+                log::error!(
+                    "contact support {id:?}: topic {key} is no longer offered; none chosen"
+                );
+            }
+            offered
+        });
         let words = message.read(cx).text().trim().to_string();
         let ready = chosen.is_some() && !words.is_empty();
         let theme = cx.theme();

src/onboarding/tests/flows.rs

@@ -1,6 +1,6 @@
 use gpui::{AnyElement, Entity, IntoElement, ParentElement, TestAppContext, div, px};
 
-use super::{Bench, bench, edit, said, say, tab, tap};
+use super::{Bench, bench, edit, said, say, settle, tab, tap};
 use crate::{
     buttons::Button,
     onboarding::{
@@ -16,19 +16,34 @@ fn wizard(bench: &Bench, owner: Entity<Bench>) -> AnyElement {
         body: "A line on it".into(),
     });
     let [stepper, finisher, skipper] = [(); 3].map(|_| owner.clone());
-    OnboardingWizard::new("welcome", steps, bench.step)
-        .ready(bench.ready)
-        .content(div().child("Fields"))
-        .on_step(move |to, _, cx| {
+    OnboardingWizard::new(
+        "welcome",
+        steps,
+        bench.step,
+        move |to, _, cx| {
             stepper.update(cx, |bench, cx| {
                 bench.step = to;
                 bench.said.push(format!("step {to}"));
                 cx.notify();
             })
-        })
-        .on_finish(move |_, cx| say(&finisher, "finish".into(), cx))
-        .on_skip(move |_, cx| say(&skipper, "skip".into(), cx))
-        .into_any_element()
+        },
+        move |_, cx| say(&finisher, "finish".into(), cx),
+    )
+    .ready(bench.ready)
+    .content(div().child("Fields"))
+    .on_skip(move |_, cx| say(&skipper, "skip".into(), cx))
+    .into_any_element()
+}
+
+#[gpui::test]
+fn a_step_past_the_last_shows_none(cx: &mut TestAppContext) {
+    let (host, cx) = bench(wizard, cx);
+    host.update(cx, |bench, cx| {
+        bench.step = 9;
+        cx.notify();
+    });
+    settle(cx);
+    assert!(cx.debug_bounds("onboarding-No step of 3").is_some());
 }
 
 /// Stops: Skip, then Next on the first step; Skip, Back, then Next or Finish after it.
@@ -92,9 +107,13 @@ fn back_to_the_first_step_hands_focus_to_next(cx: &mut TestAppContext) {
 }
 
 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))
-        .into_any_element()
+    Hotspot::new(
+        "pins",
+        "Pin a view",
+        "Keep it one press away.",
+        move |_, cx| say(&owner, "seen".into(), cx),
+    )
+    .into_any_element()
 }
 
 /// Stops: the dot; Enter opens the tip, where Got it is the one stop.
@@ -122,13 +141,18 @@ 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();
-            })
-        })
+        Hotspot::new(
+            "pins",
+            "Pin a view",
+            "Keep it one press away.",
+            move |_, cx| {
+                owner.update(cx, |bench, cx| {
+                    bench.said.push("seen".into());
+                    bench.hidden = true;
+                    cx.notify();
+                })
+            },
+        )
     });
     FocusScope::new(&bench.root)
         .root()
@@ -160,9 +184,9 @@ fn highlight(_: &Bench, owner: Entity<Bench>) -> AnyElement {
         IconName::Columns2,
         "Split view",
         "Work on two files side by side.",
+        move |_, cx| say(&owner, "try".into(), cx),
+        move |_, cx| say(&later, "later".into(), cx),
     )
-    .on_try(move |_, cx| say(&owner, "try".into(), cx))
-    .on_dismiss(move |_, cx| say(&later, "later".into(), cx))
     .into_any_element()
 }
 
@@ -193,8 +217,8 @@ fn checklist(bench: &Bench, owner: Entity<Bench>) -> AnyElement {
             task("invite", false),
             task("connect", false),
         ],
+        move |key, _, cx| say(&owner, format!("start {key}"), cx),
     )
-    .on_start(move |key, _, cx| say(&owner, format!("start {key}"), cx))
     .on_dismiss(move |_, cx| say(&hidden, "hide".into(), cx))
     .into_any_element()
 }

src/onboarding/tests/help.rs

@@ -29,8 +29,12 @@ fn panel(bench: &Bench, owner: Entity<Bench>) -> AnyElement {
     div()
         .h(px(420.0))
         .child(
-            HelpPanel::new("help", articles, &bench.search, bench.open.clone())
-                .on_open(move |key, _, cx| {
+            HelpPanel::new(
+                "help",
+                articles,
+                &bench.search,
+                bench.open.clone(),
+                move |key, _, cx| {
                     let key = key.cloned();
                     opener.update(cx, |bench, cx| {
                         bench.said.push(format!(
@@ -40,10 +44,11 @@ fn panel(bench: &Bench, owner: Entity<Bench>) -> AnyElement {
                         bench.open = key;
                         cx.notify();
                     })
-                })
-                .on_contact(move |_, cx| say(&contact, "contact".into(), cx))
-                .on_shortcuts(move |_, cx| say(&keys, "shortcuts".into(), cx))
-                .on_close(move |_, cx| say(&close, "close".into(), cx)),
+                },
+            )
+            .on_contact(move |_, cx| say(&contact, "contact".into(), cx))
+            .on_shortcuts(move |_, cx| say(&keys, "shortcuts".into(), cx))
+            .on_close(move |_, cx| say(&close, "close".into(), cx)),
         )
         .into_any_element()
 }
@@ -84,8 +89,12 @@ fn many(bench: &Bench, owner: Entity<Bench>) -> AnyElement {
         .w(px(280.0))
         .h(px(300.0))
         .child(
-            HelpPanel::new("help", articles, &bench.search, bench.open.clone())
-                .on_open(move |key, _, cx| {
+            HelpPanel::new(
+                "help",
+                articles,
+                &bench.search,
+                bench.open.clone(),
+                move |key, _, cx| {
                     let key = key.cloned();
                     opener.update(cx, |bench, cx| {
                         bench.said.push(format!(
@@ -95,8 +104,9 @@ fn many(bench: &Bench, owner: Entity<Bench>) -> AnyElement {
                         bench.open = key;
                         cx.notify();
                     })
-                })
-                .on_contact(move |_, cx| say(&contact, "contact".into(), cx)),
+                },
+            )
+            .on_contact(move |_, cx| say(&contact, "contact".into(), cx)),
         )
         .into_any_element()
 }
@@ -169,11 +179,10 @@ fn whats_new_lists_the_releases_and_got_it_closes(cx: &mut TestAppContext) {
 }
 
 fn feedback(_: &Bench, owner: Entity<Bench>) -> AnyElement {
-    FeedbackWidget::new("feedback")
-        .on_send(move |sentiment, note, _, cx| {
-            say(&owner, format!("{} {note}", sentiment.key()), cx)
-        })
-        .into_any_element()
+    FeedbackWidget::new("feedback", move |sentiment, note, _, cx| {
+        say(&owner, format!("{} {note}", sentiment.key()), cx)
+    })
+    .into_any_element()
 }
 
 /// Stops in the form: the four faces, the note, then Send once a face is picked.
@@ -207,11 +216,12 @@ fn support(bench: &Bench, owner: Entity<Bench>) -> AnyElement {
         Choice::new("billing", "Billing"),
         Choice::new("bug", "Something broke"),
     ];
-    ContactSupport::new("support", topics)
-        .busy(bench.busy)
-        .address("help@example.com")
-        .on_send(move |topic, message, _, cx| say(&owner, format!("{topic}: {message}"), cx))
-        .into_any_element()
+    ContactSupport::new("support", topics, move |topic, message, _, cx| {
+        say(&owner, format!("{topic}: {message}"), cx)
+    })
+    .busy(bench.busy)
+    .address("help@example.com")
+    .into_any_element()
 }
 
 /// Stops: the topic, the message, then Send once both are given; the message empties once sent, with focus back in it.
@@ -261,11 +271,12 @@ 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)
-                }),
+            ContactSupport::new(
+                "support",
+                [Choice::new("bug", "Something broke")],
+                move |topic, message, _, cx| say(&owner, format!("{topic}: {message}"), cx),
+            )
+            .address("support-and-billing-questions@example-company.com"),
         )
         .into_any_element()
 }
@@ -295,3 +306,40 @@ fn inline_help_offers_more(cx: &mut TestAppContext) {
     tap("space", cx);
     assert_eq!(said(&host, cx), ["more"]);
 }
+
+#[gpui::test]
+fn an_article_that_left_draws(cx: &mut TestAppContext) {
+    let (host, cx) = bench(panel, cx);
+    host.update(cx, |bench, cx| {
+        bench.open = Some("gone".into());
+        cx.notify();
+    });
+    cx.run_until_parked();
+}
+
+/// Support topics, the second leaving once the bench hides it.
+fn dynamic_topics(bench: &Bench, owner: Entity<Bench>) -> AnyElement {
+    let mut topics = vec![Choice::new("billing", "Billing")];
+    if !bench.hidden {
+        topics.push(Choice::new("bug", "Something broke"));
+    }
+    ContactSupport::new("support", topics, move |topic, message, _, cx| {
+        say(&owner, format!("{topic}: {message}"), cx)
+    })
+    .into_any_element()
+}
+
+#[gpui::test]
+fn a_topic_that_left_sends_nothing(cx: &mut TestAppContext) {
+    let (host, cx) = bench(dynamic_topics, cx);
+    tab(1, cx);
+    for key in ["enter", "down", "enter"] {
+        tap(key, cx);
+    }
+    tab(2, cx);
+    write("It froze", cx);
+    edit(&host, cx, |bench| bench.hidden = true);
+    tab(3, cx);
+    tap("space", cx);
+    assert_eq!(said(&host, cx), Vec::<String>::new());
+}

src/onboarding/wizard.rs

@@ -31,8 +31,8 @@ pub struct OnboardingWizard {
     current: usize,
     content: Option<AnyElement>,
     ready: bool,
-    on_step: Option<OnStep>,
-    on_finish: Option<Run>,
+    on_step: OnStep,
+    on_finish: Run,
     on_skip: Option<Run>,
 }
 
@@ -41,6 +41,8 @@ impl OnboardingWizard {
         id: impl Into<ElementId>,
         steps: impl IntoIterator<Item = OnboardingStep>,
         current: usize,
+        on_step: impl Fn(usize, &mut Window, &mut App) + 'static,
+        on_finish: impl Fn(&mut Window, &mut App) + 'static,
     ) -> Self {
         let steps: Vec<OnboardingStep> = steps.into_iter().collect();
         assert!(
@@ -48,19 +50,14 @@ impl OnboardingWizard {
             "an onboarding wizard needs two steps, got {}",
             steps.len()
         );
-        assert!(
-            current < steps.len(),
-            "step {current} is past the last of {}",
-            steps.len()
-        );
         Self {
             id: id.into(),
             steps,
             current,
             content: None,
             ready: true,
-            on_step: None,
-            on_finish: None,
+            on_step: Rc::new(on_step),
+            on_finish: Rc::new(on_finish),
             on_skip: None,
         }
     }
@@ -77,17 +74,6 @@ impl OnboardingWizard {
         self
     }
 
-    /// Runs with the step Back or Next leads to.
-    pub fn on_step(mut self, handler: impl Fn(usize, &mut Window, &mut App) + 'static) -> Self {
-        self.on_step = Some(Rc::new(handler));
-        self
-    }
-
-    pub fn on_finish(mut self, handler: impl Fn(&mut Window, &mut App) + 'static) -> Self {
-        self.on_finish = Some(Rc::new(handler));
-        self
-    }
-
     /// Shows Skip for now.
     pub fn on_skip(mut self, handler: impl Fn(&mut Window, &mut App) + 'static) -> Self {
         self.on_skip = Some(Rc::new(handler));
@@ -98,16 +84,19 @@ impl OnboardingWizard {
 impl RenderOnce for OnboardingWizard {
     fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement {
         let id = self.id;
-        let on_step = self
-            .on_step
-            .unwrap_or_else(|| panic!("onboarding wizard {id:?} has no on_step"));
-        let on_finish = self
-            .on_finish
-            .unwrap_or_else(|| panic!("onboarding wizard {id:?} has no on_finish"));
+        let on_step = self.on_step;
+        let on_finish = self.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 step = self.steps.get(current);
+        if step.is_none() {
+            log::error!("onboarding wizard {id:?}: step {current} of {count}; none current");
+        }
+        let reached = step.map_or(0, |_| current + 1);
+        let place = match step {
+            Some(_) => format!("Step {reached} of {count}"),
+            None => format!("No step of {count}"),
+        };
         let skip = self.on_skip.map(|run| {
             Button::new((id.clone(), "skip"), "Skip for now")
                 .variant(ButtonVariant::Ghost)
@@ -134,24 +123,25 @@ impl RenderOnce for OnboardingWizard {
                     .child(place),
             )
             .children(skip.map(|skip| div().flex_none().child(skip)));
-        let step = &self.steps[current];
-        let words = div().flex().child(
-            div()
-                .flex_1()
-                .min_w_0()
-                .child(
-                    div()
-                        .text_size(theme.text_size(TextSize::Lg))
-                        .font_weight(FontWeight::SEMIBOLD)
-                        .child(step.title.clone()),
-                )
-                .child(
-                    div()
-                        .mt_1()
-                        .text_color(theme.colors.fg_muted)
-                        .child(step.body.clone()),
-                ),
-        );
+        let words = step.map(|step| {
+            div().flex().child(
+                div()
+                    .flex_1()
+                    .min_w_0()
+                    .child(
+                        div()
+                            .text_size(theme.text_size(TextSize::Lg))
+                            .font_weight(FontWeight::SEMIBOLD)
+                            .child(step.title.clone()),
+                    )
+                    .child(
+                        div()
+                            .mt_1()
+                            .text_color(theme.colors.fg_muted)
+                            .child(step.body.clone()),
+                    ),
+            )
+        });
         let wizard = Wizard::new(
             (id.clone(), "wizard"),
             self.steps
@@ -168,7 +158,7 @@ impl RenderOnce for OnboardingWizard {
                 .flex()
                 .flex_col()
                 .gap_5()
-                .child(words)
+                .children(words)
                 .children(self.content),
         );
         div()

tasks/panics.md

@@ -51,7 +51,7 @@ Each item reads every site in its folders, fixes what the rules name, updates ca
 - [x] P34 Dashboard — `src/dashboard`
 - [x] P35 Settings — `src/settings`
 - [x] P36 Account — `src/account`
-- [ ] P37 Onboarding — `src/onboarding`
+- [x] P37 Onboarding — `src/onboarding`
 - [ ] P38 Interaction — `src/interaction`
 - [ ] P39 Theme — `src/theme`
 - [ ] P40 i18n & a11y — `src/i18n`
@@ -102,3 +102,4 @@ One line per item: what changed, what stayed and why, review rounds.
 - P34: the refresh interval selector, filter bar and dashboard grid take their handlers in `new`; `tiles::arranged` and `stepped` return `Option` for a tile gone from the grid, so a landing or step after the owner removed it moves nothing; tiles without cards and cards without tiles stay out. Kept: columns, cells, intervals offered, services and uptimes, lookups from one render. Review: 1 round.
 - P35: eighteen settings components take their handlers in `new`; a section or syntax theme not offered marks none; a font size or default outside its range pegs. Kept: options offered by construction, the proxy's Apply behind its read, drops that bring a file. Review: 1 round.
 - P36: seventeen account components take their handlers in `new`, the email step's `on_send` with them; a busy provider, a current account or workspace, and a role no longer listed mark none and log; an invitation waits for its role to be offered; an expiry year that overflows reads as unread. Kept: quotas, providers, one current session, roles to invite as, the card behind Save. Review: 2 rounds.
+- P37: seven onboarding components take their handlers in `new`, OnboardingWizard's `on_step` and `on_finish` among them; a help article no longer listed opens none; a wizard step past its last says so; a support topic no longer offered counts as none and holds Send. Kept: tasks, steps, releases and topics present, sentiments by construction, Send behind a pick. Review: 2 rounds.