fix(ui): add modal guard to profile, make modal dimensions responsive (#57, #67)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
funman300
2026-05-27 19:21:09 -07:00
parent cfdf27c8c7
commit 35fde160fa
2 changed files with 142 additions and 58 deletions
+55 -15
View File
@@ -4,11 +4,11 @@
//! summary in a single scrollable panel. Spawned on the first `P` keypress and //! summary in a single scrollable panel. Spawned on the first `P` keypress and
//! despawned on the second. //! despawned on the second.
use bevy::input::mouse::{MouseScrollUnit, MouseWheel};
use bevy::input::ButtonInput; use bevy::input::ButtonInput;
use bevy::input::mouse::{MouseScrollUnit, MouseWheel};
use bevy::prelude::*; use bevy::prelude::*;
use chrono::{Duration, Local, NaiveDate}; use chrono::{Duration, Local, NaiveDate};
use solitaire_core::achievement::{achievement_by_id, ALL_ACHIEVEMENTS}; use solitaire_core::achievement::{ALL_ACHIEVEMENTS, achievement_by_id};
use solitaire_data::SyncBackend; use solitaire_data::SyncBackend;
use crate::achievement_plugin::AchievementsResource; use crate::achievement_plugin::AchievementsResource;
@@ -18,10 +18,10 @@ use crate::font_plugin::FontResource;
use crate::progress_plugin::ProgressResource; use crate::progress_plugin::ProgressResource;
use crate::resources::{SyncStatus, SyncStatusResource}; use crate::resources::{SyncStatus, SyncStatusResource};
use crate::settings_plugin::SettingsResource; use crate::settings_plugin::SettingsResource;
use crate::stats_plugin::{format_fastest_win, format_win_rate, StatsResource}; use crate::stats_plugin::{StatsResource, format_fastest_win, format_win_rate};
use crate::ui_modal::{ use crate::ui_modal::{
spawn_modal, spawn_modal_actions, spawn_modal_button, spawn_modal_header, ButtonVariant, ButtonVariant, ModalScrim, ScrimDismissible, spawn_modal, spawn_modal_actions,
ScrimDismissible, spawn_modal_button, spawn_modal_header,
}; };
use crate::ui_theme::{ use crate::ui_theme::{
ACCENT_PRIMARY, BG_ELEVATED, BORDER_STRONG, SPACE_1, STATE_INFO, STATE_SUCCESS, TEXT_PRIMARY, ACCENT_PRIMARY, BG_ELEVATED, BORDER_STRONG, SPACE_1, STATE_INFO, STATE_SUCCESS, TEXT_PRIMARY,
@@ -31,8 +31,8 @@ use crate::ui_theme::{
/// Number of days surfaced in the daily-challenge calendar row. /// Number of days surfaced in the daily-challenge calendar row.
/// ///
/// 14 = trailing two weeks ending today. At ~12 px per dot with a 6 px gap /// 14 = trailing two weeks ending today. At ~12 px per dot with a 6 px gap
/// the row is ~246 px wide — well inside the 360 px minimum modal width on /// the row is ~246 px wide — comfortably inside the responsive modal card
/// the smallest supported window (800 px). /// even on narrow phone layouts.
const CALENDAR_DAYS: usize = 14; const CALENDAR_DAYS: usize = 14;
/// Diameter of each calendar dot, in pixels. /// Diameter of each calendar dot, in pixels.
@@ -146,6 +146,7 @@ fn toggle_profile_screen(
font_res: Option<Res<FontResource>>, font_res: Option<Res<FontResource>>,
avatar: Option<Res<AvatarResource>>, avatar: Option<Res<AvatarResource>>,
screens: Query<Entity, With<ProfileScreen>>, screens: Query<Entity, With<ProfileScreen>>,
scrims: Query<(), With<ModalScrim>>,
) { ) {
let button_clicked = requests.read().count() > 0; let button_clicked = requests.read().count() > 0;
let p_pressed = keys.just_pressed(KeyCode::KeyP); let p_pressed = keys.just_pressed(KeyCode::KeyP);
@@ -161,6 +162,9 @@ fn toggle_profile_screen(
if !want_open && !want_close { if !want_open && !want_close {
return; return;
} }
if want_open && !scrims.is_empty() {
return;
}
if let Ok(entity) = screens.single() { if let Ok(entity) = screens.single() {
commands.entity(entity).despawn(); commands.entity(entity).despawn();
} else { } else {
@@ -257,7 +261,10 @@ fn spawn_profile_screen(
flex_direction: FlexDirection::Row, flex_direction: FlexDirection::Row,
align_items: AlignItems::Center, align_items: AlignItems::Center,
column_gap: Val::Px(10.0), column_gap: Val::Px(10.0),
margin: UiRect { bottom: Val::Px(4.0), ..default() }, margin: UiRect {
bottom: Val::Px(4.0),
..default()
},
..default() ..default()
}) })
.with_children(|row| { .with_children(|row| {
@@ -275,7 +282,13 @@ fn spawn_profile_screen(
)); ));
} else { } else {
// Initials fallback: coloured disc with the first letter. // Initials fallback: coloured disc with the first letter.
let initial = username.chars().next().unwrap_or('?').to_uppercase().next().unwrap_or('?'); let initial = username
.chars()
.next()
.unwrap_or('?')
.to_uppercase()
.next()
.unwrap_or('?');
row.spawn(( row.spawn((
Node { Node {
width: Val::Px(SIZE), width: Val::Px(SIZE),
@@ -335,7 +348,10 @@ fn spawn_profile_screen(
let pct = if xp_span == 0 { let pct = if xp_span == 0 {
100u64 100u64
} else { } else {
xp_done.saturating_mul(100).checked_div(xp_span).unwrap_or(100) xp_done
.saturating_mul(100)
.checked_div(xp_span)
.unwrap_or(100)
}; };
body.spawn(( body.spawn((
Text::new(format!( Text::new(format!(
@@ -378,7 +394,10 @@ fn spawn_profile_screen(
let records = &ar.0; let records = &ar.0;
let unlocked_count = records.iter().filter(|r| r.unlocked).count(); let unlocked_count = records.iter().filter(|r| r.unlocked).count();
body.spawn(( body.spawn((
Text::new(format!("{unlocked_count} / {} unlocked", ALL_ACHIEVEMENTS.len())), Text::new(format!(
"{unlocked_count} / {} unlocked",
ALL_ACHIEVEMENTS.len()
)),
font_row.clone(), font_row.clone(),
TextColor(ACCENT_PRIMARY), TextColor(ACCENT_PRIMARY),
)); ));
@@ -533,7 +552,11 @@ fn spawn_daily_calendar(
// accent border) regardless of completion; past days use a // accent border) regardless of completion; past days use a
// subtle border so the row reads as a row of pills, not a // subtle border so the row reads as a row of pills, not a
// strip of bare squares. // strip of bare squares.
let border_color = if is_today { ACCENT_PRIMARY } else { BORDER_STRONG }; let border_color = if is_today {
ACCENT_PRIMARY
} else {
BORDER_STRONG
};
let border_width = if is_today { 2.0 } else { 0.0 }; let border_width = if is_today { 2.0 } else { 0.0 };
row.spawn(( row.spawn((
DailyCalendarDot { DailyCalendarDot {
@@ -569,9 +592,7 @@ fn calendar_dot_color(completed: bool) -> Color {
fn sync_info(backend: &SyncBackend) -> (&'static str, String) { fn sync_info(backend: &SyncBackend) -> (&'static str, String) {
match backend { match backend {
SyncBackend::Local => ("Local", "".to_string()), SyncBackend::Local => ("Local", "".to_string()),
SyncBackend::SolitaireServer { username, .. } => { SyncBackend::SolitaireServer { username, .. } => ("Solitaire Server", username.clone()),
("Solitaire Server", username.clone())
}
} }
} }
@@ -641,6 +662,25 @@ mod tests {
); );
} }
#[test]
fn pressing_p_does_not_stack_profile_over_existing_modal_scrim() {
let mut app = headless_app();
app.world_mut().spawn(ModalScrim);
app.world_mut()
.resource_mut::<ButtonInput<KeyCode>>()
.press(KeyCode::KeyP);
app.update();
assert_eq!(
app.world_mut()
.query::<&ProfileScreen>()
.iter(app.world())
.count(),
0,
"Profile should not open when another modal scrim already exists"
);
}
#[test] #[test]
fn profile_modal_body_is_scrollable() { fn profile_modal_body_is_scrollable() {
let mut app = headless_app(); let mut app = headless_app();
+87 -43
View File
@@ -58,11 +58,11 @@ use crate::font_plugin::FontResource;
use crate::platform::SHOW_KEYBOARD_ACCELERATORS; use crate::platform::SHOW_KEYBOARD_ACCELERATORS;
use crate::settings_plugin::SettingsResource; use crate::settings_plugin::SettingsResource;
use crate::ui_theme::{ use crate::ui_theme::{
scaled_duration, ACCENT_PRIMARY, ACCENT_PRIMARY_HOVER, ACCENT_SECONDARY, BG_BASE, BG_ELEVATED, ACCENT_PRIMARY, ACCENT_PRIMARY_HOVER, ACCENT_SECONDARY, BG_BASE, BG_ELEVATED, BG_ELEVATED_HI,
BG_ELEVATED_HI, BG_ELEVATED_PRESSED, BG_ELEVATED_TOP, BORDER_STRONG, BORDER_SUBTLE, BG_ELEVATED_PRESSED, BG_ELEVATED_TOP, BORDER_STRONG, BORDER_SUBTLE, HighContrastBorder,
HighContrastBorder, MOTION_MODAL_SECS, RADIUS_LG, RADIUS_MD, SCRIM, TEXT_PRIMARY, MOTION_MODAL_SECS, RADIUS_LG, RADIUS_MD, SCRIM, TEXT_PRIMARY, TEXT_SECONDARY, TYPE_BODY_LG,
TEXT_SECONDARY, TYPE_BODY_LG, TYPE_CAPTION, TYPE_HEADLINE, VAL_SPACE_2, VAL_SPACE_3, TYPE_CAPTION, TYPE_HEADLINE, VAL_SPACE_2, VAL_SPACE_3, VAL_SPACE_4, VAL_SPACE_5,
VAL_SPACE_4, VAL_SPACE_5, scaled_duration,
}; };
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
@@ -193,7 +193,10 @@ where
.spawn(( .spawn((
plugin_marker, plugin_marker,
ModalScrim, ModalScrim,
ModalEntering { elapsed: 0.0, duration }, ModalEntering {
elapsed: 0.0,
duration,
},
Node { Node {
position_type: PositionType::Absolute, position_type: PositionType::Absolute,
left: Val::Px(0.0), left: Val::Px(0.0),
@@ -227,11 +230,11 @@ where
Node { Node {
flex_direction: FlexDirection::Column, flex_direction: FlexDirection::Column,
row_gap: VAL_SPACE_4, row_gap: VAL_SPACE_4,
width: Val::Percent(90.0),
padding: UiRect::all(VAL_SPACE_5), padding: UiRect::all(VAL_SPACE_5),
border: UiRect::all(Val::Px(1.0)), border: UiRect::all(Val::Px(1.0)),
border_radius: BorderRadius::all(Val::Px(RADIUS_LG)), border_radius: BorderRadius::all(Val::Px(RADIUS_LG)),
max_width: Val::Px(720.0), max_width: Val::Px(720.0),
min_width: Val::Px(360.0),
align_items: AlignItems::Stretch, align_items: AlignItems::Stretch,
..default() ..default()
}, },
@@ -295,12 +298,7 @@ pub fn spawn_modal_body_text(
font_size: TYPE_BODY_LG, font_size: TYPE_BODY_LG,
..default() ..default()
}; };
parent.spawn(( parent.spawn((ModalBody, Text::new(text.into()), font, TextColor(color)));
ModalBody,
Text::new(text.into()),
font,
TextColor(color),
));
} }
/// Spawns the bottom actions row — flex-row with primary right-aligned. /// Spawns the bottom actions row — flex-row with primary right-aligned.
@@ -343,7 +341,11 @@ pub fn spawn_modal_button<M: Component>(
variant: ButtonVariant, variant: ButtonVariant,
font_res: Option<&FontResource>, font_res: Option<&FontResource>,
) { ) {
let hotkey = if SHOW_KEYBOARD_ACCELERATORS { hotkey } else { None }; let hotkey = if SHOW_KEYBOARD_ACCELERATORS {
hotkey
} else {
None
};
let font_handle = font_res.map(|f| f.0.clone()).unwrap_or_default(); let font_handle = font_res.map(|f| f.0.clone()).unwrap_or_default();
let font_label = TextFont { let font_label = TextFont {
font: font_handle.clone(), font: font_handle.clone(),
@@ -517,7 +519,10 @@ pub fn apply_modal_enter_speed(
pub fn advance_modal_enter( pub fn advance_modal_enter(
time: Res<Time>, time: Res<Time>,
mut commands: Commands, mut commands: Commands,
mut scrims: Query<(Entity, &mut ModalEntering, &mut BackgroundColor, &Children), With<ModalScrim>>, mut scrims: Query<
(Entity, &mut ModalEntering, &mut BackgroundColor, &Children),
With<ModalScrim>,
>,
mut cards: Query<&mut Transform, With<ModalCard>>, mut cards: Query<&mut Transform, With<ModalCard>>,
) { ) {
let dt = time.delta_secs(); let dt = time.delta_secs();
@@ -653,10 +658,7 @@ pub fn dismiss_modal_on_scrim_click(
/// paint system. /// paint system.
#[allow(clippy::type_complexity)] #[allow(clippy::type_complexity)]
pub fn paint_modal_buttons( pub fn paint_modal_buttons(
mut buttons: Query< mut buttons: Query<(&Interaction, &ModalButton, &mut BackgroundColor), Changed<Interaction>>,
(&Interaction, &ModalButton, &mut BackgroundColor),
Changed<Interaction>,
>,
) { ) {
for (interaction, modal_button, mut bg) in &mut buttons { for (interaction, modal_button, mut bg) in &mut buttons {
bg.0 = match interaction { bg.0 = match interaction {
@@ -687,7 +689,12 @@ impl Plugin for UiModalPlugin {
// before advance computes `t`. // before advance computes `t`.
app.add_systems( app.add_systems(
Update, Update,
(apply_modal_enter_speed, advance_modal_enter, paint_modal_buttons).chain(), (
apply_modal_enter_speed,
advance_modal_enter,
paint_modal_buttons,
)
.chain(),
); );
// Click-outside-to-dismiss is independent of the open // Click-outside-to-dismiss is independent of the open
// animation chain — it reads `just_pressed(Left)` and runs // animation chain — it reads `just_pressed(Left)` and runs
@@ -774,6 +781,11 @@ mod tests {
(card_scale - MODAL_ENTER_START_SCALE).abs() < 1e-6, (card_scale - MODAL_ENTER_START_SCALE).abs() < 1e-6,
"card should spawn at MODAL_ENTER_START_SCALE; got {card_scale}" "card should spawn at MODAL_ENTER_START_SCALE; got {card_scale}"
); );
let card_node = card_node_of(&app, scrim);
assert_eq!(card_node.width, Val::Percent(90.0));
assert_eq!(card_node.max_width, Val::Px(720.0));
assert_eq!(card_node.min_width, Val::Auto);
} }
/// After enough simulated ticks for `elapsed >= duration`, the /// After enough simulated ticks for `elapsed >= duration`, the
@@ -817,23 +829,42 @@ mod tests {
); );
} }
/// Returns the `Entity` of the first `ModalCard` child of the given
/// scrim.
fn card_entity_of(world: &World, scrim: Entity) -> Entity {
let children = world
.entity(scrim)
.get::<Children>()
.expect("scrim should have a card child");
children
.iter()
.find(|child| world.entity(*child).get::<ModalCard>().is_some())
.expect("scrim should have a ModalCard child")
}
/// Returns the `Node` of the first `ModalCard` child of the given
/// scrim.
fn card_node_of(app: &App, scrim: Entity) -> &Node {
let world = app.world();
let card = card_entity_of(world, scrim);
world
.entity(card)
.get::<Node>()
.expect("ModalCard child should have a Node")
}
/// Returns the X-component of the first `ModalCard` child of the /// Returns the X-component of the first `ModalCard` child of the
/// given scrim's `Transform::scale`. All three components are kept /// given scrim's `Transform::scale`. All three components are kept
/// in sync by the system so reading X is sufficient. /// in sync by the system so reading X is sufficient.
fn card_scale_of(app: &mut App, scrim: Entity) -> f32 { fn card_scale_of(app: &mut App, scrim: Entity) -> f32 {
let world = app.world(); let world = app.world();
let children = world let card = card_entity_of(world, scrim);
.entity(scrim) world
.get::<Children>() .entity(card)
.expect("scrim should have a card child"); .get::<Transform>()
for child in children.iter() { .expect("ModalCard child should have a Transform")
if let Some(t) = world.entity(child).get::<Transform>() .scale
&& world.entity(child).get::<ModalCard>().is_some() .x
{
return t.scale.x;
}
}
panic!("no ModalCard child with a Transform under scrim {scrim:?}");
} }
/// Tells `TimePlugin` to advance the clock by `secs` on the next /// Tells `TimePlugin` to advance the clock by `secs` on the next
@@ -844,9 +875,9 @@ mod tests {
fn set_manual_time_step(app: &mut App, secs: f32) { fn set_manual_time_step(app: &mut App, secs: f32) {
use bevy::time::TimeUpdateStrategy; use bevy::time::TimeUpdateStrategy;
use std::time::Duration; use std::time::Duration;
app.insert_resource(TimeUpdateStrategy::ManualDuration( app.insert_resource(TimeUpdateStrategy::ManualDuration(Duration::from_secs_f32(
Duration::from_secs_f32(secs), secs,
)); )));
} }
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
@@ -871,10 +902,26 @@ mod tests {
fn cursor_is_inside_rect_outside_returns_false() { fn cursor_is_inside_rect_outside_returns_false() {
let centre = Vec2::new(200.0, 150.0); let centre = Vec2::new(200.0, 150.0);
let size = Vec2::new(100.0, 60.0); let size = Vec2::new(100.0, 60.0);
assert!(!cursor_is_inside_rect(Vec2::new(149.0, 150.0), centre, size)); // left assert!(!cursor_is_inside_rect(
assert!(!cursor_is_inside_rect(Vec2::new(251.0, 150.0), centre, size)); // right Vec2::new(149.0, 150.0),
assert!(!cursor_is_inside_rect(Vec2::new(200.0, 119.0), centre, size)); // above centre,
assert!(!cursor_is_inside_rect(Vec2::new(200.0, 181.0), centre, size)); // below size
)); // left
assert!(!cursor_is_inside_rect(
Vec2::new(251.0, 150.0),
centre,
size
)); // right
assert!(!cursor_is_inside_rect(
Vec2::new(200.0, 119.0),
centre,
size
)); // above
assert!(!cursor_is_inside_rect(
Vec2::new(200.0, 181.0),
centre,
size
)); // below
} }
/// Builds a headless app capable of running /// Builds a headless app capable of running
@@ -966,9 +1013,7 @@ mod tests {
/// `MinimalPlugins` doesn't run the input clear pass, so we mark /// `MinimalPlugins` doesn't run the input clear pass, so we mark
/// the clear by hand on the resource between presses. /// the clear by hand on the resource between presses.
fn press_left_mouse(app: &mut App) { fn press_left_mouse(app: &mut App) {
let mut input = app let mut input = app.world_mut().resource_mut::<ButtonInput<MouseButton>>();
.world_mut()
.resource_mut::<ButtonInput<MouseButton>>();
input.clear(); input.clear();
input.press(MouseButton::Left); input.press(MouseButton::Left);
} }
@@ -1068,4 +1113,3 @@ mod tests {
); );
} }
} }