diff --git a/clients/windows/src/app/hosts.rs b/clients/windows/src/app/hosts.rs index 6ec2a6ea..87c91a91 100644 --- a/clients/windows/src/app/hosts.rs +++ b/clients/windows/src/app/hosts.rs @@ -40,14 +40,18 @@ const TILE_GAP: f64 = 12.0; /// Props for the hosts page: the services plus the changing discovery/status data that must /// drive its re-render (compared by value, so a new host list or error refreshes the page). /// -/// `forget` and `rename` are the per-host action state, and they live in ROOT (not this page's -/// own `use_state`) on purpose: the "…" overflow is a WinUI `MenuFlyout`, whose item clicks are -/// wired directly in the reactor backend (`add_Click`) and so bypass the normal event-dispatch -/// flush — a *sync* child `SetState` from that handler marks state dirty but never pumps the -/// reconciler, so nothing re-renders. Root `AsyncSetState` re-renders the whole tree; because -/// these values are props, the changed value propagates back into this page (a child's own async -/// state would be memoised away when its props are unchanged). `(fp_hex, _)` in each identifies -/// the target saved host; `rename`'s second field is the in-progress draft name. +/// `forget`, `rename` and `hover` used to live here too, hoisted into root on the belief that +/// MenuFlyout item clicks and pointer enter/exit — wired straight in the reactor backend +/// (`add_Click` etc.) — bypassed the render flush, so a *sync* child `SetState` from such a +/// handler would never repaint. The characterization suite disproved that at the current pin +/// (`tests/reactor_semantics.rs`: `sync_state_from_backend_fired_event_rerenders`, +/// `sync_state_child_under_element_equal_border_rerenders` — a sync `use_state` write +/// re-renders its owner, from backend-fired handlers and under element-equal wrappers alike), +/// so they moved back down into `hosts_page`'s own `use_state` on 2026-07-29. +/// +/// `show_add` is the one that STAYS root, for a different reason: the add-modal entrance +/// tween in `app/mod.rs` is keyed on it, and the tween worker thread writes root async state +/// (`add_anim`) — root can only start the tween if it owns the trigger. #[derive(Clone)] pub(crate) struct HostsProps { pub(crate) svc: Svc, @@ -60,21 +64,12 @@ pub(crate) struct HostsProps { /// Connected-controller count (root state, mirrored from the gamepad service) — a /// pad plus a paired host surfaces the "Open console UI" hint card. pub(crate) pads: usize, - pub(crate) forget: Option<(String, String)>, - pub(crate) rename: Option<(String, String)>, - /// Whether the "Add host" modal is open. Root state (like `forget`/`rename`), not the page's - /// own `use_state`: a child component's sync `SetState` marks its slot dirty but does not - /// re-render when its props are otherwise unchanged, so the toggle wouldn't take. + /// Whether the "Add host" modal is open. Root state (see the struct docs: the entrance + /// tween in `app/mod.rs` keys on it). pub(crate) show_add: bool, /// The modal's entrance-tween progress (0 → 1, root-driven): opacity + slide-up offset. pub(crate) add_anim: f64, - /// The hovered tile's stable id (saved: fp_hex, discovered: `addr:port`) — root state because - /// the pointer enter/exit handlers bypass the reconciler flush, like the flyout clicks above. - pub(crate) hover: Option, - pub(crate) set_forget: AsyncSetState>, - pub(crate) set_rename: AsyncSetState>, pub(crate) set_show_add: AsyncSetState, - pub(crate) set_hover: AsyncSetState>, } impl PartialEq for HostsProps { @@ -85,11 +80,8 @@ impl PartialEq for HostsProps { && self.probed == other.probed && self.status == other.status && self.pads == other.pads - && self.forget == other.forget - && self.rename == other.rename && self.show_add == other.show_add && self.add_anim == other.add_anim - && self.hover == other.hover } } @@ -161,10 +153,12 @@ fn host_tile( .into() } -/// The hover-tracking pair `host_tile` needs: the currently hovered tile id + its root setter. +/// The hover-tracking pair `host_tile` needs: the currently hovered tile id + the page's own +/// sync setter (pointer enter/exit handlers re-render this component reliably — measured by +/// `sync_state_from_backend_fired_event_rerenders`). pub(crate) struct Hover { pub(crate) current: Option, - pub(crate) set: AsyncSetState>, + pub(crate) set: SetState>, } /// The status row at the bottom of a tile: the host's OS mark (when advertised), presence @@ -259,7 +253,7 @@ fn edit_editor( port_draft: HookRef, mac_draft: HookRef, clip_draft: HookRef, - set_edit: AsyncSetState>, + set_edit: SetState>, ) -> Element { let commit = { let (fp, se) = (fp.to_string(), set_edit.clone()); @@ -478,15 +472,19 @@ pub(crate) fn hosts_page(props: &HostsProps, cx: &mut RenderCx) -> Element { // would connect to the empty mount-time value. Mirror every keystroke into this stable ref (the // pair-screen PIN pattern). `manual` still drives the text box's displayed value. let manual_live = cx.use_ref(String::new()); - // "Add host" modal open state lives in ROOT (see `HostsProps`). + // "Add host" modal open state lives in ROOT (see `HostsProps`: the entrance tween keys on it). let show_add = props.show_add; let set_show_add = &props.set_show_add; - // Forget confirmation and in-progress rename live in ROOT state (see `HostsProps`) — the - // overflow menu's flyout clicks can't re-render off a sync setter. Both are `(fp_hex, _)`. - let forget = props.forget.clone(); - let rename = props.rename.clone(); - let set_forget = &props.set_forget; - let set_rename = &props.set_rename; + // Forget confirmation and in-progress edit target, `(fp_hex, name)` each. Page-local sync + // state (moved back down from root 2026-07-29): the flyout's item clicks fire through the + // backend attach path, and a sync `use_state` write from such a handler re-renders this + // component reliably (`sync_state_from_backend_fired_event_rerenders`). + let (forget, set_forget) = cx.use_state(Option::<(String, String)>::None); + let (rename, set_rename) = cx.use_state(Option::<(String, String)>::None); + // The hovered tile's stable id (saved: fp_hex, discovered: `addr:port`), driving the + // WinUI-style card hover fill. Same measured rule as above for pointer enter/exit. + let (hover_id, set_hover) = cx.use_state(Option::::None); + let (set_forget, set_rename) = (&set_forget, &set_rename); // The live edit drafts, read at Save time (see `edit_editor`). Root `rename` carries only // the target's fingerprint + initial name, so typing never round-trips through a // re-render. Every draft is re-seeded from the STORED host whenever the edit target @@ -525,8 +523,8 @@ pub(crate) fn hosts_page(props: &HostsProps, cx: &mut RenderCx) -> Element { } } let hover = Hover { - current: props.hover.clone(), - set: props.set_hover.clone(), + current: hover_id, + set: set_hover, }; let known = KnownHosts::load(); // The experimental library gate ("Show game library" in Settings) — GTK/Apple parity. diff --git a/clients/windows/src/app/mod.rs b/clients/windows/src/app/mod.rs index 82bcc6f5..e8ab0c26 100644 --- a/clients/windows/src/app/mod.rs +++ b/clients/windows/src/app/mod.rs @@ -30,6 +30,10 @@ //! (`async_state_child_under_element_equal_border_rerenders` asserts the drop). So //! everything THREAD-driven (discovery, HUD stats, speed-test results, spawn events) //! is held as *root* state and passed down as props. +//! * Corollary: state a root tween is KEYED on (`screen`, the settings section, `show_add`) +//! also stays in root even though it is user-event-driven — the tween workers are root +//! `use_effect`s writing root async state, and root can only observe the trigger (and +//! start the tween) if it owns it. mod connect; mod help; @@ -100,8 +104,8 @@ pub(crate) struct Target { } /// Stable app services handed to the page components as props. Each routed screen that uses -/// hooks (`hosts_page`/`pair_page`/`speed_page`/`library_page`) is mounted as its own -/// `component(...)`, so its hooks live in an isolated slot list — calling them on the shared +/// hooks (`hosts_page`/`settings_page`/`pair_page`/`speed_page`/`library_page`) is mounted as +/// its own `component(...)`, so its hooks live in an isolated slot list — calling them on the shared /// parent `cx` would change the hook order whenever the screen changes (reactor's /// Rules-of-Hooks guard aborts). /// @@ -259,40 +263,23 @@ fn root(cx: &mut RenderCx, ctx: &Arc) -> Element { let (status, set_status) = cx.use_async_state(String::new()); let (hud, set_hud) = cx.use_async_state(stream::HudSample::default()); let (speed, set_speed) = cx.use_async_state(SpeedState::Running); - // Per-host action state for the hosts page. Root, not page-local: the "…" overflow is a WinUI - // MenuFlyout whose item clicks are wired straight in the reactor backend, bypassing the normal - // event-dispatch flush — a sync page-local setter marks state dirty but never re-renders. See - // `hosts::HostsProps`. - let (forget, set_forget) = cx.use_async_state(Option::<(String, String)>::None); - let (rename, set_rename) = cx.use_async_state(Option::<(String, String)>::None); + // "Add host" modal open state. Root, not hosts-page-local — NOT for a re-render reason + // (a page-local sync setter would repaint fine per the measured rules) but because the + // modal's entrance tween below is keyed on it, and the tween worker thread writes root + // async state (`add_anim`): root can only start the tween if it owns the trigger. The + // page's other action state (forget/rename/hover) moved into `hosts_page` 2026-07-29. let (show_add, set_show_add) = cx.use_async_state(false); - // Hovered host tile (its stable id), driving the WinUI-style card hover fill. Root state for - // the same reason as `forget`/`rename`: pointer enter/exit handlers are wired straight in the - // reactor backend, so only a root `AsyncSetState` reliably re-renders the page. - let (hover, set_hover) = cx.use_async_state(Option::::None); // Which Settings section the NavigationView shows (persists across visits this run). // Opens on General — the first sidebar item, matching the Apple client's landing category. + // Root for the same tween-coupling reason as `show_add`: the section-switch entrance tween + // below keys on it. The page's other UI state (scope/delete/edit/revision) is its own + // sync `use_state` — see `settings::SettingsProps`. let (settings_nav, set_settings_nav) = cx.use_async_state("general".to_string()); - // Which LAYER the settings screen edits: "" = the global defaults, else a profile id - // (design/client-settings-profiles.md §5.1). Root state for the same reason as the section - // above — the ComboBox's change handler is wired in the reactor backend. - let (settings_scope, set_settings_scope) = cx.use_async_state(String::new()); - // The profile a Delete… click is asking about; `Some` renders the confirmation. Root state - // because this page stays hook-free (its handlers are wired in the reactor backend). - let (settings_delete, set_settings_delete) = cx.use_async_state(Option::::None); - // Whether the Edit-profile modal is up. Root state for the reactor-backend-handler reason - // above; guarded in the page so it only renders while a profile is actually in scope. - let (settings_edit, set_settings_edit) = cx.use_async_state(false); // Window size, read at ROOT: the settings screen places its scope switcher by the same // width threshold WinUI's NavigationView uses to collapse its pane (reactor exposes no // pane-opened/closed event). Registering the size here re-renders the tree on resize, // which the per-screen `use_inner_size` readers (hosts, library) already caused anyway. let window = cx.use_inner_size(); - // Bumped when a settings edit changes what the page should SHOW without changing any state - // it already reads — ANY edit through `settings::commit` (creating an override must surface - // its marker as immediately as resetting one clears it), a reset, a profile colour change. - // Root state comparison makes same-value calls free, so a counter is what forces the pass. - let (settings_rev, set_settings_rev) = cx.use_async_state(0u64); // `punktfunk://` links: the receiver thread queues them (from this launch's argv, or from a // later instance over WM_COPYDATA) and this poll pulls them onto the UI thread. Thread-fed // state must be root state, like the pad count below. @@ -610,37 +597,26 @@ fn root(cx: &mut RenderCx, ctx: &Arc) -> Element { probed, status, pads, - forget, - rename, show_add, add_anim, - hover, - set_forget, - set_rename, set_show_add, - set_hover, }, ), - // connecting_page / request_access_page / waking_page / settings_page / licenses_page / - // help_page use no hooks (they never touch `cx`), so calling them inline is sound. + // connecting_page / request_access_page / waking_page / licenses_page / help_page use + // no hooks (they never touch `cx`), so calling them inline is sound. Screen::Connecting => connect::connecting_page(ctx, &status), Screen::RequestAccess => connect::request_access_page(ctx, &set_screen), Screen::Waking => connect::waking_page(ctx, &set_screen), - Screen::Settings => settings::settings_page( - ctx, - &set_screen, - &settings_nav, - &set_settings_nav, - &settings_scope, - &set_settings_scope, - &settings_delete, - &set_settings_delete, - settings_edit, - &set_settings_edit, - settings_rev, - &set_settings_rev, - nav_progress, - window.width, + Screen::Settings => component( + settings::settings_page, + settings::SettingsProps { + ctx: ctx.clone(), + set_screen: set_screen.clone(), + section: settings_nav.clone(), + set_section: set_settings_nav.clone(), + progress: nav_progress, + window_width: window.width, + }, ), Screen::Licenses => licenses::licenses_page(&set_screen), Screen::Help => help::help_page(&set_screen), diff --git a/clients/windows/src/app/settings.rs b/clients/windows/src/app/settings.rs index 244c6ff9..4198af52 100644 --- a/clients/windows/src/app/settings.rs +++ b/clients/windows/src/app/settings.rs @@ -147,7 +147,7 @@ pub(crate) fn hex_color(hex: &str) -> Option { } /// The colour row: one tappable swatch per palette entry, the current one ringed. -fn colour_swatches(profile: &StreamProfile, rev: u64, set_rev: &AsyncSetState) -> Element { +fn colour_swatches(profile: &StreamProfile, rev: u64, set_rev: &SetState) -> Element { let current = profile.accent.clone().unwrap_or_default(); let mut row: Vec = vec![text_block("Colour") .font_size(12.0) @@ -202,11 +202,11 @@ fn colour_swatches(profile: &StreamProfile, rev: u64, set_rev: &AsyncSetState, - set_delete: &AsyncSetState>, - set_edit: &AsyncSetState, + set_scope: &SetState, + set_delete: &SetState>, + set_edit: &SetState, rev: u64, - set_rev: &AsyncSetState, + set_rev: &SetState, ) -> Element { let id = profile.id.clone(); let name_box = { @@ -320,7 +320,7 @@ fn edit_profile_modal( fn commit( ctx: &Arc, scope: &str, - rev: (u64, &AsyncSetState), + rev: (u64, &SetState), edit: impl FnOnce(&mut Settings), ) { if scope.is_empty() { @@ -405,7 +405,7 @@ fn active_profile(scope: &str) -> Option { fn setting_combo( ctx: &Arc, scope: &str, - rev: (u64, &AsyncSetState), + rev: (u64, &SetState), header: &str, names: Vec, current: usize, @@ -435,7 +435,7 @@ fn presets(table: &[(V, &str)], is_current: impl Fn(&V) -> bool) -> (Vec, scope: &str, - rev: (u64, &AsyncSetState), + rev: (u64, &SetState), header: &str, on: bool, apply: impl Fn(&mut Settings, bool) + 'static, @@ -466,7 +466,7 @@ fn setting_toggle( /// its row, and "not overridden" needs an explicit Reset. (Linux marks a literal no-op /// touch too — unobservable here, the one intentional divergence.) fn described_overridable( - rev: (u64, &AsyncSetState), + rev: (u64, &SetState), scope: &str, field: &'static str, overridden: bool, @@ -559,32 +559,77 @@ fn group(header: Option<&str>, fields: Vec, footer: Option<&str>) -> Ve out } +/// Props for the settings page (mounted as its own `component(...)` so its hooks live in an +/// isolated slot list — see `Svc`'s docs in `app/mod.rs`). +/// +/// What is a prop here (vs the page's own `use_state`) is deliberate: +/// * `section` + `set_section` — the selected pane tag stays ROOT state because the +/// section-switch entrance tween in `app/mod.rs` is keyed on it: the tween worker thread +/// writes root async state (`nav_anim`), and root can only start it if it owns the trigger. +/// The NavigationView's `on_selection_changed` therefore keeps calling the root setter. +/// * `progress` — that tween's value (0 → 1), root-driven for the same reason. +/// * `window_width` — read at root (`use_inner_size`) so resize re-renders the tree; the page +/// only compares it against WinUI's pane-collapse threshold. +/// +/// Everything else the page shows (scope, delete confirmation, edit modal, revision) is its +/// own sync `use_state` — see `settings_page`. +#[derive(Clone)] +pub(crate) struct SettingsProps { + pub(crate) ctx: Arc, + pub(crate) set_screen: AsyncSetState, + pub(crate) section: String, + pub(crate) set_section: AsyncSetState, + pub(crate) progress: f64, + pub(crate) window_width: f64, +} + +impl PartialEq for SettingsProps { + fn eq(&self, other: &Self) -> bool { + // Setters are identity-stable; only the data fields drive re-render. + Arc::ptr_eq(&self.ctx, &other.ctx) + && self.section == other.section + && self.progress == other.progress + && self.window_width == other.window_width + } +} + /// The settings screen: a stock WinUI `NavigationView` (the Windows-Settings sidebar pattern) — /// one pane item per section, the section's card as the content, the built-in back arrow -/// returning to the host list. `section`/`set_section` are the selected pane tag, held in ROOT -/// state (this page stays hook-free): `on_selection_changed` is wired in the reactor backend, so -/// only a root `AsyncSetState` reliably re-renders the new section in. `progress` is the -/// section-switch entrance tween (0 → 1), mapped onto the content column's opacity + offset. -#[allow(clippy::too_many_arguments)] -pub(crate) fn settings_page( - ctx: &Arc, - set_screen: &AsyncSetState, - section: &str, - set_section: &AsyncSetState, - scope_id: &str, - set_scope: &AsyncSetState, - delete_pending: &Option, - set_delete: &AsyncSetState>, - edit_open: bool, - set_edit: &AsyncSetState, - rev: u64, - set_rev: &AsyncSetState, - progress: f64, - window_width: f64, -) -> Element { +/// returning to the host list. +/// +/// The page-only UI state (which layer is edited, the delete confirmation, the Edit-profile +/// modal, the repaint revision) lives HERE as sync `use_state`, de-hoisted from root +/// 2026-07-29: every writer is a UI-thread handler (ComboBox `on_selection_changed`, button +/// clicks, swatch taps), and the characterization suite measured that a sync `use_state` +/// write from a backend-wired handler re-renders its owning component +/// (`tests/reactor_semantics.rs::sync_state_from_backend_fired_event_rerenders`). The section +/// tag is the exception — root-owned, tween-coupled; see `SettingsProps`. +pub(crate) fn settings_page(props: &SettingsProps, cx: &mut RenderCx) -> Element { + // Hooks first, unconditionally, in a stable order (reactor's Rules-of-Hooks guard aborts + // on a changed order). + // Which LAYER the screen edits: "" = the global defaults, else a profile id + // (design/client-settings-profiles.md §5.1). + let (scope_id, set_scope) = cx.use_state(String::new()); + // The profile a Delete… click is asking about; `Some` renders the confirmation. + let (delete_pending, set_delete) = cx.use_state(Option::::None); + // Whether the Edit-profile modal is up; guarded below so it only renders while a profile + // is actually in scope. + let (edit_open, set_edit) = cx.use_state(false); + // Bumped when an edit changes what the page should SHOW without changing any state it + // already reads — ANY edit through `commit` (creating an override must surface its marker + // as immediately as resetting one clears it), a reset, a profile colour change. The + // same-value compare in `call` makes no-op bumps free; the counter is what forces the pass. + let (rev, set_rev) = cx.use_state(0u64); + let (set_scope, set_delete, set_edit, set_rev) = (&set_scope, &set_delete, &set_edit, &set_rev); + let ctx = &props.ctx; + let set_screen = &props.set_screen; + let section = props.section.as_str(); + let set_section = &props.set_section; + let progress = props.progress; + let window_width = props.window_width; // The layer being edited. A scope pointing at a deleted profile degrades to the defaults, // the same rule a dangling host binding follows. - let active = active_profile(scope_id); + let active = active_profile(&scope_id); let scope: &str = match &active { Some(p) => &p.id, None => "",