ZacharyZhang-NY/Ely-GPUI-Components
1
1 Fork 0
Code Issues Pull Requests Actions 11 Packages Projects Releases Wiki Activity

Panics · P07 Navigation: Wizard takes its handlers; stale places in tabs, crumbs, history, steps and pages mark none

Browse Source
This commit is contained in:
ZacharyZhang-NY committed 2026-10-06 17:06:25 -04:00
1 parent e2d880fb4f
commit 919f26fcc6
14 files changed
+261 -83
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

examples/gallery/pages/navigation/paths.rs
+8 -4
View File
@@ -108,10 +108,14 @@ pub fn steps(window: &mut Window, cx: &mut App) -> impl IntoElement + use<> {
probe(
"wizard",
div().w(px(560.0)).child(
Wizard::new("wizard", setup_steps(), now)
.content(div().h(px(72.0)).child(Paragraph::new(body)))
.on_step(move |to, _, cx| set(&walk, to, cx))
.on_finish(move |_, cx| set(&finish, true, cx)),
Wizard::new(
"wizard",
setup_steps(),
now,
move |to, _, cx| set(&walk, to, cx),
move |_, cx| set(&finish, true, cx),
)
.content(div().h(px(72.0)).child(Paragraph::new(body))),
),
),
cx,
src/navigation/breadcrumb.rs
+10 -7
View File
@@ -38,13 +38,16 @@ impl Crumb {
/// The level's neighbors, itself among them; a click lists them.
pub fn siblings(mut self, siblings: impl IntoIterator<Item = Choice>) -> Self {
self.siblings = siblings.into_iter().collect();
assert!(
self.siblings
.iter()
.any(|sibling| sibling.value == self.value),
"crumb {} is not among its siblings",
self.value
);
if !self
.siblings
.iter()
.any(|sibling| sibling.value == self.value)
{
log::error!(
"crumb {} is not among its siblings; none marked",
self.value
);
}
self
}
}
src/navigation/history.rs
+11 -7
View File
@@ -29,11 +29,7 @@ impl BackForwardNavigation {
current: usize,
) -> Self {
let places: Vec<Choice> = places.into_iter().collect();
assert!(
current < places.len(),
"place {current} is past the last of {}",
places.len()
);
assert!(!places.is_empty(), "history has no places");
Self {
id: id.into(),
places,
@@ -52,6 +48,14 @@ impl BackForwardNavigation {
impl RenderOnce for BackForwardNavigation {
fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement {
let (current, last) = (self.current, self.places.len() - 1);
let valid = current <= last;
if !valid {
log::error!(
"history {:?}: place {current} of {}; none marked",
self.id,
last + 1
);
}
let go: OnGo = {
let (id, on_go) = (self.id.clone(), self.on_go);
Rc::new(move |to, window, cx| {
@@ -96,7 +100,7 @@ impl RenderOnce for BackForwardNavigation {
.size(ControlSize::Sm)
.variant(ButtonVariant::Ghost)
.tooltip("Back")
.disabled(current == 0)
.disabled(!valid || current == 0)
.on_click(move |_, window, cx| back(current - 1, window, cx)),
)
.child(
@@ -104,7 +108,7 @@ impl RenderOnce for BackForwardNavigation {
.size(ControlSize::Sm)
.variant(ButtonVariant::Ghost)
.tooltip("Forward")
.disabled(current == last)
.disabled(!valid || current == last)
.on_click(move |_, window, cx| ahead(current + 1, window, cx)),
)
.child(list)
src/navigation/pages.rs
+16 -4
View File
@@ -94,7 +94,19 @@ impl Pagination {
impl RenderOnce for Pagination {
fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement {
let list = page_list(self.page, self.pages, self.near);
let valid = (1..=self.pages).contains(&self.page);
if !valid {
log::error!(
"pagination {:?}: page {} of {}; none marked",
self.id,
self.page,
self.pages
);
}
let list = match self.pages {
0 => Vec::new(),
pages => page_list(self.page.clamp(1, pages), pages, self.near),
};
let (page, pages) = (self.page, self.pages);
let go = {
let (id, on_change) = (self.id.clone(), self.on_change);
@@ -113,7 +125,7 @@ impl RenderOnce for Pagination {
);
let (back, ahead, jump) = (go.clone(), go.clone(), go.clone());
let to = i18n::text(cx, "pagination.jump", &[]);
let jump = self.jump.then(|| {
let jump = (self.jump && pages > 0).then(|| {
let words = div()
.whitespace_nowrap()
.text_size(small)
@@ -177,7 +189,7 @@ impl RenderOnce for Pagination {
.size(size)
.variant(ButtonVariant::Ghost)
.tooltip(i18n::text(cx, "pagination.previous", &[]))
.disabled(page == 1)
.disabled(!valid || page == 1)
.on_click(move |_, window, cx| back(page - 1, window, cx)),
),
)
@@ -188,7 +200,7 @@ impl RenderOnce for Pagination {
.size(size)
.variant(ButtonVariant::Ghost)
.tooltip(i18n::text(cx, "pagination.next", &[]))
.disabled(page == pages)
.disabled(!valid || page == pages)
.on_click(move |_, window, cx| ahead(page + 1, window, cx)),
),
)
src/navigation/palette/kinds.rs
+8 -4
View File
@@ -130,18 +130,22 @@ impl RenderOnce for CommandPalette {
let query = input.read(cx).text().trim().to_string();
let groups = if query.is_empty() {
let find = |value: &SharedString| {
self.groups
let found = self
.groups
.iter()
.flat_map(|(_, commands)| commands)
.find(|command| command.value == *value)
.unwrap_or_else(|| panic!("recent command {value} is not in the palette"))
.find(|command| command.value == *value);
if found.is_none() {
log::error!("command palette {:?}: recent {value} is gone", self.id);
}
found
};
let recent = Group {
title: Some(i18n::text(cx, "palette.recent", &[])),
rows: self
.recent
.iter()
.map(|value| find(value).row(Vec::new(), cx))
.filter_map(|value| Some(find(value)?.row(Vec::new(), cx)))
.collect(),
};
iter::once(recent)
src/navigation/palette/tests.rs
+1 -1
View File
@@ -120,7 +120,7 @@ impl Render for Host {
Command::new("light", "Light theme"),
],
)
.recent(["dark"])
.recent(["dark", "retired"])
.on_run(got)
.into_any_element(),
Kind::Files => QuickOpen::new(
src/navigation/steps.rs
+26 -39
View File
@@ -33,15 +33,9 @@ impl Steps {
steps: impl IntoIterator<Item = Choice>,
current: usize,
) -> Self {
let steps: Vec<Choice> = steps.into_iter().collect();
assert!(
current < steps.len(),
"step {current} is past the last of {}",
steps.len()
);
Self {
id: id.into(),
steps,
steps: steps.into_iter().collect(),
current,
vertical: false,
on_select: None,
@@ -65,12 +59,16 @@ impl RenderOnce for Steps {
let id = self.id.clone();
let turn = motion::changes((id.clone(), "current"), self.current, window, cx);
let (current, vertical, count) = (self.current, self.vertical, self.steps.len());
let valid = current < count;
if !valid {
log::error!("steps {id:?}: step {current} of {count}; none marked");
}
let duration = motion::duration(motion::SLOW, cx);
let theme = cx.theme();
let colors = &theme.colors;
let mark = theme.control_height(ControlSize::Sm);
let line = |ix: usize| {
let filled = ix < current;
let filled = valid && ix < current;
let fill = div().bg(colors.accent).map(|fill| {
if vertical {
fill.w_full()
@@ -102,7 +100,7 @@ impl RenderOnce for Steps {
.child(fill)
};
let steps = self.steps.into_iter().enumerate().map(|(ix, step)| {
let (done, now) = (ix < current, ix == current);
let (done, now) = (valid && ix < current, valid && ix == current);
let circle = div()
.flex()
.flex_none()
@@ -217,31 +215,28 @@ pub struct Wizard {
content: Option<AnyElement>,
ready: bool,
headless: bool,
on_step: Option<OnStep>,
on_finish: Option<Run>,
on_step: OnStep,
on_finish: Run,
}
impl Wizard {
/// `on_step` runs with the step Back, Next or a finished step leads to; `on_finish` with Finish.
pub fn new(
id: impl Into<ElementId>,
steps: impl IntoIterator<Item = Choice>,
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<Choice> = steps.into_iter().collect();
assert!(
current < steps.len(),
"step {current} is past the last of {}",
steps.len()
);
Self {
id: id.into(),
steps,
steps: steps.into_iter().collect(),
current,
content: None,
ready: true,
headless: false,
on_step: None,
on_finish: None,
on_step: Rc::new(on_step),
on_finish: Rc::new(on_finish),
}
}
@@ -262,30 +257,22 @@ impl Wizard {
self.ready = ready;
self
}
/// Runs with the step Back, Next or a finished step 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
}
}
impl RenderOnce for Wizard {
fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement {
let (current, last) = (self.current, self.steps.len().saturating_sub(1));
let valid = current < self.steps.len();
if !valid {
log::error!(
"wizard {:?}: step {current} of {}",
self.id,
self.steps.len()
);
}
let turn = motion::changes((self.id.clone(), "content"), current, window, cx);
let id = self.id.clone();
let on_step = self
.on_step
.unwrap_or_else(|| panic!("wizard {id:?} has no on_step"));
let on_finish = self
.on_finish
.unwrap_or_else(|| panic!("wizard {id:?} has no on_finish"));
let (on_step, on_finish) = (self.on_step, self.on_finish);
let step: OnStep = {
let id = id.clone();
Rc::new(move |to, window, cx| {
@@ -322,7 +309,7 @@ impl RenderOnce for Wizard {
if current == last { "Finish" } else { "Next" },
)
.primary()
.disabled(!self.ready)
.disabled(!valid || !self.ready)
.focus_handle(&advance)
.on_click(move |_, window, cx| match current == last {
true => finish(window, cx),
@@ -352,7 +339,7 @@ impl RenderOnce for Wizard {
.child(
Button::new((id.clone(), "back"), "Back")
.variant(ButtonVariant::Ghost)
.disabled(current == 0)
.disabled(!valid || current == 0)
.on_click(move |_, window, cx| {
if current == 1 {
window.focus(&to_advance, cx);
src/navigation/tabs.rs
+12 -8
View File
@@ -88,10 +88,7 @@ impl Tabs {
impl RenderOnce for Tabs {
fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement {
let values: Vec<SharedString> = self.tabs.iter().map(|tab| tab.value.clone()).collect();
let chosen = values
.iter()
.position(|value| *value == self.selected)
.unwrap_or_else(|| panic!("tabs {:?} have no tab {}", self.id, self.selected));
let chosen = values.iter().position(|value| *value == self.selected);
let placement = self.placement;
let axis = match placement {
TabPlacement::Left => Axis::Vertical,
@@ -155,7 +152,7 @@ impl RenderOnce for Tabs {
.iter()
.enumerate()
.map(|(ix, tab)| {
let on = ix == chosen;
let on = Some(ix) == chosen;
let fg = match (tab.disabled, on) {
(true, _) => colors.fg_disabled,
(false, true) => colors.fg,
@@ -235,6 +232,9 @@ impl RenderOnce for Tabs {
TabPlacement::Left => strip.border_r_1().pr_1(),
})
.on_key_down(move |event, window, cx| {
if keys.is_empty() {
return;
}
let back = if axis == Axis::Vertical { "up" } else { "left" };
let ahead = if axis == Axis::Vertical {
"down"
@@ -242,15 +242,19 @@ impl RenderOnce for Tabs {
"right"
};
let key = event.keystroke.key.as_str();
let (from_back, from_ahead) = match chosen {
Some(at) => (at, at),
None => (0, keys.len() - 1),
};
let to = match key {
key if key == back => step(&keys, chosen, -1),
key if key == ahead => step(&keys, chosen, 1),
key if key == back => step(&keys, from_back, -1),
key if key == ahead => step(&keys, from_ahead, 1),
"home" => step(&keys, keys.len() - 1, 1),
"end" => step(&keys, 0, -1),
_ => return,
};
cx.stop_propagation();
if to != chosen {
if Some(to) != chosen && !keys[to].disabled {
key_pick(to, window, cx);
}
})
src/navigation/tests/mod.rs
+9 -4
View File
@@ -1,4 +1,5 @@
mod pages;
mod stale;
mod tabs;
use gpui::{
@@ -83,14 +84,18 @@ impl Render for Flow {
Choice::new("plan", "Plan"),
Choice::new("confirm", "Confirm"),
];
Wizard::new("flow", steps, self.step)
.on_step(move |to, _, cx| {
Wizard::new(
"flow",
steps,
self.step,
move |to, _, cx| {
stepper.update(cx, |view, cx| {
view.step = to;
cx.notify();
})
})
.on_finish(move |_, cx| finisher.update(cx, |view, _| view.done = true))
},
move |_, cx| finisher.update(cx, |view, _| view.done = true),
)
}
}
src/navigation/tests/pages.rs
+25
View File
@@ -117,3 +117,28 @@ fn the_jump_field_holds_the_last_page_at_every_size(cx: &mut TestAppContext) {
);
}
}
/// Pages the owner has not caught up with: `page` of `pages`.
struct Behind(usize, usize);
impl Render for Behind {
fn render(&mut self, _: &mut Window, _: &mut Context<Self>) -> impl IntoElement {
div().child(Pagination::new("behind", self.0, self.1).jump())
}
}
#[gpui::test]
fn a_page_past_the_last_or_no_pages_marks_none(cx: &mut TestAppContext) {
setup(cx);
let (_, cx) = cx.add_window_view(|_, _| Behind(7, 3));
cx.run_until_parked();
assert!(cx.debug_bounds("pagination-page-3").is_some());
assert!(cx.debug_bounds("pagination-page-7").is_none());
let (_, cx) = cx.add_window_view(|_, _| Behind(1, 0));
cx.run_until_parked();
assert!(cx.debug_bounds("pagination-page-1").is_none());
assert!(
cx.debug_bounds("number-root").is_none(),
"no field to jump with"
);
}
src/navigation/tests/stale.rs
+112
View File
@@ -0,0 +1,112 @@
use gpui::{Context, IntoElement, Render, TestAppContext, Window};
use super::setup;
use crate::{
forms::Choice,
navigation::{BackForwardNavigation, Breadcrumb, Crumb, Steps, Tabs, Wizard},
};
/// Tabs that may lose every tab, or keep only disabled ones under a gone choice.
struct TabHost {
empty: bool,
disabled: bool,
picked: bool,
}
impl Render for TabHost {
fn render(&mut self, _: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
let host = cx.entity();
let tabs = match (self.empty, self.disabled) {
(true, _) => Vec::new(),
(false, true) => vec![
Choice::new("a", "A").disabled(),
Choice::new("b", "B").disabled(),
],
(false, false) => vec![Choice::new("a", "A")],
};
let selected = if self.disabled { "gone" } else { "a" };
Tabs::new("tabs", tabs, selected)
.on_change(move |_, _, cx| host.update(cx, |host, _| host.picked = true))
}
}
#[gpui::test]
fn an_arrow_on_tabs_that_all_closed_does_nothing(cx: &mut TestAppContext) {
setup(cx);
let (host, cx) = cx.add_window_view(|_, _| TabHost {
empty: false,
disabled: false,
picked: false,
});
cx.update(|window, cx| window.focus_next(cx));
host.update(cx, |host, cx| {
host.empty = true;
cx.notify();
});
cx.run_until_parked();
cx.simulate_keystrokes("right");
assert!(!host.read_with(cx, |host, _| host.picked));
}
#[gpui::test]
fn an_arrow_never_picks_a_disabled_tab(cx: &mut TestAppContext) {
setup(cx);
let (host, cx) = cx.add_window_view(|_, _| TabHost {
empty: false,
disabled: true,
picked: false,
});
cx.update(|window, cx| window.focus_next(cx));
cx.simulate_keystrokes("right");
assert!(!host.read_with(cx, |host, _| host.picked));
}
#[derive(Clone, Copy)]
enum Kind {
Steps,
Wizard,
History,
Crumb,
}
/// A list of two that drops its second while the place still names it.
struct Shrinking {
kind: Kind,
shortened: bool,
}
impl Render for Shrinking {
fn render(&mut self, _: &mut Window, _: &mut Context<Self>) -> impl IntoElement {
let mut rows = vec![Choice::new("a", "A"), Choice::new("b", "B")];
if self.shortened {
rows.pop();
}
match self.kind {
Kind::Steps => Steps::new("steps", rows, 1).into_any_element(),
Kind::Wizard => {
Wizard::new("wizard", rows, 1, |_, _, _| {}, |_, _| {}).into_any_element()
}
Kind::History => BackForwardNavigation::new("history", rows, 1).into_any_element(),
Kind::Crumb => {
Breadcrumb::new("crumb", [Crumb::new("b", "B").siblings(rows)]).into_any_element()
}
}
}
}
#[gpui::test]
fn a_place_past_a_shortened_list_marks_none(cx: &mut TestAppContext) {
setup(cx);
for kind in [Kind::Steps, Kind::Wizard, Kind::History, Kind::Crumb] {
let (host, cx) = cx.add_window_view(move |_, _| Shrinking {
kind,
shortened: false,
});
cx.run_until_parked();
host.update(cx, |host, cx| {
host.shortened = true;
cx.notify();
});
cx.run_until_parked();
}
}
src/navigation/tests/tabs.rs
+17
View File
@@ -163,3 +163,20 @@ fn a_right_click_names_its_tab_and_keeps_the_choice(cx: &mut TestAppContext) {
);
assert_eq!(chosen, "c");
}
#[gpui::test]
fn tabs_whose_choice_closed_mark_none_and_arrows_start_at_the_ends(cx: &mut TestAppContext) {
setup(cx);
let (view, cx) = cx.add_window_view(|_, _| Strip {
chosen: "gone".into(),
});
cx.update(|window, cx| window.focus_next(cx));
cx.simulate_keystrokes("right");
assert_eq!(view.read_with(cx, |view, _| view.chosen.clone()), "a");
view.update(cx, |view, cx| {
view.chosen = "gone".into();
cx.notify();
});
cx.simulate_keystrokes("left");
assert_eq!(view.read_with(cx, |view, _| view.chosen.clone()), "c");
}
src/onboarding/wizard.rs
+3 -3
View File
@@ -158,6 +158,8 @@ impl RenderOnce for OnboardingWizard {
.iter()
.map(|step| Choice::new(step.key.clone(), step.title.clone())),
current,
move |to, window, cx| on_step(to, window, cx),
move |window, cx| on_finish(window, cx),
)
.headless()
.ready(self.ready)
@@ -168,9 +170,7 @@ impl RenderOnce for OnboardingWizard {
.gap_5()
.child(words)
.children(self.content),
)
.on_step(move |to, window, cx| on_step(to, window, cx))
.on_finish(move |window, cx| on_finish(window, cx));
);
div()
.flex()
.flex_col()
tasks/panics.md
+3 -2
View File
@@ -6,7 +6,7 @@ Rules, at the owner's word:
- A handler a component needs is an argument of `new`. Leaving it out fails to compile, not at render.
- A state live data or the outside world can reach (a key gone from a list that changed, a failed load) logs an error naming the owner and draws a visible state: nothing chosen, or an error line in the component's box. The app keeps running.
- An assert on a caller's argument, or on a fact true by construction, stays, as std's do. One that runtime data or input can trip is fixed. Reachable means what a component meets on its own: input inside it, its own later state, a call to the outside, or two arguments that agree at one time and not the next (a selected key and its list). One value checked against itself (a ratio above zero) is the caller's contract.
- An assert on a caller's argument, or on a fact true by construction, stays, as std's do. One that runtime data or input can trip is fixed. Reachable means what a component meets on its own: input inside it, its own later state, a call to the outside, or two arguments that agree at one time and not the next (a selected key and its list). One value checked against itself (a ratio above zero) is the caller's contract. A current index or key checked against its list is two arguments even in one call: an owner that rebuilds from live data reaches the mismatch. A list's own emptiness is one value.
- An unknown system time zone logs once and reads as UTC; every time shown in the system zone goes through `format::datetime`, which says "UTC" then. Each item routes its own times there.
- A paint call that fails inside gpui (glyphs, the sprite atlas) logs an error naming the component and skips that draw; drawing an error state would meet the same fault.
@@ -21,7 +21,7 @@ Each item reads every site in its folders, fixes what the rules name, updates ca
- [x] P04 Window & Shell — `src/shell`
- [x] P05 Buttons & Actions — `src/buttons`
- [x] P06 Forms — `src/forms`
- [ ] P07 Navigation — `src/navigation`
- [x] P07 Navigation — `src/navigation`
- [ ] P08 Menus — `src/menus`
- [ ] P09 Overlays — `src/overlays`
- [ ] P10 Feedback — `src/feedback`
@@ -71,3 +71,4 @@ One line per item: what changed, what stayed and why, review rounds.
- P04: no change. Kept: splash progress and column count (caller contracts), the objc class under `Once`, `Tray::pick` (items fixed at creation). Review: 1 round.
- P05: a `SegmentedControl` whose choice left its segments marks none; the cause, `motion::slide`, logs and returns no marker (P11 inherits it). Kept: one-value contracts (group and quick-action counts, toggle label or icon, share payload). Review: 1 round.
- 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.