From 4700bd79126905b2fa22c35e8e7ed0e69f4fb291 Mon Sep 17 00:00:00 2001 From: funman300 Date: Wed, 8 Jul 2026 20:31:55 -0700 Subject: [PATCH] fix(engine): close Settings, Help, Leaderboard, and theme store on Esc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase C dismissal audit: Esc / scrim-tap / Done must behave the same on every modal. Stragglers found and fixed — none of these had any Esc path (pause's toggle guard swallowed the key while they were open): - Settings: Esc clears SettingsScreen, gated on being the topmost modal so a stacked sync-setup / theme-store dialog owns Esc - Help: Esc closes alongside F1/Done (the code comment already claimed an Esc path existed — now it does) - Leaderboard: Esc closes when topmost; the display-name dialog stacked above it now Esc-cancels like sync-setup's dialog - Theme store: Esc closes (always topmost when open) Scrim-tap opt-ins are unchanged — ui_modal documents which modals deliberately stay non-dismissible on outside clicks. Tests: escape_closes_help_screen, escape_closes_settings_screen_flag. Co-Authored-By: Claude Fable 5 --- solitaire_engine/src/help_plugin.rs | 38 ++++++++++++++++++- solitaire_engine/src/leaderboard_plugin.rs | 18 ++++++++- solitaire_engine/src/settings_plugin/input.rs | 8 +++- solitaire_engine/src/settings_plugin/tests.rs | 25 ++++++++++++ solitaire_engine/src/theme_store_plugin.rs | 23 +++++++---- 5 files changed, 99 insertions(+), 13 deletions(-) diff --git a/solitaire_engine/src/help_plugin.rs b/solitaire_engine/src/help_plugin.rs index a22950a..76199ab 100644 --- a/solitaire_engine/src/help_plugin.rs +++ b/solitaire_engine/src/help_plugin.rs @@ -86,13 +86,18 @@ fn toggle_help_screen( } /// Click handler for the modal's "Done" button. F1 toggles the overlay -/// the same way; this just exposes the close action to mouse / touch. +/// the same way; Esc closes too, so dismissal matches every other +/// modal (Phase C dismissal audit). Nothing ever stacks above Help, +/// so Esc needs no topmost gate. fn handle_help_close_button( mut commands: Commands, + keys: Res>, close_buttons: Query<&Interaction, (With, Changed)>, screens: Query>, ) { - if !close_buttons.iter().any(|i| *i == Interaction::Pressed) { + let clicked = close_buttons.iter().any(|i| *i == Interaction::Pressed); + let esc = keys.just_pressed(KeyCode::Escape) && !screens.is_empty(); + if !clicked && !esc { return; } for entity in &screens { @@ -583,4 +588,33 @@ mod tests { 0 ); } + + /// Esc must dismiss the Help modal like Done and F1 do (Phase C + /// dismissal audit). + #[test] + fn escape_closes_help_screen() { + let mut app = headless_app(); + app.world_mut() + .resource_mut::>() + .press(KeyCode::F1); + app.update(); + + { + let mut input = app.world_mut().resource_mut::>(); + input.release(KeyCode::F1); + input.clear(); + input.press(KeyCode::Escape); + } + app.update(); + app.update(); + + assert_eq!( + app.world_mut() + .query::<&HelpScreen>() + .iter(app.world()) + .count(), + 0, + "Esc must close the Help modal" + ); + } } diff --git a/solitaire_engine/src/leaderboard_plugin.rs b/solitaire_engine/src/leaderboard_plugin.rs index 0803fd7..49df78a 100644 --- a/solitaire_engine/src/leaderboard_plugin.rs +++ b/solitaire_engine/src/leaderboard_plugin.rs @@ -343,13 +343,21 @@ fn scroll_leaderboard_panel( } } +/// Done click or Esc dismisses the leaderboard (Phase C dismissal +/// audit). Esc only fires when the leaderboard is the topmost modal — +/// with the display-name dialog stacked on top, that dialog owns Esc. fn handle_leaderboard_close_button( mut commands: Commands, + keys: Res>, close_buttons: Query<&Interaction, (With, Changed)>, screens: Query>, + other_modal_scrims: Query<(), (With, Without)>, mut closed_flag: ResMut, ) { - if !close_buttons.iter().any(|i| *i == Interaction::Pressed) { + let clicked = close_buttons.iter().any(|i| *i == Interaction::Pressed); + let esc = + keys.just_pressed(KeyCode::Escape) && !screens.is_empty() && other_modal_scrims.is_empty(); + if !clicked && !esc { return; } for entity in &screens { @@ -888,12 +896,18 @@ fn handle_display_name_confirm( } /// Discards any typed text and closes the display-name editor modal. +/// Cancel click or Esc dismisses the display-name dialog without +/// saving (Phase C dismissal audit — same contract as the sync-setup +/// dialog's Cancel/Esc pair). fn handle_display_name_cancel( button_q: Query<&Interaction, (Changed, With)>, + keys: Res>, screens: Query>, mut commands: Commands, ) { - if !button_q.iter().any(|i| *i == Interaction::Pressed) { + let clicked = button_q.iter().any(|i| *i == Interaction::Pressed); + let esc = keys.just_pressed(KeyCode::Escape) && !screens.is_empty(); + if !clicked && !esc { return; } for entity in &screens { diff --git a/solitaire_engine/src/settings_plugin/input.rs b/solitaire_engine/src/settings_plugin/input.rs index f69c679..72c3676 100644 --- a/solitaire_engine/src/settings_plugin/input.rs +++ b/solitaire_engine/src/settings_plugin/input.rs @@ -50,15 +50,21 @@ pub(super) fn handle_volume_keys( } /// Opens or closes the Settings panel — `O` keyboard accelerator or -/// `ToggleSettingsRequestEvent` from the HUD Menu popover. +/// `ToggleSettingsRequestEvent` from the HUD Menu popover. Esc closes +/// too (Phase C dismissal audit), but only when Settings is the +/// topmost modal — with sync-setup or the theme store stacked on top, +/// the stacked dialog owns Esc. pub(super) fn toggle_settings_screen( keys: Res>, mut requests: MessageReader, mut screen: ResMut, + other_modal_scrims: Query<(), (With, Without)>, ) { let button_clicked = requests.read().count() > 0; if keys.just_pressed(KeyCode::KeyO) || button_clicked { screen.0 = !screen.0; + } else if keys.just_pressed(KeyCode::Escape) && screen.0 && other_modal_scrims.is_empty() { + screen.0 = false; } } diff --git a/solitaire_engine/src/settings_plugin/tests.rs b/solitaire_engine/src/settings_plugin/tests.rs index e6b7173..9916dfd 100644 --- a/solitaire_engine/src/settings_plugin/tests.rs +++ b/solitaire_engine/src/settings_plugin/tests.rs @@ -140,6 +140,31 @@ fn pressing_o_toggles_settings_screen_flag() { ); } +/// Esc closes the Settings panel like O / Done do (Phase C dismissal +/// audit). Esc while the panel is closed must NOT open it. +#[test] +fn escape_closes_settings_screen_flag() { + let mut app = headless_app(); + + press(&mut app, KeyCode::Escape); + app.update(); + assert!( + !app.world().resource::().0, + "Esc on a closed panel stays closed" + ); + + press(&mut app, KeyCode::KeyO); + app.update(); + assert!(app.world().resource::().0, "O opens"); + + press(&mut app, KeyCode::Escape); + app.update(); + assert!( + !app.world().resource::().0, + "Esc closes settings" + ); +} + // cycle_unlocked pure-function tests #[test] fn cycle_unlocked_wraps_at_end() { diff --git a/solitaire_engine/src/theme_store_plugin.rs b/solitaire_engine/src/theme_store_plugin.rs index 4b4f325..49645c5 100644 --- a/solitaire_engine/src/theme_store_plugin.rs +++ b/solitaire_engine/src/theme_store_plugin.rs @@ -118,6 +118,9 @@ impl Plugin for ThemeStorePlugin { .init_resource::() .init_resource::() .init_resource::() + // Esc-close reads keyboard input; register defensively so + // the plugin works under MinimalPlugins in tests. + .init_resource::>() .add_message::() .add_message::() .add_message::() @@ -344,19 +347,23 @@ fn poll_install_task( ); } -/// Despawns the store modal when Close is pressed. +/// Despawns the store modal when Close is pressed or on Esc (Phase C +/// dismissal audit). The store only ever stacks over Settings and +/// nothing stacks over the store, so it owns Esc whenever it is open +/// (Settings' own Esc handler is gated on being topmost). fn handle_close_button( interactions: Query<&Interaction, (Changed, With)>, + keys: Res>, screens: Query>, mut commands: Commands, ) { - for interaction in &interactions { - if *interaction != Interaction::Pressed { - continue; - } - for entity in &screens { - commands.entity(entity).despawn(); - } + let clicked = interactions.iter().any(|i| *i == Interaction::Pressed); + let esc = keys.just_pressed(KeyCode::Escape) && !screens.is_empty(); + if !clicked && !esc { + return; + } + for entity in &screens { + commands.entity(entity).despawn(); } }