diff --git a/clients/windows/src/app/hosts.rs b/clients/windows/src/app/hosts.rs index 6ec2a6ea..e8f767e2 100644 --- a/clients/windows/src/app/hosts.rs +++ b/clients/windows/src/app/hosts.rs @@ -168,20 +168,26 @@ pub(crate) struct Hover { } /// The status row at the bottom of a tile: the host's OS mark (when advertised), presence -/// dot + Online/Offline, plus the trust chip. -fn status_row(os: &str, online: Option, badge: &str, kind: Pill) -> Element { - status_row_with(os, online, badge, kind, None) +/// dot + Online/Offline, plus a trust chip only where it says something (see +/// [`status_row_with`]). +fn status_row(os: &str, online: Option, badge: Option<(&str, Pill)>) -> Element { + status_row_with(os, online, badge, None) } -/// [`status_row`] plus the profile chip: what a plain click on THIS tile will use — its own +/// [`status_row`] plus the profile: what a plain click on THIS tile will use — its own /// profile on a pinned tile, the host's binding on the primary one. A binding whose profile /// was deleted shows nothing and resolves as the defaults, which is what will happen on /// connect (design §6). +/// +/// The row is METADATA, not a badge shelf — three chips side by side read as noise. Paired +/// is the normal resting state of a saved host, so it earns NO chip at all; a chip appears +/// only where it carries a decision ("Trusted" = TOFU without pairing, "PIN"/"Open" on a +/// discovered host). The profile is a small dot in the profile's own colour plus its name +/// in plain caption text — recognisable at a glance without competing with the host name. fn status_row_with( os: &str, online: Option, - badge: &str, - kind: Pill, + badge: Option<(&str, Pill)>, profile: Option<(&str, Option)>, ) -> Element { let mut items: Vec = Vec::new(); @@ -212,29 +218,45 @@ fn status_row_with( .into(), ); } - items.push( - pill(badge, kind) - .vertical_alignment(VerticalAlignment::Center) - .into(), - ); + if let Some((badge, kind)) = badge { + items.push( + pill(badge, kind) + .vertical_alignment(VerticalAlignment::Center) + .into(), + ); + } if let Some((name, accent)) = profile { - // A profile's own colour where it has one, the neutral chip where it doesn't — the + // The profile's own colour where it has one, a neutral disc where it doesn't — the // palette stays opt-in, and an unparsable value falls back rather than being trusted. - let chip = match accent.as_deref().and_then(super::settings::hex_color) { - Some(colour) => border( - text_block(name) - .font_size(11.0) - .semibold() - .foreground(colour), - ) - .background(Color { a: 46, ..colour }) - .border_brush(ThemeRef::CardStroke) - .border_thickness(uniform(1.0)) - .corner_radius(10.0) - .padding(edges(9.0, 2.0, 9.0, 2.0)), - None => pill(name, Pill::Neutral), - }; - items.push(chip.vertical_alignment(VerticalAlignment::Center).into()); + let colour = accent + .as_deref() + .and_then(super::settings::hex_color) + .unwrap_or(Color { + a: 120, + r: 128, + g: 128, + b: 128, + }); + items.push( + border(vstack(Vec::::new()).width(8.0).height(8.0)) + .background(colour) + .corner_radius(4.0) + .margin(edges( + if items.is_empty() { 0.0 } else { 4.0 }, + 0.0, + 0.0, + 0.0, + )) + .vertical_alignment(VerticalAlignment::Center) + .into(), + ); + items.push( + text_block(name) + .font_size(11.0) + .foreground(ThemeRef::SecondaryText) + .vertical_alignment(VerticalAlignment::Center) + .into(), + ); } hstack(items) .spacing(6.0) @@ -809,9 +831,9 @@ pub(crate) fn hosts_page(props: &HostsProps, cx: &mut RenderCx) -> Element { status_row_with( &k.os, Some(online), - if k.paired { "Paired" } else { "Trusted" }, - if k.paired { Pill::Good } else { Pill::Info }, - // The chip carries the profile's own colour where it has one — + // Paired is the resting state — no chip; TOFU-only trust is worth one. + (!k.paired).then_some(("Trusted", Pill::Info)), + // The dot carries the profile's own colour where it has one — // that is what makes two bound hosts tell apart at a glance. k.profile_id .as_ref() @@ -853,8 +875,7 @@ pub(crate) fn hosts_page(props: &HostsProps, cx: &mut RenderCx) -> Element { status_row_with( &k.os, Some(online), - if k.paired { "Paired" } else { "Trusted" }, - if k.paired { Pill::Good } else { Pill::Info }, + (!k.paired).then_some(("Trusted", Pill::Info)), Some((name.as_str(), accent.clone())), ), None, @@ -916,7 +937,7 @@ pub(crate) fn hosts_page(props: &HostsProps, cx: &mut RenderCx) -> Element { &hover, &h.name, &format!("{}:{}", h.addr, h.port), - status_row(&h.os, None, badge, kind), + status_row(&h.os, None, Some((badge, kind))), None, Some(Box::new(move || initiate(&ctx2, target.clone(), &ss, &st))), )); @@ -924,35 +945,45 @@ pub(crate) fn hosts_page(props: &HostsProps, cx: &mut RenderCx) -> Element { body.push(tile_grid(tiles, cols, TILE_GAP)); } - // Forget confirmation (modal; shown while `forget` holds a pending host). Confirmed first, - // since it's destructive and re-establishing trust needs a fresh pairing. - if let Some((fp, name)) = forget { + // Forget confirmation, armed while `forget` holds a pending host. ALWAYS MOUNTED with + // `is_open` doing the arming, and in a STABLE trailing layer rather than in `body` + // (whose child list shifts with discovery): unmounting — or positionally re-pairing — + // a ContentDialog trips the reactor backend's phantom-child bookkeeping (the handle + // dies before `remove_child` runs, the dialog stops being recognised as phantom, and a + // visual child that never existed gets RemoveAt()'d — E_BOUNDS panic; see the + // delete-profile dialog in settings.rs). Confirmed first, since it's destructive and + // re-establishing trust needs a fresh pairing. + let forget_confirm: Element = { let sf = set_forget.clone(); - body.push( - ContentDialog::new("Remove saved host?") - .content(format!( + let pending = forget.clone(); + let content = pending + .as_ref() + .map(|(_, name)| { + format!( "Forget \u{201C}{name}\u{201D}? You'll need to pair (or trust) it again to \ reconnect." - )) - .primary_button_text("Remove") - .close_button_text("Cancel") - .is_open(true) - .on_closed(move |r: ContentDialogResult| { - if r == ContentDialogResult::Primary { + ) + }) + .unwrap_or_default(); + ContentDialog::new("Remove saved host?") + .content(content) + .primary_button_text("Remove") + .close_button_text("Cancel") + .is_open(pending.is_some()) + .on_closed(move |r: ContentDialogResult| { + if r == ContentDialogResult::Primary { + if let Some((fp, _)) = &pending { let mut known = KnownHosts::load(); - known.remove_by_fp(&fp); + known.remove_by_fp(fp); let _ = known.save(); } - sf.call(None); // re-renders the page; the row is gone on the next load - }) - .into(), - ); - } + } + sf.call(None); // re-renders the page; the row is gone on the next load + }) + .into() + }; let page = page_wide(body); - if !show_add { - return page; - } // "Add host" modal: a scrim + centered card. It's an in-tree overlay, not a WinUI // ContentDialog, because ContentDialog is text-only in windows-reactor (no room for a text @@ -1044,12 +1075,20 @@ pub(crate) fn hosts_page(props: &HostsProps, cx: &mut RenderCx) -> Element { 24.0, )); - // The scrim fades in with the same tween. - let scrim = border(modal).background(Color { - a: (140.0 * props.add_anim) as u8, - r: 0, - g: 0, - b: 0, - }); - grid(vec![page, scrim.into()]).into() + // The scrim fades in with the same tween. Its layer slot is STABLE (a same-kind, + // background-less Border when closed — invisible and not hit-testable) so the layer + // list never changes shape around the always-mounted dialog after it. + let add_slot: Element = if show_add { + border(modal) + .background(Color { + a: (140.0 * props.add_anim) as u8, + r: 0, + g: 0, + b: 0, + }) + .into() + } else { + border(vstack(Vec::::new())).into() + }; + grid(vec![page, add_slot, forget_confirm]).into() } diff --git a/clients/windows/src/app/settings.rs b/clients/windows/src/app/settings.rs index b80c24c6..d987c69f 100644 --- a/clients/windows/src/app/settings.rs +++ b/clients/windows/src/app/settings.rs @@ -518,58 +518,47 @@ fn described_overridable( control: impl Into, caption: &str, ) -> Element { - if scope.is_empty() { + if scope.is_empty() || !overridden { return described(control, caption); } - // Nothing that appears on an edit may move what was edited (the GTK client's rule, - // learned there the hard way): both states of a profile-scope row reserve the same - // caption-line height, so the marker + Reset materialise IN PLACE instead of shoving - // every row below them down by a button's height. - const CAPTION_MIN_H: f64 = 34.0; - if !overridden { - return vstack(( - control.into(), - hstack(vec![text_block(caption) - .font_size(12.0) - .foreground(ThemeRef::SecondaryText) - .wrap() - .max_width(420.0) - .horizontal_alignment(HorizontalAlignment::Left) - .vertical_alignment(VerticalAlignment::Center) - .into()]) - .min_height(CAPTION_MIN_H), - )) - .spacing(5.0) - .into(); - } + // The override marker lives on the CONTROL's line, not in the caption: a small tinted + // "Overridden" chip and the Reset button sit to the control's right, bottom-aligned to + // its box. The caption below stays plain and identical in both states — appearing to + // the SIDE means nothing moves and the description stays a description (the mixed + // marker-plus-caption line read as neither). let (rev, set_rev) = (rev.0, rev.1.clone()); let scope = scope.to_string(); vstack(( - control.into(), hstack(( - text_block(format!("\u{25cf} Overridden here \u{00b7} {caption}")) - .font_size(12.0) - .foreground(ThemeRef::AccentText) - .wrap() - .max_width(360.0) - .horizontal_alignment(HorizontalAlignment::Left) - .vertical_alignment(VerticalAlignment::Center), - button("Reset").icon(Symbol::Undo).on_click(move || { - let mut catalog = ProfilesFile::load(); - if let Some(p) = catalog.profiles.iter_mut().find(|p| p.id == scope) { - p.overrides.clear(field); - if let Err(e) = catalog.save() { - tracing::warn!(error = %format!("{e:#}"), "clearing an override"); + control.into(), + pill("Overridden", Pill::Info) + .vertical_alignment(VerticalAlignment::Bottom) + .margin(edges(0.0, 0.0, 0.0, 5.0)), + button("Reset") + .icon(Symbol::Undo) + .tooltip("Back to inheriting from Default settings") + .vertical_alignment(VerticalAlignment::Bottom) + .on_click(move || { + let mut catalog = ProfilesFile::load(); + if let Some(p) = catalog.profiles.iter_mut().find(|p| p.id == scope) { + p.overrides.clear(field); + if let Err(e) = catalog.save() { + tracing::warn!(error = %format!("{e:#}"), "clearing an override"); + } } - } - // The catalog changed behind the controls, and nothing the page reads as - // state did — bump the revision so the row re-renders showing the inherited - // value again. - set_rev.call(rev + 1); - }), + // The catalog changed behind the controls, and nothing the page reads + // as state did — bump the revision so the row re-renders showing the + // inherited value again. + set_rev.call(rev + 1); + }), )) - .spacing(8.0) - .min_height(CAPTION_MIN_H), + .spacing(8.0), + text_block(caption) + .font_size(12.0) + .foreground(ThemeRef::SecondaryText) + .wrap() + .max_width(420.0) + .horizontal_alignment(HorizontalAlignment::Left), )) .spacing(5.0) .into() @@ -607,7 +596,13 @@ fn group_heading(label: &str) -> Element { /// One settings group: an optional sub-section label, a card of fields, and an optional /// form-level note under it (Apple's Section header/footer). Groups stack down the page. +/// A group with NO fields renders NOTHING — several groups pass an empty list in profile +/// scope (Decoding, Library: device facts, never per profile), and a heading over an empty +/// card read as a bug. fn group(header: Option<&str>, fields: Vec, footer: Option<&str>) -> Vec { + if fields.is_empty() { + return Vec::new(); + } let mut out = Vec::with_capacity(3); if let Some(h) = header { out.push(group_heading(h)); @@ -1484,68 +1479,83 @@ pub(crate) fn settings_page( .opacity(progress) .margin(edges(0.0, (1.0 - progress) * 22.0, 0.0, 0.0)); let content: Element = scrolled.into(); - // The delete confirmation, when armed. Declarative, like every other dialog in this shell: - // it is an element in the tree with `is_open`, not a call. - let confirm: Option = delete_pending.as_ref().and_then(|id| { - let p = ProfilesFile::load().find_by_id(id).cloned()?; - // The warning counts what actually breaks: hosts that fall back to the defaults, and - // pinned cards that disappear (design §6). - let known = KnownHosts::load(); - let bound = known - .hosts - .iter() - .filter(|h| h.profile_id.as_deref() == Some(p.id.as_str())) - .count(); - let pinned = known - .hosts - .iter() - .filter(|h| h.pinned_profiles.iter().any(|x| x == &p.id)) - .count(); - let mut body = format!("\u{201c}{}\u{201d} will be removed.", p.name); - if bound > 0 { - body.push_str(&format!( - " {bound} host{} will fall back to Default settings.", - if bound == 1 { "" } else { "s" } - )); - } - if pinned > 0 { - body.push_str(&format!( - " {pinned} pinned card{} will disappear.", - if pinned == 1 { "" } else { "s" } - )); - } + // The delete confirmation. Declarative like every dialog in this shell — but ALWAYS + // MOUNTED, with `is_open` doing the arming: a ContentDialog is a "phantom" child in the + // reactor backend (tracked logically, never attached to the panel), and unmounting one + // destroys its handle before `remove_child` runs, so the backend stops recognising it + // as phantom and RemoveAt()s a visual child that does not exist — E_BOUNDS, main-thread + // panic ("Daten außerhalb des gültigen Bereichs"), reliably on every delete. A mounted + // dialog is never removed, so the bug has nothing to bite. (Upstream report material — + // the third windows-reactor bug this client documents.) + let confirm: Element = { + let pending = delete_pending + .as_ref() + .and_then(|id| ProfilesFile::load().find_by_id(id).cloned()); + // The warning counts what actually breaks: hosts that fall back to the defaults, + // and pinned cards that disappear (design §6). + let body = pending + .as_ref() + .map(|p| { + let known = KnownHosts::load(); + let bound = known + .hosts + .iter() + .filter(|h| h.profile_id.as_deref() == Some(p.id.as_str())) + .count(); + let pinned = known + .hosts + .iter() + .filter(|h| h.pinned_profiles.iter().any(|x| x == &p.id)) + .count(); + let mut body = format!("\u{201c}{}\u{201d} will be removed.", p.name); + if bound > 0 { + body.push_str(&format!( + " {bound} host{} will fall back to Default settings.", + if bound == 1 { "" } else { "s" } + )); + } + if pinned > 0 { + body.push_str(&format!( + " {pinned} pinned card{} will disappear.", + if pinned == 1 { "" } else { "s" } + )); + } + body + }) + .unwrap_or_default(); let (id, set_scope, set_delete, set_edit) = ( - p.id.clone(), + pending.as_ref().map(|p| p.id.clone()), set_scope.clone(), set_delete.clone(), set_edit.clone(), ); - Some( - ContentDialog::new("Delete profile?") - .content(body) - .primary_button_text("Delete") - .close_button_text("Cancel") - .is_open(true) - .on_closed(move |r: ContentDialogResult| { - set_delete.call(None); - if r != ContentDialogResult::Primary { - return; - } - let mut catalog = ProfilesFile::load(); - catalog.profiles.retain(|p| p.id != id); - // Bindings and pins are left dangling on purpose: they resolve as "no - // profile" everywhere, and rewriting every host record here would be a - // second, racier source of truth. - if catalog.save().is_ok() { - set_scope.call(String::new()); - // The profile the Edit modal was showing is gone — without this, the - // still-armed flag would pop the modal open on the NEXT profile pick. - set_edit.call(false); - } - }) - .into(), - ) - }); + ContentDialog::new("Delete profile?") + .content(body) + .primary_button_text("Delete") + .close_button_text("Cancel") + .is_open(pending.is_some()) + .on_closed(move |r: ContentDialogResult| { + set_delete.call(None); + if r != ContentDialogResult::Primary { + return; + } + let Some(id) = id.clone() else { + return; + }; + let mut catalog = ProfilesFile::load(); + catalog.profiles.retain(|p| p.id != id); + // Bindings and pins are left dangling on purpose: they resolve as "no + // profile" everywhere, and rewriting every host record here would be a + // second, racier source of truth. + if catalog.save().is_ok() { + set_scope.call(String::new()); + // The profile the sheet was showing is gone — without this, the + // still-armed flag would pop the sheet open on the NEXT profile pick. + set_edit.call(false); + } + }) + .into() + }; let nav = NavigationView::new(items, content) .pane_title("Settings") .selected_tag(section) @@ -1560,14 +1570,16 @@ pub(crate) fn settings_page( move || ss.call(Screen::Hosts) }); // Overlay layers fill the NAV's cell (grids stretch children; a vstack would hand the - // NavigationView its desired height — clipped short, floating tall): the nav, the - // profile-sheet scrim + card over it, and the delete confirmation (a ContentDialog, its - // own WinUI layer, so its slot is irrelevant — it just needs to be in the tree). - let mut layers: Vec = vec![nav.into()]; - // The profile sheet — "Edit profile…" in the bar. The bar owns the scope choice, so - // the sheet carries only the profile being edited. - if edit_open && profile_mode { - layers.push(edit_profile_modal( + // NavigationView its desired height — clipped short, floating tall). The layer list is + // STABLE — always [nav, sheet slot, dialog] — so no pass ever removes a grid child: + // removals are where the reconciler's phantom-dialog bookkeeping breaks (see `confirm` + // above), and a closed sheet leaves a same-kind, background-less Border in its slot + // (invisible, and per style.rs a null background is not hit-testable, so it swallows + // no clicks). + let sheet_slot: Element = if edit_open && profile_mode { + // The profile sheet — "Edit profile…" in the bar. The bar owns the scope choice, + // so the sheet carries only the profile being edited. + edit_profile_modal( active.as_ref(), None, set_scope, @@ -1575,16 +1587,15 @@ pub(crate) fn settings_page( set_edit, rev, set_rev, - )); - } - if let Some(dialog) = confirm { - layers.push(dialog); - } + ) + } else { + border(vstack(Vec::::new())).into() + }; // The bar rides an Auto row above the nav's Star row, so the nav (and the sheet's scrim // over it) still fills the rest of the window. grid(vec![ scope_bar.grid_row(0), - Element::from(grid(layers)).grid_row(1), + Element::from(grid(vec![nav.into(), sheet_slot, confirm])).grid_row(1), ]) .rows([GridLength::Auto, GridLength::STAR]) .into()