feat(replay): highlight active row in Move Log panel
Wraps the active-row Text in a Node with BackgroundColor(ACCENT_PRIMARY) so the row reads as "current focus" against the panel's elevated background. Inner Text colour bumps from TEXT_PRIMARY (#d0d0d0) to TEXT_PRIMARY_HC (#f5f5f5) for legible contrast against the brick-red highlight. format_active_move_row now prefixes the row with `▶` (the focus marker) so the visual hierarchy is reinforced even before the background paints (HC mode, future palette tweaks). The empty case still returns empty — cursor=0 doesn't paint a stray "▶ " prefix on an otherwise-empty row. Mirrors the mockup at docs/ui-mockups/replay-overlay-mobile.html § "Move Log Card" where the active row has bg-suit-red-cb (brick-red equivalent) + dark text + the ▶ marker. 3 new tests: - active_row_wrapper_carries_accent_primary_background: walks from the active-row Text to its parent Node and asserts the wrapper carries BackgroundColor(ACCENT_PRIMARY). - active_row_text_uses_high_contrast_color_for_highlight: pins the TextColor as TEXT_PRIMARY_HC. - active_row_format_includes_focus_prefix: pure-helper guard for the ▶ prefix + the cursor=0-stays-empty contract. Plus 2 existing tests updated for the new prefixed format (format_active_move_row_handles_cursor_zero_and_positive, move_log_active_row_repaints_on_cursor_advance). Tests: 1266 → 1269 (+3 net new, +2 updated). Clippy clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -38,8 +38,8 @@ use solitaire_data::ReplayMove;
|
||||
use crate::ui_modal::{spawn_modal_button, ButtonVariant};
|
||||
use crate::ui_theme::{
|
||||
ACCENT_PRIMARY, BG_ELEVATED_HI, BORDER_SUBTLE, HighContrastBackground, HighContrastBorder,
|
||||
STATE_SUCCESS, TEXT_PRIMARY, TEXT_SECONDARY, TYPE_BODY, TYPE_CAPTION, TYPE_HEADLINE,
|
||||
VAL_SPACE_1, VAL_SPACE_2, VAL_SPACE_4, Z_DROP_OVERLAY,
|
||||
STATE_SUCCESS, TEXT_PRIMARY, TEXT_PRIMARY_HC, TEXT_SECONDARY, TYPE_BODY, TYPE_CAPTION,
|
||||
TYPE_HEADLINE, VAL_SPACE_1, VAL_SPACE_2, VAL_SPACE_4, Z_DROP_OVERLAY,
|
||||
};
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -941,12 +941,27 @@ fn spawn_overlay(
|
||||
TextColor(TEXT_SECONDARY),
|
||||
));
|
||||
}
|
||||
// Active move row. Empty at spawn time when cursor=0;
|
||||
// Active move row. Wrapped in a Node with an
|
||||
// ACCENT_PRIMARY background so the row reads as
|
||||
// "current focus" — the player can scan vertically
|
||||
// and the highlighted row is the move that just
|
||||
// applied. Empty text at spawn time when cursor=0;
|
||||
// the per-frame update system populates it as the
|
||||
// cursor advances. TEXT_PRIMARY (vs prev rows'
|
||||
// TEXT_SECONDARY) gives the active row more visual
|
||||
// weight — it's the load-bearing information.
|
||||
panel.spawn((
|
||||
// cursor advances. Text colour is TEXT_PRIMARY_HC
|
||||
// (near-white) for contrast against the brick-red
|
||||
// background — same trick as the modal-button
|
||||
// primary-variant paint.
|
||||
panel
|
||||
.spawn((
|
||||
Node {
|
||||
width: Val::Percent(100.0),
|
||||
padding: UiRect::axes(VAL_SPACE_2, VAL_SPACE_1),
|
||||
..default()
|
||||
},
|
||||
BackgroundColor(ACCENT_PRIMARY),
|
||||
))
|
||||
.with_children(|active| {
|
||||
active.spawn((
|
||||
ReplayOverlayMoveLogActiveRow,
|
||||
Text::new(format_active_move_row(state)),
|
||||
TextFont {
|
||||
@@ -954,9 +969,10 @@ fn spawn_overlay(
|
||||
font_size: TYPE_BODY,
|
||||
..default()
|
||||
},
|
||||
TextColor(TEXT_PRIMARY),
|
||||
TextColor(TEXT_PRIMARY_HC),
|
||||
));
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
/// Pure helper — returns the scrub-fill width as a percentage of the
|
||||
@@ -1339,11 +1355,18 @@ fn format_kth_recent_row(state: &ReplayPlaybackState, k: usize) -> String {
|
||||
}
|
||||
|
||||
/// Pure helper — formats the active-row text for the move-log
|
||||
/// panel. Thin wrapper around [`format_kth_recent_row`] with `k=1`.
|
||||
/// The active row IS the kth-most-recent for k=1, so this exists
|
||||
/// to keep call sites readable.
|
||||
/// panel. Wraps [`format_kth_recent_row`] with `k=1` and prepends
|
||||
/// a `▶` focus marker so the active row reads visually distinct
|
||||
/// from prev rows even before the highlight background lands.
|
||||
/// Returns empty when there's no row to render (cursor=0 or
|
||||
/// non-`Playing` state) — never `"▶ "` alone, which would paint
|
||||
/// a stray prefix.
|
||||
fn format_active_move_row(state: &ReplayPlaybackState) -> String {
|
||||
format_kth_recent_row(state, 1)
|
||||
let body = format_kth_recent_row(state, 1);
|
||||
if body.is_empty() {
|
||||
return String::new();
|
||||
}
|
||||
format!("\u{25B6} {body}") // ▶
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -2697,10 +2720,12 @@ mod tests {
|
||||
// synthetic_replay produces all StockClicks, so the body
|
||||
// is "stock cycle". The displayed index is 3 (cursor),
|
||||
// matching the most-recently-applied move at moves[2].
|
||||
// Active row carries the `▶` focus prefix; prev rows
|
||||
// (kth-recent for k>1) don't.
|
||||
assert_eq!(
|
||||
format_active_move_row(&cursor_three),
|
||||
"3 \u{2502} stock cycle",
|
||||
"row body must read `cursor │ {{move body}}` with the 1-based displayed index",
|
||||
"\u{25B6} 3 \u{2502} stock cycle",
|
||||
"active row must read `▶ cursor │ {{move body}}` with the 1-based displayed index",
|
||||
);
|
||||
}
|
||||
|
||||
@@ -2788,8 +2813,8 @@ mod tests {
|
||||
app.update();
|
||||
assert_eq!(
|
||||
move_log_active_row_text(&mut app),
|
||||
"2 \u{2502} stock cycle",
|
||||
"active row must repaint to the cursor's position when state changes",
|
||||
"\u{25B6} 2 \u{2502} stock cycle",
|
||||
"active row must repaint to the cursor's position when state changes (with ▶ prefix)",
|
||||
);
|
||||
}
|
||||
|
||||
@@ -2948,6 +2973,110 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// Active row sits inside a wrapper Node with
|
||||
/// `BackgroundColor(ACCENT_PRIMARY)` so it reads as "current
|
||||
/// focus" against the panel background. Validates the wrapper
|
||||
/// is present and carries the expected colour.
|
||||
#[test]
|
||||
fn active_row_wrapper_carries_accent_primary_background() {
|
||||
let mut app = headless_app();
|
||||
set_state(
|
||||
&mut app,
|
||||
ReplayPlaybackState::Playing {
|
||||
replay: synthetic_replay(10),
|
||||
cursor: 3,
|
||||
secs_to_next: 0.5,
|
||||
paused: false,
|
||||
},
|
||||
);
|
||||
app.update();
|
||||
|
||||
// Find the active-row Text entity, then walk to its
|
||||
// parent — that's the wrapper Node which should carry
|
||||
// the highlight BackgroundColor.
|
||||
let world = app.world_mut();
|
||||
let mut row_q = world.query_filtered::<Entity, With<ReplayOverlayMoveLogActiveRow>>();
|
||||
let row = row_q
|
||||
.iter(world)
|
||||
.next()
|
||||
.expect("active row Text entity must exist while overlay is spawned");
|
||||
let mut parent_q = world.query::<&ChildOf>();
|
||||
let parent = parent_q
|
||||
.get(world, row)
|
||||
.map(|p| p.parent())
|
||||
.expect("active row must have a parent (the highlight wrapper)");
|
||||
let mut bg_q = world.query::<&BackgroundColor>();
|
||||
let bg = bg_q
|
||||
.get(world, parent)
|
||||
.expect("active row's parent must carry BackgroundColor (highlight)");
|
||||
assert_eq!(
|
||||
bg.0, ACCENT_PRIMARY,
|
||||
"active-row wrapper background must be ACCENT_PRIMARY for the focus highlight",
|
||||
);
|
||||
}
|
||||
|
||||
/// Active-row Text uses TEXT_PRIMARY_HC for legible contrast
|
||||
/// against the brick-red ACCENT_PRIMARY background. Without
|
||||
/// this the default TEXT_PRIMARY (#d0d0d0) on red would have
|
||||
/// borderline contrast; the HC variant (#f5f5f5) keeps the
|
||||
/// row readable.
|
||||
#[test]
|
||||
fn active_row_text_uses_high_contrast_color_for_highlight() {
|
||||
let mut app = headless_app();
|
||||
set_state(
|
||||
&mut app,
|
||||
ReplayPlaybackState::Playing {
|
||||
replay: synthetic_replay(10),
|
||||
cursor: 3,
|
||||
secs_to_next: 0.5,
|
||||
paused: false,
|
||||
},
|
||||
);
|
||||
app.update();
|
||||
|
||||
let world = app.world_mut();
|
||||
let mut q = world
|
||||
.query_filtered::<&TextColor, With<ReplayOverlayMoveLogActiveRow>>();
|
||||
let color = q
|
||||
.iter(world)
|
||||
.next()
|
||||
.expect("active row TextColor must exist");
|
||||
assert_eq!(
|
||||
color.0, TEXT_PRIMARY_HC,
|
||||
"active row text colour must be TEXT_PRIMARY_HC for contrast against the highlight",
|
||||
);
|
||||
}
|
||||
|
||||
/// Active-row text starts with the `▶` focus marker prefix.
|
||||
/// Pure-helper guard — pins the prefix so a future refactor
|
||||
/// dropping it has to also update this test.
|
||||
#[test]
|
||||
fn active_row_format_includes_focus_prefix() {
|
||||
let state = ReplayPlaybackState::Playing {
|
||||
replay: synthetic_replay(10),
|
||||
cursor: 5,
|
||||
secs_to_next: 0.5,
|
||||
paused: false,
|
||||
};
|
||||
let row = format_active_move_row(&state);
|
||||
assert!(
|
||||
row.starts_with('\u{25B6}'),
|
||||
"active-row format must start with ▶ focus marker; got {row:?}",
|
||||
);
|
||||
// Cursor=0 still returns empty, never just the prefix.
|
||||
let cursor_zero = ReplayPlaybackState::Playing {
|
||||
replay: synthetic_replay(10),
|
||||
cursor: 0,
|
||||
secs_to_next: 0.5,
|
||||
paused: false,
|
||||
};
|
||||
assert_eq!(
|
||||
format_active_move_row(&cursor_zero),
|
||||
"",
|
||||
"cursor=0 must return empty (no stray prefix on empty row)",
|
||||
);
|
||||
}
|
||||
|
||||
/// Panel shares the overlay tree's lifecycle — it despawns on
|
||||
/// `Playing → Inactive` along with the banner root.
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user