From 9a8dbcb92ea882a01af4806c21a10e5268d2772c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sun, 27 Sep 2026 12:53:08 -0400 Subject: [PATCH] Settings: controls give way, notices fold, and every edit needs its owner A settings row's control slot shrinks to the box, so a control's text wraps and a palette's row folds; the font size's sample wraps in a plain block. A notice's boxes carry their channel's name and fold under the kind's name when narrow. canvas::panel::editing takes the owner's handler itself: the design panels, the structure editor, the index manager and the privacy, notice and startup settings take theirs at render and fail loud without one. Apply fails loud if pressed while the proxy does not read. Tests cover Apply resting on a bad port, the way back resting at the default size and a box pressed off. Co-Authored-By: Claude Opus 5.5 --- TASKS.md | 1 + src/canvas/autolayout.rs | 5 ++- src/canvas/constraints.rs | 5 ++- src/canvas/export.rs | 14 ++++--- src/canvas/inspector.rs | 5 ++- src/canvas/panel.rs | 6 +-- src/canvas/style.rs | 21 ++++++---- src/canvas/typography.rs | 5 ++- src/devtools/structure.rs | 18 ++++---- src/devtools/tests/mod.rs | 17 ++++---- src/settings/appearance.rs | 12 ++++-- src/settings/network.rs | 12 +++--- src/settings/preferences.rs | 83 ++++++++++++++++++++----------------- src/settings/section.rs | 5 ++- src/settings/tests.rs | 48 +++++++++++++++++++++ 15 files changed, 172 insertions(+), 85 deletions(-) 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"]); +}