diff --git a/TASKS.md b/TASKS.md index 4ce127a..7e3e8d7 100644 --- a/TASKS.md +++ b/TASKS.md @@ -397,3 +397,4 @@ Per-component lines live in `tasks/`. Tags there: | T33d | 2 | PASS | | | T34 | 1 | FAIL | a focused tile's Left and Right, and the guard that leaves arrows to a focused control inside a card, untested | | T34 | 2 | PASS | | +| T35 | 1 | FAIL | a row's control kept its full width past a 280px box; a notice's name shrank to nothing; privacy, notices and startup dropped edits with no handler; Apply's and the reset's rest and a box turned off untested | diff --git a/src/canvas/autolayout.rs b/src/canvas/autolayout.rs index 447f5e8..ac5ba2c 100644 --- a/src/canvas/autolayout.rs +++ b/src/canvas/autolayout.rs @@ -127,7 +127,10 @@ impl AutoLayoutControls { impl RenderOnce for AutoLayoutControls { fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { - let (id, layout, on_change) = (self.id, self.layout, &self.on_change); + let on_change = self + .on_change + .unwrap_or_else(|| panic!("auto layout controls {:?} has no on_change", self.id)); + let (id, layout, on_change) = (self.id, self.layout, &on_change); let place = Rc::new(editing( "auto layout", &layout, diff --git a/src/canvas/constraints.rs b/src/canvas/constraints.rs index 6dc1c3c..378c6c4 100644 --- a/src/canvas/constraints.rs +++ b/src/canvas/constraints.rs @@ -135,7 +135,10 @@ fn hairline(x: f32, y: f32, len: f32, across: bool, color: Hsla) -> gpui::Div { impl RenderOnce for ConstraintsEditor { fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement { - let (id, now, on_change) = (self.id, self.constraints, &self.on_change); + let on_change = self + .on_change + .unwrap_or_else(|| panic!("constraints editor {:?} has no on_change", self.id)); + let (id, now, on_change) = (self.id, self.constraints, &on_change); let theme = cx.theme(); let (lit, rest) = (theme.colors.accent, theme.colors.border); let pin = |on: bool| if on { lit } else { rest }; diff --git a/src/canvas/export.rs b/src/canvas/export.rs index 0a6c0e2..beb02ce 100644 --- a/src/canvas/export.rs +++ b/src/canvas/export.rs @@ -138,7 +138,13 @@ impl ExportPanel { impl RenderOnce for ExportPanel { fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement { - let (id, settings, on_change) = (self.id, self.settings, &self.on_change); + let on_change = self + .on_change + .unwrap_or_else(|| panic!("export panel {:?} has no on_change", self.id)); + let on_export = self + .on_export + .unwrap_or_else(|| panic!("export panel {:?} has no on_export", self.id)); + let (id, settings, on_change) = (self.id, self.settings, &on_change); let files = export_files(&self.name, &settings); let next = added(&settings); let add = editing("export panel", &settings, on_change, move |all, _: ()| { @@ -204,7 +210,7 @@ impl RenderOnce for ExportPanel { ) }); let theme = cx.theme(); - let (exported, on_export) = (files.clone(), self.on_export); + let (exported, on_export) = (files.clone(), on_export); div() .flex() .flex_col() @@ -233,9 +239,7 @@ impl RenderOnce for ExportPanel { .disabled(exported.is_empty()) .on_click(move |_, window, cx| { log::info!("export panel: {exported:?}"); - if let Some(on_export) = &on_export { - on_export(exported.clone(), window, cx); - } + on_export(exported.clone(), window, cx); }), ) } diff --git a/src/canvas/inspector.rs b/src/canvas/inspector.rs index f798734..b583d32 100644 --- a/src/canvas/inspector.rs +++ b/src/canvas/inspector.rs @@ -45,6 +45,9 @@ impl InspectorPanel { impl RenderOnce for InspectorPanel { fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement { + let on_change = self + .on_change + .unwrap_or_else(|| panic!("inspector panel {:?} has no on_change", self.id)); let id = self.id; let [shape] = self.selection.as_slice() else { let title: SharedString = match self.selection.len() { @@ -55,7 +58,7 @@ impl RenderOnce for InspectorPanel { .body("Select one layer to see its properties.") .into_any_element(); }; - let on_change = &self.on_change; + let on_change = &on_change; let at = |edit: fn(&mut Shape, f64)| editing("inspector", shape, on_change, edit); let scrub = |name: &'static str, value: f32, least: f64| { ScrubInput::new((id.clone(), name), name, value as f64) diff --git a/src/canvas/panel.rs b/src/canvas/panel.rs index 2471aae..f5352cb 100644 --- a/src/canvas/panel.rs +++ b/src/canvas/panel.rs @@ -17,7 +17,7 @@ pub(crate) type OnEdit = Rc; pub(crate) fn editing( what: &'static str, value: &V, - on_change: &Option>, + on_change: &OnEdit, edit: impl Fn(&mut V, T) + 'static, ) -> impl Fn(T, &mut Window, &mut App) + 'static where @@ -29,9 +29,7 @@ where let mut next = value.clone(); edit(&mut next, input); log::info!("{what}: {next:?}"); - if let Some(on_change) = &on_change { - on_change(next, window, cx); - } + on_change(next, window, cx); } } diff --git a/src/canvas/style.rs b/src/canvas/style.rs index 65b837f..3d9fb94 100644 --- a/src/canvas/style.rs +++ b/src/canvas/style.rs @@ -56,13 +56,14 @@ impl ColorPanel { impl RenderOnce for ColorPanel { fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement { - let (picked, chosen) = (self.on_change.clone(), self.on_change); - let report = |on_change: Option>| { + let on_change = self + .on_change + .unwrap_or_else(|| panic!("color panel {:?} has no on_change", self.id)); + let (picked, chosen) = (on_change.clone(), on_change); + let report = |on_change: OnEdit| { move |color: Hsla, window: &mut Window, cx: &mut App| { log::info!("color panel: {color:?}"); - if let Some(on_change) = &on_change { - on_change(color, window, cx); - } + on_change(color, window, cx); } }; div() @@ -120,6 +121,9 @@ impl ShadowEditor { impl RenderOnce for ShadowEditor { fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement { + let on_change = self + .on_change + .unwrap_or_else(|| panic!("shadow editor {:?} has no on_change", self.id)); let lift = cx .theme() .elevation(Elevation::Floating) @@ -132,7 +136,7 @@ impl RenderOnce for ShadowEditor { color: shadow.color, }) .expect("a floating elevation casts a shadow"); - let (id, shadows, on_change) = (self.id, self.shadows, &self.on_change); + let (id, shadows, on_change) = (self.id, self.shadows, &on_change); let add = editing("shadow editor", &shadows, on_change, move |all, _: ()| { all.push(lift) }); @@ -265,7 +269,10 @@ const CORNERS: [&str; 4] = ["TL", "TR", "BR", "BL"]; impl RenderOnce for BorderEditor { fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { - let (id, border, on_change) = (self.id, self.border, &self.on_change); + let on_change = self + .on_change + .unwrap_or_else(|| panic!("border editor {:?} has no on_change", self.id)); + let (id, border, on_change) = (self.id, self.border, &on_change); let parted = window.use_keyed_state((id.clone(), "split"), cx, |_, _| false); let uneven = border.radii.iter().any(|radius| *radius != border.radii[0]); let split = *parted.read(cx) || uneven; diff --git a/src/canvas/typography.rs b/src/canvas/typography.rs index d91de99..0a3d836 100644 --- a/src/canvas/typography.rs +++ b/src/canvas/typography.rs @@ -73,7 +73,10 @@ impl TypographyPanel { impl RenderOnce for TypographyPanel { fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement { - let (id, style, on_change) = (self.id, self.style, &self.on_change); + let on_change = self + .on_change + .unwrap_or_else(|| panic!("typography panel {:?} has no on_change", self.id)); + let (id, style, on_change) = (self.id, self.style, &on_change); let theme = cx.theme(); let family = editing( "typography panel", diff --git a/src/devtools/structure.rs b/src/devtools/structure.rs index 24902c7..e1697b1 100644 --- a/src/devtools/structure.rs +++ b/src/devtools/structure.rs @@ -123,7 +123,10 @@ impl TableStructureEditor { impl RenderOnce for TableStructureEditor { fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { - let (id, fields, on_change) = (self.id, self.fields, &self.on_change); + let on_change = self + .on_change + .unwrap_or_else(|| panic!("table structure editor {:?} has no on_change", self.id)); + let (id, fields, on_change) = (self.id, self.fields, &on_change); let room = window.use_keyed_state((id.clone(), "room"), cx, |_, _| Room::default()); let rem = window.rem_size(); let theme = cx.theme(); @@ -336,6 +339,8 @@ impl IndexManager { impl RenderOnce for IndexManager { fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { + let on_create = self.on_create.expect("an index manager needs on_create"); + let on_drop = self.on_drop.expect("an index manager needs on_drop"); let id = self.id; let draft = window.use_keyed_state((id.clone(), "draft"), cx, |window, cx| Draft { name: cx.new(|cx| TextInput::new(window, cx).placeholder("Index name")), @@ -345,7 +350,7 @@ impl RenderOnce for IndexManager { }); let theme = cx.theme(); let rows = self.indexes.iter().map(|index| { - let (name, on_drop) = (index.name.clone(), self.on_drop.clone()); + let (name, on_drop) = (index.name.clone(), on_drop.clone()); div() .flex() .items_center() @@ -391,9 +396,7 @@ impl RenderOnce for IndexManager { .tooltip("Drop the index") .on_click(move |_, window, cx| { log::info!("index manager: drop {name}"); - if let Some(on_drop) = &on_drop { - on_drop(&name, window, cx); - } + on_drop(&name, window, cx); }), ), ) @@ -407,7 +410,6 @@ impl RenderOnce for IndexManager { ); let ready = !name.read(cx).text().trim().is_empty() && !picked.is_empty(); let [chose, flipped, pick_method, made] = [(); 4].map(|_| draft.clone()); - let on_create = self.on_create; div() .flex() .flex_col() @@ -487,9 +489,7 @@ impl RenderOnce for IndexManager { index }); log::info!("index manager: create {}", index.name); - if let Some(on_create) = &on_create { - on_create(index, window, cx); - } + on_create(index, window, cx); }), ), ) diff --git a/src/devtools/tests/mod.rs b/src/devtools/tests/mod.rs index 983b430..e438a70 100644 --- a/src/devtools/tests/mod.rs +++ b/src/devtools/tests/mod.rs @@ -204,13 +204,16 @@ fn another_engine_brings_its_port(cx: &mut TestAppContext) { fn narrow(_: &Bench, _: &mut Window, _: &mut App, _: Entity) -> AnyElement { div() .w(px(280.0)) - .child(TableStructureEditor::new( - "structure", - [ - Field::new("user_id", "timestamptz"), - Field::new("total", "numeric"), - ], - )) + .child( + TableStructureEditor::new( + "structure", + [ + Field::new("user_id", "timestamptz"), + Field::new("total", "numeric"), + ], + ) + .on_change(|_, _, _| {}), + ) .into_any_element() } diff --git a/src/settings/appearance.rs b/src/settings/appearance.rs index 8b94284..389245c 100644 --- a/src/settings/appearance.rs +++ b/src/settings/appearance.rs @@ -236,10 +236,14 @@ impl RenderOnce for FontSizeControl { ), ) .child( - div() - .text_size(Pixels::from(self.size)) - .text_color(theme.colors.fg) - .child("The quick brown fox jumps over the lazy dog."), + div().flex().child( + div() + .flex_1() + .min_w_0() + .text_size(Pixels::from(self.size)) + .text_color(theme.colors.fg) + .child("The quick brown fox jumps over the lazy dog."), + ), ) } } diff --git a/src/settings/network.rs b/src/settings/network.rs index 98935f2..1950656 100644 --- a/src/settings/network.rs +++ b/src/settings/network.rs @@ -129,7 +129,10 @@ impl RenderOnce for ProxySettings { "manual" => manual(&text(&host), &text(&port), &text(&bypass)), other => panic!("proxy settings: no way named {other}"), }; - let (switched, on_apply) = (fields.clone(), self.on_apply); + let on_apply = self + .on_apply + .unwrap_or_else(|| panic!("proxy settings {id:?} has no on_apply")); + let switched = fields.clone(); div() .flex() .flex_col() @@ -181,10 +184,9 @@ impl RenderOnce for ProxySettings { .variant(ButtonVariant::Primary) .disabled(read.is_none()) .on_click(move |_, window, cx| { - if let (Some(proxy), Some(on_apply)) = (read.clone(), &on_apply) { - log::info!("proxy settings: apply {proxy:?}"); - on_apply(proxy, window, cx); - } + let proxy = read.clone().expect("Apply rests until the proxy reads"); + log::info!("proxy settings: apply {proxy:?}"); + on_apply(proxy, window, cx); }), ), ) diff --git a/src/settings/preferences.rs b/src/settings/preferences.rs index b12c33d..ba95b10 100644 --- a/src/settings/preferences.rs +++ b/src/settings/preferences.rs @@ -46,7 +46,10 @@ impl PrivacySettings { impl RenderOnce for PrivacySettings { fn render(self, _: &mut Window, _: &mut App) -> impl IntoElement { - let (id, privacy, on_change) = (self.id, self.privacy, &self.on_change); + let on_change = self + .on_change + .unwrap_or_else(|| panic!("privacy settings {:?} has no on_change", self.id)); + let (id, privacy, on_change) = (self.id, self.privacy, &on_change); let row = |key: &'static str, title: &'static str, text: &'static str, @@ -100,7 +103,7 @@ pub struct Notices { pub quiet: bool, } -/// A grid of notices: a row for each kind, a box for each channel it may arrive by, and a switch that holds every notice back. +/// Notices by kind: a row for each, a named box for each channel it may arrive by that folds under the name when narrow, and a switch that holds every notice back. #[derive(IntoElement)] pub struct NotificationSettings { id: ElementId, @@ -125,55 +128,54 @@ impl NotificationSettings { impl RenderOnce for NotificationSettings { fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement { - let (id, notices, on_change) = (self.id, self.notices, &self.on_change); + let on_change = self + .on_change + .unwrap_or_else(|| panic!("notification settings {:?} has no on_change", self.id)); + let (id, notices, on_change) = (self.id, self.notices, &on_change); let theme = cx.theme(); - let cell = theme.label_width() * 0.5; - let head = div() - .flex() - .items_center() - .gap_2() - .text_size(theme.text_size(TextSize::Xs)) - .text_color(theme.colors.fg_subtle) - .child(div().flex_1().min_w_0()) - .children(notices.channels.iter().map(|channel| { - div() - .flex_none() - .w(cell) - .child(Ellipsis::new(channel.clone())) - })); let rows = notices.kinds.iter().map(|(kind, name)| { + let boxes = notices.channels.iter().map(|channel| { + let pair = (kind.clone(), channel.clone()); + let on = notices.on.contains(&pair); + let set = editing( + "notifications", + ¬ices, + on_change, + move |notices: &mut Notices, on: bool| { + notices.on.retain(|each| *each != pair); + if on { + notices.on.push(pair.clone()); + } + }, + ); + Checkbox::new((id.clone(), format!("{kind}-{channel}")), on) + .label(channel.clone()) + .on_change(set) + }); div() .flex() + .flex_wrap() .items_center() - .gap_2() + .gap_x_4() + .gap_y_1() .py_2() .border_b_1() .border_color(theme.colors.border) .child( div() .flex_1() - .min_w_0() + .min_w(theme.label_width()) .text_size(theme.text_size(TextSize::Sm)) .child(Ellipsis::new(name.clone())), ) - .children(notices.channels.iter().map(|channel| { - let pair = (kind.clone(), channel.clone()); - let on = notices.on.contains(&pair); - let set = editing( - "notifications", - ¬ices, - on_change, - move |notices: &mut Notices, on: bool| { - notices.on.retain(|each| *each != pair); - if on { - notices.on.push(pair.clone()); - } - }, - ); - div().flex_none().w(cell).child( - Checkbox::new((id.clone(), format!("{kind}-{channel}")), on).on_change(set), - ) - })) + .child( + div() + .flex() + .flex_wrap() + .flex_none() + .gap_x_3() + .children(boxes), + ) }); let quiet = editing( "notifications", @@ -188,7 +190,7 @@ impl RenderOnce for NotificationSettings { .description("Hold every notice back until it is off.") .control(Switch::new((id.clone(), "quiet"), notices.quiet).on_change(quiet)), ) - .row(div().pt_3().child(head).children(rows)) + .row(div().pt_3().children(rows)) } } @@ -232,7 +234,10 @@ impl StartupSettings { impl RenderOnce for StartupSettings { fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement { - let (id, startup, on_change) = (self.id, self.startup, &self.on_change); + let on_change = self + .on_change + .unwrap_or_else(|| panic!("startup settings {:?} has no on_change", self.id)); + let (id, startup, on_change) = (self.id, self.startup, &on_change); let switch = |key: &'static str, on: bool, set: fn(&mut Startup, bool)| { Switch::new((id.clone(), key), on) .on_change(editing("startup", &startup, on_change, set)) diff --git a/src/settings/section.rs b/src/settings/section.rs index bfd11c0..dd004b9 100644 --- a/src/settings/section.rs +++ b/src/settings/section.rs @@ -217,6 +217,9 @@ impl RenderOnce for SettingsRow { .child(text) })), ) - .children(self.control.map(|control| div().flex_none().child(control))) + .children( + self.control + .map(|control| div().flex_none().min_w_0().max_w_full().child(control)), + ) } } diff --git a/src/settings/tests.rs b/src/settings/tests.rs index fccf2d9..49d8ef4 100644 --- a/src/settings/tests.rs +++ b/src/settings/tests.rs @@ -430,3 +430,51 @@ fn export_asks_the_owner(cx: &mut TestAppContext) { tap("space", cx); assert_eq!(said(&host, cx), ["export"]); } + +fn bad_port(_: &mut Window, _: &mut App, owner: Entity) -> AnyElement { + let proxy = Proxy::Manual { + host: "proxy.corp".into(), + port: 0, + bypass: Vec::new(), + }; + ProxySettings::new("proxy", proxy) + .on_apply(move |proxy, _, cx| say(&owner, format!("{proxy:?}"), cx)) + .into_any_element() +} + +/// With port 0 Apply rests, so a step back from nothing lands on the field before it. +#[gpui::test] +fn apply_rests_while_the_port_is_bad(cx: &mut TestAppContext) { + let (host, cx) = desk(bad_port, cx); + cx.update(|window, _| { + window.blur(); + window.focus_prev(); + }); + settle(cx); + tap("space", cx); + assert!(said(&host, cx).is_empty()); +} + +fn default_size(_: &mut Window, _: &mut App, owner: Entity) -> AnyElement { + FontSizeControl::new("size", 14.0, 14.0, (12.0, 24.0)) + .on_change(move |size, _, cx| say(&owner, format!("size {size}"), cx)) + .into_any_element() +} + +/// At the default size the way back rests, so the second stop wraps to the slider. +#[gpui::test] +fn the_way_back_rests_at_the_default_size(cx: &mut TestAppContext) { + let (host, cx) = desk(default_size, cx); + tab(2, cx); + tap("space", cx); + assert!(said(&host, cx).is_empty()); +} + +/// Stops: Do not disturb, then App, which is on. +#[gpui::test] +fn a_box_pressed_off_lets_its_pair_go(cx: &mut TestAppContext) { + let (host, cx) = desk(notices, cx); + tab(2, cx); + tap("space", cx); + assert_eq!(said(&host, cx), ["[] false"]); +}