From 8f00d2eabb22dd6225c3c825c214d3a110902011 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Tue, 6 Oct 2026 17:14:09 -0400 Subject: [PATCH] =?UTF-8?q?Panics=20=C2=B7=20P09=20Overlays:=20a=20lightbo?= =?UTF-8?q?x=20or=20tour=20past=20its=20last=20item=20closes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/overlays/lightbox.rs | 16 +++++++++------- src/overlays/spotlight.rs | 13 ++++++------- src/overlays/tests/guides.rs | 18 ++++++++++++++++++ tasks/panics.md | 3 ++- 4 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/overlays/lightbox.rs b/src/overlays/lightbox.rs index d192219..a20642c 100644 --- a/src/overlays/lightbox.rs +++ b/src/overlays/lightbox.rs @@ -96,15 +96,9 @@ impl Lightbox { at: usize, on_close: impl Fn(&mut Window, &mut App) + 'static, ) -> Self { - let slides: Vec = slides.into_iter().collect(); - assert!( - slides.is_empty() || at < slides.len(), - "slide {at} is past the last of {}", - slides.len() - ); Self { id: id.into(), - slides, + slides: slides.into_iter().collect(), at, on_step: None, on_close: Rc::new(on_close), @@ -140,6 +134,14 @@ impl RenderOnce for Lightbox { window.defer(cx, move |window, cx| close(window, cx)); return div().into_any_element(); } + if at >= count { + log::error!( + "lightbox {:?}: slide {at} of {count} is gone; closing", + self.id + ); + window.defer(cx, move |window, cx| close(window, cx)); + return div().into_any_element(); + } let step: OnStep = { let (id, on_step) = (self.id.clone(), self.on_step); Rc::new(move |to, window, cx| { diff --git a/src/overlays/spotlight.rs b/src/overlays/spotlight.rs index bc9af13..bd1be32 100644 --- a/src/overlays/spotlight.rs +++ b/src/overlays/spotlight.rs @@ -318,15 +318,9 @@ impl Tour { at: usize, on_close: impl Fn(&mut Window, &mut App) + 'static, ) -> Self { - let steps: Vec = steps.into_iter().collect(); - assert!( - at < steps.len(), - "step {at} is past the last of {}", - steps.len() - ); Self { id: id.into(), - steps, + steps: steps.into_iter().collect(), at, on_step: None, on_close: Rc::new(on_close), @@ -361,6 +355,11 @@ impl RenderOnce for Tour { } }) }; + if at >= count { + log::error!("tour {:?}: step {at} of {count} is gone; closing", self.id); + window.defer(cx, move |window, cx| close(window, cx)); + return div().into_any_element(); + } let (back, next) = (at.checked_sub(1), (at + 1 < count).then_some(at + 1)); let current = self.steps[at].clone(); let turn = motion::changes((self.id.clone(), "at"), at, window, cx); diff --git a/src/overlays/tests/guides.rs b/src/overlays/tests/guides.rs index 31ac193..b3fb1a2 100644 --- a/src/overlays/tests/guides.rs +++ b/src/overlays/tests/guides.rs @@ -352,3 +352,21 @@ fn left_from_back_keeps_the_tour_keys(cx: &mut TestAppContext) { press("right", cx); assert_eq!(state(&view, cx).1, 1); } + +#[gpui::test] +fn a_lightbox_or_tour_past_its_last_closes(cx: &mut TestAppContext) { + let (view, cx) = guides(cx); + for open in [Open::Lightbox, Open::Tour] { + view.update(cx, |guides, cx| { + guides.open = Some(open); + guides.at = 5; + cx.notify(); + }); + settle(cx); + assert_eq!( + view.read_with(cx, |guides, _| guides.open), + None, + "{open:?}" + ); + } +} diff --git a/tasks/panics.md b/tasks/panics.md index 5592e3b..babeccc 100644 --- a/tasks/panics.md +++ b/tasks/panics.md @@ -23,7 +23,7 @@ Each item reads every site in its folders, fixes what the rules name, updates ca - [x] P06 Forms — `src/forms` - [x] P07 Navigation — `src/navigation` - [x] P08 Menus — `src/menus` -- [ ] P09 Overlays — `src/overlays` +- [x] P09 Overlays — `src/overlays` - [ ] P10 Feedback — `src/feedback` - [ ] P11 Loading & Motion — `src/motion` - [ ] P12 Data Display — `src/data_display` @@ -73,3 +73,4 @@ One line per item: what changed, what stayed and why, review rounds. - P06: a root path shows whole; `TextInput::select` logs a range past the text or inside a character; the text element logs shaping and paint faults; a unit gone from its list logs; cron runs in the system zone go through `datetime`. Kept: empty choice lists (the owner disables, as AGENTS.md says), one-value contracts, facts by construction. Review: 2 rounds. - P07: `Wizard::new` takes `on_step` and `on_finish`; tabs, crumbs, history, steps, wizards and pagination with a place their list no longer holds log and mark none; an empty or all-disabled tab strip ignores arrows; a gone recent command is skipped. Kept: non-empty lists, disabled-free switcher and search items, row index parse. Review: 3 rounds. - P08: no change. Kept: a bar's menus and a pie's slices (one value each); the panel's level lookup, which `settle` fits to the menu before every draw. Review: 1 round. +- P09: a `Lightbox` or `Tour` past its last item logs and closes, handing focus back. Kept: dialog Enter without `focus_first` and popover face builders on an owner's opener (static builder misuse). Review: 1 round.