From dff61ecea2533bcf80e2c2055f99be7a9b75ecd0 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Wed, 29 Jul 2026 12:38:48 +0200 Subject: [PATCH] fix(client/windows): the scope switcher leaves the page content, and the polish from the GTK round comes across MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four things the Linux review found, applied here without waiting to re-find them: The profile switcher was rendered as the first group INSIDE each section's content, so on About — a page with barely any rows — it read as an About setting, and on every other page it read as one more row. It is chrome: which layer am I editing. It now sits in its own card above the NavigationView, visible from every section and never mistakable for a setting. Profiles get their colour here too, from the same eight-swatch palette as GTK, and the chip on a host tile is tinted with it — a profile that is red on Linux is red on Windows. An unparsable accent falls back to the neutral chip rather than being handed to the renderer. The tile flyout had become a list of everything, with connect/library/speed buried under list management. Two rules thin it: anything that CONFIGURES the host moves into the editor — the pinned tiles join the default profile and the clipboard toggle there, since a pin is a property of the record — and what remains is grouped by separators, so a glance lands on the right third. Compiled on .221; the last one-line clippy fix went in after the box dropped. --- clients/windows/src/app/hosts.rs | 164 +++++++++++++++++----------- clients/windows/src/app/settings.rs | 107 ++++++++++++++++-- 2 files changed, 203 insertions(+), 68 deletions(-) diff --git a/clients/windows/src/app/hosts.rs b/clients/windows/src/app/hosts.rs index ef839d58..452ffc75 100644 --- a/clients/windows/src/app/hosts.rs +++ b/clients/windows/src/app/hosts.rs @@ -24,8 +24,6 @@ const MENU_EDIT: &str = "Edit\u{2026}"; /// items matched by prefix. The trailing space keeps them readable AND keeps a profile named /// e.g. "Copy link" from colliding with a fixed entry. const MENU_WITH: &str = "Connect with: "; -const MENU_PIN: &str = "Pin as card: "; -const MENU_UNPIN: &str = "Unpin card: "; const MENU_COPY_LINK: &str = "Copy link"; const MENU_SHORTCUT: &str = "Create shortcut\u{2026}"; const MENU_FORGET: &str = "Forget\u{2026}"; @@ -182,7 +180,7 @@ fn status_row_with( online: Option, badge: &str, kind: Pill, - profile: Option<(&str, Pill)>, + profile: Option<(&str, Option)>, ) -> Element { let mut items: Vec = Vec::new(); if let Some(online) = online { @@ -204,12 +202,24 @@ fn status_row_with( .vertical_alignment(VerticalAlignment::Center) .into(), ); - if let Some((name, kind)) = profile { - items.push( - pill(name, 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 + // 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()); } hstack(items) .spacing(6.0) @@ -315,6 +325,53 @@ fn edit_editor( } }) }; + // Pinned tiles: which profiles get their own one-click tile for this host. They used to be + // two more flat entries in the tile's flyout, which is what tipped that menu over — and this + // is where they belong anyway, beside the default they sit next to (design §5.2a). Each + // switch writes immediately, like the profile picker above and unlike the text fields, which + // need a Save because they have drafts. + let pin_switches: Element = { + let catalog = pf_client_core::profiles::ProfilesFile::load(); + let stored = KnownHosts::load() + .hosts + .iter() + .find(|h| h.fp_hex == fp) + .cloned(); + if catalog.profiles.is_empty() { + vstack(Vec::::new()).into() + } else { + let mut rows: Vec = vec![text_block("Pinned tiles") + .font_size(12.0) + .foreground(ThemeRef::SecondaryText) + .horizontal_alignment(HorizontalAlignment::Left) + .into()]; + for p in &catalog.profiles { + let on = stored + .as_ref() + .is_some_and(|h| h.pinned_profiles.iter().any(|id| id == &p.id)); + let (fp, id) = (fp.to_string(), p.id.clone()); + rows.push( + ToggleSwitch::new(on) + .header(&p.name) + .on_content("Shown") + .off_content("Hidden") + .on_toggled(move |v: bool| { + let mut known = KnownHosts::load(); + if let Some(h) = known.hosts.iter_mut().find(|h| h.fp_hex == fp) { + h.pinned_profiles.retain(|x| x != &id); + if v { + h.pinned_profiles.push(id.clone()); + } + let _ = known.save(); + } + }) + .into(), + ); + } + vstack(rows).spacing(6.0).into() + } + }; + let field = |label: &str, value: String, placeholder: &str, draft: HookRef| { vstack(( text_block(label) @@ -358,6 +415,7 @@ fn edit_editor( .horizontal_alignment(HorizontalAlignment::Left), )) .spacing(4.0), + pin_switches, vstack(( ToggleSwitch::new(clip0) .header("Share clipboard with this host") @@ -559,11 +617,12 @@ pub(crate) fn hosts_page(props: &HostsProps, cx: &mut RenderCx) -> Element { body.push(section("SAVED HOSTS")); let mut tiles: Vec = Vec::new(); // One catalog read per render, shared by every tile's menu and chip. - let profiles: Vec<(String, String)> = pf_client_core::profiles::ProfilesFile::load() - .profiles - .into_iter() - .map(|p| (p.id, p.name)) - .collect(); + let profiles: Vec<(String, String, Option)> = + pf_client_core::profiles::ProfilesFile::load() + .profiles + .into_iter() + .map(|p| (p.id, p.name, p.accent)) + .collect(); for k in &known.hosts { // Rust 2021 (no let-chains): match the "this tile is being edited" case explicitly. if matches!(&rename, Some((fp, _)) if fp == &k.fp_hex) { @@ -618,40 +677,42 @@ pub(crate) fn hosts_page(props: &HostsProps, cx: &mut RenderCx) -> Element { .tooltip("More options") .automation_name("More options") .menu_flyout({ + // Kept short deliberately, and in sections. It had grown into a list of + // everything, with the entries you actually reach for (connect, library, + // speed) buried in list management. Two rules thin it: anything that + // CONFIGURES the host lives in the editor — the default profile and the + // pinned tiles are properties of the record, and the editor is where you + // already go to change its name or address — and what is left is + // grouped, so a glance lands on the right third of the menu. let mut items = vec![menu_item(MENU_CONNECT)]; - // One-off connects, flat: this flyout has no submenus, so each - // profile is its own item. "Connect with" NEVER rebinds — the default - // is changed in the host editor (design §5.2). - for (_, name) in profiles.iter() { + // One-off connects, flat: this flyout has no submenus, so each profile + // is its own item. "Connect with" NEVER rebinds the host. + for (_, name, _) in profiles.iter() { items.push(menu_item(format!("{MENU_WITH}{name}"))); } if !profiles.is_empty() { items.push(menu_item(format!("{MENU_WITH}Default settings"))); - for (id, name) in profiles.iter() { - let label = if k.pinned_profiles.iter().any(|p| p == id) { - format!("{MENU_UNPIN}{name}") - } else { - format!("{MENU_PIN}{name}") - }; - items.push(menu_item(label)); - } } - items.push(menu_item(MENU_COPY_LINK)); - items.push(menu_item(MENU_SHORTCUT)); - // The library surfaces — mouse/KB page and the gamepad console UI — - // for paired hosts only (the mgmt API needs the paired identity); - // the page additionally sits behind the experimental toggle, the - // console UI behind the x64-only skia build. + + items.push(menu_separator()); + // The library surfaces — mouse/KB page and the gamepad console UI — for + // paired hosts only (the mgmt API needs the paired identity); the page + // additionally sits behind the experimental toggle. if library_enabled && k.paired { items.push(menu_item(MENU_LIBRARY)); } items.push(menu_item(MENU_SPEED)); - // Offer an explicit wake only when the host is offline and we have a MAC. + // An explicit wake only when the host is offline and we have a MAC. if can_wake { items.push(menu_item(MENU_WAKE)); } - items.push(menu_item(MENU_EDIT)); + items.push(menu_separator()); + items.push(menu_item(MENU_COPY_LINK)); + items.push(menu_item(MENU_SHORTCUT)); + + items.push(menu_separator()); + items.push(menu_item(MENU_EDIT)); items.push(menu_item(MENU_FORGET)); items }) @@ -666,33 +727,12 @@ pub(crate) fn hosts_page(props: &HostsProps, cx: &mut RenderCx) -> Element { target.profile = Some( menu_profiles .iter() - .find(|(_, n)| n == name) - .map(|(id, _)| id.clone()) + .find(|(_, n, _)| n == name) + .map(|(id, _, _)| id.clone()) .unwrap_or_default(), ); initiate(&svc.ctx, target, &svc.set_screen, &svc.set_status) } - _ if item.starts_with(MENU_PIN) || item.starts_with(MENU_UNPIN) => { - let pin = item.starts_with(MENU_PIN); - let name = item - .trim_start_matches(MENU_PIN) - .trim_start_matches(MENU_UNPIN); - let Some((id, _)) = - menu_profiles.iter().find(|(_, n)| n == name).cloned() - else { - return; - }; - let mut known = KnownHosts::load(); - if let Some(h) = known.hosts.iter_mut().find(|h| h.fp_hex == fp) { - h.pinned_profiles.retain(|p| p != &id); - if pin { - h.pinned_profiles.push(id); - } - let _ = known.save(); - } - // The tile list re-reads the store on the next render; nudge it. - sr.call(None); - } MENU_SHORTCUT => { let url = pf_client_core::deeplink::DeepLink::for_host( &shortcut_host, @@ -749,10 +789,12 @@ pub(crate) fn hosts_page(props: &HostsProps, cx: &mut RenderCx) -> Element { 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 — + // that is what makes two bound hosts tell apart at a glance. k.profile_id .as_ref() - .and_then(|id| profiles.iter().find(|(pid, _)| pid == id)) - .map(|(_, name)| (name.as_str(), Pill::Neutral)), + .and_then(|id| profiles.iter().find(|(pid, _, _)| pid == id)) + .map(|(_, name, accent)| (name.as_str(), accent.clone())), ), Some(menu), Some(Box::new(move || { @@ -774,7 +816,7 @@ pub(crate) fn hosts_page(props: &HostsProps, cx: &mut RenderCx) -> Element { // own: a pinned tile is a shortcut, not a second host, and pin/unpin already live // on the primary tile's menu — the one place you decide it. for id in &k.pinned_profiles { - let Some((id, name)) = profiles.iter().find(|(pid, _)| pid == id) else { + let Some((id, name, accent)) = profiles.iter().find(|(pid, ..)| pid == id) else { continue; }; let (ctx3, ss3, st3) = (ctx.clone(), set_screen.clone(), set_status.clone()); @@ -790,7 +832,7 @@ pub(crate) fn hosts_page(props: &HostsProps, cx: &mut RenderCx) -> Element { Some(online), if k.paired { "Paired" } else { "Trusted" }, if k.paired { Pill::Good } else { Pill::Info }, - Some((name.as_str(), Pill::Info)), + Some((name.as_str(), accent.clone())), ), None, Some(Box::new(move || { diff --git a/clients/windows/src/app/settings.rs b/clients/windows/src/app/settings.rs index 6efd58fd..dd478c27 100644 --- a/clients/windows/src/app/settings.rs +++ b/clients/windows/src/app/settings.rs @@ -118,10 +118,90 @@ const NEW_PROFILE: &str = "\u{1}new"; /// Rename / duplicate / delete for the profile in scope. Rename is a text box rather than a /// modal because this toolkit's ContentDialog is text-only (the same constraint that put the /// host editor in a tile); it commits on change, which is how every other control here works. +/// The chip palette a profile can carry (`StreamProfile.accent`), same set as the GTK client so +/// a profile looks the same on both. Eight legible colours rather than a free picker: the job is +/// telling profiles apart at a glance on a host tile, and the schema still accepts any +/// `#RRGGBB` a hand-edit writes. +const SWATCHES: &[(&str, &str)] = &[ + ("", "None"), + ("#e01b24", "Red"), + ("#ff7800", "Orange"), + ("#f6d32d", "Yellow"), + ("#33d17a", "Green"), + ("#3584e4", "Blue"), + ("#9141ac", "Purple"), + ("#d16d9e", "Pink"), + ("#77767b", "Slate"), +]; + +/// `#RRGGBB` to a brush colour. Anything else is refused rather than guessed at — the value is +/// user data and reaches the renderer. +pub(crate) fn hex_color(hex: &str) -> Option { + let h = hex.strip_prefix('#')?; + if h.len() != 6 || !h.chars().all(|c| c.is_ascii_hexdigit()) { + return None; + } + Some(Color { + a: 255, + r: u8::from_str_radix(&h[0..2], 16).ok()?, + g: u8::from_str_radix(&h[2..4], 16).ok()?, + b: u8::from_str_radix(&h[4..6], 16).ok()?, + }) +} + +/// The colour row: one tappable swatch per palette entry, the current one ringed. +fn colour_swatches(profile: &StreamProfile, rev: u64, set_rev: &AsyncSetState) -> Element { + let current = profile.accent.clone().unwrap_or_default(); + let mut row: Vec = vec![text_block("Colour") + .font_size(12.0) + .foreground(ThemeRef::SecondaryText) + .vertical_alignment(VerticalAlignment::Center) + .margin(edges(0.0, 0.0, 6.0, 0.0)) + .into()]; + for (hex, name) in SWATCHES { + let selected = current == *hex; + // "None" (and anything unparsable) draws as a faint neutral disc, so the row still + // reads as a palette with a clear "no colour" end. + let fill = hex_color(hex).unwrap_or(Color { + a: 40, + r: 128, + g: 128, + b: 128, + }); + let (id, set_rev, hex_owned) = (profile.id.clone(), set_rev.clone(), hex.to_string()); + row.push( + border(vstack(Vec::::new()).width(20.0).height(20.0)) + .background(fill) + .corner_radius(10.0) + .border_brush(if selected { + ThemeRef::Accent + } else { + ThemeRef::CardStroke + }) + .border_thickness(uniform(if selected { 2.0 } else { 1.0 })) + .tooltip(*name) + .on_tapped(move || { + let mut catalog = ProfilesFile::load(); + if let Some(p) = catalog.profiles.iter_mut().find(|p| p.id == id) { + p.accent = (!hex_owned.is_empty()).then(|| hex_owned.clone()); + if let Err(e) = catalog.save() { + tracing::warn!(error = %format!("{e:#}"), "saving the profile colour"); + } + } + set_rev.call(rev + 1); + }) + .into(), + ); + } + hstack(row).spacing(8.0).into() +} + fn profile_actions( profile: &StreamProfile, set_scope: &AsyncSetState, set_delete: &AsyncSetState>, + rev: u64, + set_rev: &AsyncSetState, ) -> Element { let id = profile.id.clone(); let name_box = { @@ -171,7 +251,12 @@ fn profile_actions( button("Delete\u{2026}").on_click(move || set_delete.call(Some(id.clone()))) }; described( - vstack((name_box, hstack((duplicate, delete)).spacing(8.0))).spacing(8.0), + vstack(( + name_box, + colour_swatches(profile, rev, set_rev), + hstack((duplicate, delete)).spacing(8.0), + )) + .spacing(10.0), "Renaming applies as you type. Deleting leaves hosts that used it on Default settings.", ) } @@ -1067,10 +1152,18 @@ pub(crate) fn settings_page( "A profile overrides only what you change while it is selected; everything else \ follows Default settings.", ); - let mut header_rows = vec![switcher]; + // The switcher is CHROME, not a setting: it belongs above the whole surface, so it reads as + // "which layer am I editing" and is visible from every section. Inside the section content + // it looked like one more row — and on the About page, where there are barely any rows, it + // looked like it lived there. + let mut bar_rows: Vec = vec![switcher]; if let Some(p) = &active { - header_rows.push(profile_actions(p, set_scope, set_delete)); + bar_rows.push(profile_actions(p, set_scope, set_delete, rev, set_rev)); } + let scope_bar = card(vstack(bar_rows).spacing(10.0)) + .margin(edges(24.0, 12.0, 24.0, 0.0)) + .into(); + let titled: Vec = std::iter::once( text_block(title) .font_size(28.0) @@ -1079,7 +1172,6 @@ pub(crate) fn settings_page( .margin(edges(0.0, 0.0, 0.0, 6.0)) .into(), ) - .chain(group(None, header_rows, None)) .chain(groups) .collect(); // The keyed column MUST sit inside a panel's child list, not directly under the @@ -1171,8 +1263,9 @@ pub(crate) fn settings_page( let ss = set_screen.clone(); move || ss.call(Screen::Hosts) }); - match confirm { - Some(dialog) => vstack(vec![nav.into(), dialog]).into(), - None => nav.into(), + let mut surface: Vec = vec![scope_bar, nav.into()]; + if let Some(dialog) = confirm { + surface.push(dialog); } + vstack(surface).into() }