From a9d3ca2fb10130eed2f32a330f59467a689493f8 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Wed, 29 Jul 2026 18:22:04 +0200 Subject: [PATCH] feat(client/windows): the sheets learn to be dismissed, and the profile sheet says Save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Escape closes every overlay — the profile sheet, the host editor, the add-host modal — and so does a tap on the scrim. The scrim-tap needed one trick: WinUI bubbles `Tapped` out of the card into the scrim (reactor cannot mark it handled), so the card raises a shared flag first and the scrim's handler swallows exactly that tap; only a genuine outside tap dismisses. The profile sheet's dismiss button reads Save (with the Save glyph) instead of Close — every field commits as you type, so Save is the promise the button already kept, and a sheet full of edits wants a verb. Co-Authored-By: Claude Fable 5 --- clients/windows/src/app/hosts.rs | 69 ++++++++++++++++++++++++----- clients/windows/src/app/settings.rs | 65 +++++++++++++++++++-------- 2 files changed, 103 insertions(+), 31 deletions(-) diff --git a/clients/windows/src/app/hosts.rs b/clients/windows/src/app/hosts.rs index f53e26f4..cee80ccf 100644 --- a/clients/windows/src/app/hosts.rs +++ b/clients/windows/src/app/hosts.rs @@ -396,6 +396,13 @@ fn edit_editor( // below the fold with nothing hinting at it (live-diagnosed 2026-07-29: a control's // visible rect was a 9-px sliver). A sheet centres at its own height — and its content // sits in a scroll_view, so a short window scrolls the card instead of clipping it. + // A tap on the scrim, or Escape, cancels (a tap INSIDE the card bubbles to the scrim — + // the flag makes the scrim swallow exactly that one). + let inside_tap = std::rc::Rc::new(std::cell::Cell::new(false)); + let cancel = { + let se = set_edit.clone(); + move || se.call(None) + }; let modal = dialog_surface(scroll_view( vstack(( text_block(format!("Edit \u{201c}{initial_name}\u{201d}")) @@ -452,19 +459,35 @@ fn edit_editor( )) .spacing(10.0), )) + .on_tapped({ + let inside_tap = inside_tap.clone(); + move || inside_tap.set(true) + }) .max_width(460.0) .horizontal_alignment(HorizontalAlignment::Center) .vertical_alignment(VerticalAlignment::Center) .margin(uniform(24.0)); - // The scrim: blocks the page, closes only via Save/Cancel (same rules as the add modal). - border(modal) - .background(Color { - a: 140, - r: 0, - g: 0, - b: 0, - }) - .into() + let scrim_cancel = cancel.clone(); + Element::from( + border(modal) + .background(Color { + a: 140, + r: 0, + g: 0, + b: 0, + }) + .on_tapped(move || { + if inside_tap.replace(false) { + return; + } + scrim_cancel(); + }), + ) + .keyboard_accelerator(KeyboardAccelerator::new( + VirtualKey::Escape, + VirtualKeyModifiers::None, + cancel, + )) } pub(crate) fn hosts_page(props: &HostsProps, cx: &mut RenderCx) -> Element { @@ -1074,16 +1097,38 @@ pub(crate) fn hosts_page(props: &HostsProps, cx: &mut RenderCx) -> Element { // 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. + // list never changes shape around the always-mounted dialog after it. A tap on the + // scrim, or Escape, cancels — with the same bubble-swallow flag the sheets use. let add_slot: Element = if show_add { - border(modal) + let inside_tap = std::rc::Rc::new(std::cell::Cell::new(false)); + let cancel = { + let sa = set_show_add.clone(); + move || sa.call(false) + }; + let scrim_cancel = cancel.clone(); + Element::from( + border(Element::from(modal).on_tapped({ + let inside_tap = inside_tap.clone(); + move || inside_tap.set(true) + })) .background(Color { a: (140.0 * props.add_anim) as u8, r: 0, g: 0, b: 0, }) - .into() + .on_tapped(move || { + if inside_tap.replace(false) { + return; + } + scrim_cancel(); + }), + ) + .keyboard_accelerator(KeyboardAccelerator::new( + VirtualKey::Escape, + VirtualKeyModifiers::None, + cancel, + )) } else { border(vstack(Vec::::new())).into() }; diff --git a/clients/windows/src/app/settings.rs b/clients/windows/src/app/settings.rs index 7dcc2a69..c03d7005 100644 --- a/clients/windows/src/app/settings.rs +++ b/clients/windows/src/app/settings.rs @@ -303,18 +303,25 @@ fn edit_profile_modal( .into(), ); } + // "Save", not "Close": every field in the sheet commits as you type, so this is really + // "done" — but the review is right that a sheet full of edits wants a verb, and Save + // is the promise the button already keeps. + let close_sheet = { + let (set_edit, set_rev) = (set_edit.clone(), set_rev.clone()); + move || { + set_edit.call(false); + // The deferred repaint: the bar dropdown (and any pinned tiles) pick up the + // rename now, in one pass, instead of remounting per keystroke. + set_rev.call(rev + 1); + } + }; buttons.push( { - let (set_edit, set_rev) = (set_edit.clone(), set_rev.clone()); - button("Close") + let close_sheet = close_sheet.clone(); + button("Save") .accent() - .icon(Symbol::Accept) - .on_click(move || { - set_edit.call(false); - // The deferred repaint: the pane dropdown (and any pinned tiles) pick up the - // rename now, in one pass, instead of remounting per keystroke. - set_rev.call(rev + 1); - }) + .icon(Symbol::Save) + .on_click(close_sheet) } .into(), ); @@ -327,21 +334,41 @@ fn edit_profile_modal( ); // The content scrolls when the window is shorter than the sheet (same rule as the host // editor) — a sheet must never clip its own controls. + // A tap INSIDE the card bubbles up to the scrim (WinUI bubbles `Tapped`; reactor can't + // mark it handled), so the card raises this flag first and the scrim's handler swallows + // exactly that tap — a tap on the scrim itself, and Escape, dismiss the sheet. + let inside_tap = std::rc::Rc::new(std::cell::Cell::new(false)); let modal = dialog_surface(scroll_view(vstack(rows).spacing(12.0))) + .on_tapped({ + let inside_tap = inside_tap.clone(); + move || inside_tap.set(true) + }) .max_width(420.0) .horizontal_alignment(HorizontalAlignment::Center) .vertical_alignment(VerticalAlignment::Center) .margin(uniform(24.0)); - // The scrim fills the cell and is hit-testable, so it blocks the page behind; it closes - // only via the buttons (a scrim tap would bubble `Tapped` up from the card too). - border(modal) - .background(Color { - a: 140, - r: 0, - g: 0, - b: 0, - }) - .into() + let scrim_close = close_sheet.clone(); + let esc_close = close_sheet; + Element::from( + border(modal) + .background(Color { + a: 140, + r: 0, + g: 0, + b: 0, + }) + .on_tapped(move || { + if inside_tap.replace(false) { + return; + } + scrim_close(); + }), + ) + .keyboard_accelerator(KeyboardAccelerator::new( + VirtualKey::Escape, + VirtualKeyModifiers::None, + esc_close, + )) } /// Persist one control's edit into the layer being edited.