UX remediation: pause timers, valid defaults, error summaries, import gating, career stats entry, mobile More disclosure#244
Closed
Copilot wants to merge 14 commits into
Closed
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Copilot created this pull request from a session on behalf of
maniator
April 22, 2026 20:21
View session
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements a multi-phase UX remediation pass across gameplay controls, saves import, custom team editor validation, and career stats discoverability, with supporting unit/E2E updates and refreshed visual snapshots.
Changes:
- Introduces reference-counted UI pause coordination (modals pause autoplay + manager decision countdown) and a mobile “More” disclosure for secondary game controls.
- Improves save import UX by gating CTAs on parseable envelope shape and adding helper/status messaging.
- Makes custom team editor “valid by default” for newly added rows and adds a canonical error summary + inline error gating.
Reviewed changes
Copilot reviewed 47 out of 64 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/setup.ts | Adds a jsdom window.matchMedia mock for useMediaQuery-based components/tests. |
| src/storage/types.ts | Removes hasCareerStats from outlet context; career stats entry is always available. |
| src/shared/contexts/UIPauseContext.tsx | Adds reference-counted UI pause context + useUIPauseScope. |
| src/shared/contexts/UIPauseContext.test.tsx | Unit tests for UI pause reference counting and scope behavior. |
| src/router.tsx | Always passes onCareerStats into HomeScreen. |
| src/index.tsx | Wraps app in UIPauseProvider. |
| src/features/saves/pages/SavesPage/styles.ts | Adds invalid border styling + helper text styles for import UX. |
| src/features/saves/pages/SavesPage/index.tsx | Wires useImportSave parse gating, helper/status region, and post-import focus management. |
| src/features/saves/pages/SavesPage/SavesPage.test.tsx | Updates unit tests to reflect disabled-until-valid import behavior. |
| src/features/saves/hooks/useImportSave.ts | Adds debounced parse-state computation, helper microcopy, and canImport gating. |
| src/features/saves/components/SavesModal/useSavesModal.ts | Tracks modal open state, ties to UI pause, and exposes import parse state + helper fields. |
| src/features/saves/components/SavesModal/styles.ts | Adds disabled styling and helper text + invalid border styling. |
| src/features/saves/components/SavesModal/index.tsx | Uses canImport gating and helper/status UI for import; wires blur/paste events. |
| src/features/saves/components/SavesModal/SavesModalPause.test.tsx | Verifies UI pause is pushed/popped on modal open/close and unmount. |
| src/features/saves/components/SavesModal/SavesModal.test.tsx | Updates import tests to respect new gating and debounce. |
| src/features/saves/components/SaveSlotList/styles.ts | Adds disabled styling for action buttons. |
| src/features/gameplay/pages/GamePage/index.test.tsx | Updates outlet context mock to remove hasCareerStats. |
| src/features/gameplay/components/HomeScreen/index.tsx | Makes “Career stats” always visible and requires onCareerStats. |
| src/features/gameplay/components/HomeScreen/HomeScreen.test.tsx | Updates tests for always-visible “Career stats” entry and required prop. |
| src/features/gameplay/components/GameControls/useGameControls.ts | Combines manual pause with UI pause for autoplay scheduler (`paused |
| src/features/gameplay/components/GameControls/styles.ts | Adds styled-components for mobile “More” disclosure trigger + panel. |
| src/features/gameplay/components/GameControls/index.tsx | Moves secondary controls into mobile “More” panel; keeps desktop/tablet inline. |
| src/features/gameplay/components/GameControls/MoreMenu.tsx | New mobile disclosure component with focus management + inert/aria-hidden behavior. |
| src/features/gameplay/components/GameControls/MoreMenu.test.tsx | Unit tests for “More” disclosure open/close, inert, escape, click-outside, focus, disabled. |
| src/features/gameplay/components/DecisionPanel/styles.ts | Adds paused visual treatment, pause pill, resume announcement, and paused countdown styling. |
| src/features/gameplay/components/DecisionPanel/index.tsx | Freezes countdown while UI-paused; adds pause/resume announcements; adds progressbar ARIA. |
| src/features/gameplay/components/DecisionPanel/PinchHitterDecisionButtons.tsx | Adds paused styling + aria-disabled plumbing for pinch hitter decision UI. |
| src/features/gameplay/components/DecisionPanel/DecisionPanel.test.tsx | Adds unit coverage for UI pause behavior (freeze + disable + ARIA). |
| src/features/gameplay/components/DecisionPanel/DecisionButtons.tsx | Threads paused state through decision buttons using $paused + aria-disabled. |
| src/features/gameplay/components/AppShell/index.tsx | Removes DB probing/gating for career stats; career stats navigation always available. |
| src/features/gameplay/components/AppShell/AppShell.test.tsx | Updates tests to reflect always-visible career stats and removal of getDb probing. |
| src/features/customTeams/components/CustomTeamEditor/styles.ts | Adds error summary components, inline error styling, new-row highlight, and SR live regions. |
| src/features/customTeams/components/CustomTeamEditor/index.tsx | Adds canonical error summary + blur/submit gating; adds default row add pipeline + announcements. |
| src/features/customTeams/components/CustomTeamEditor/editorState.ts | Adds default-row factories, structured validation issue collection, and section anchor IDs. |
| src/features/customTeams/components/CustomTeamEditor/editorState.test.ts | Adds tests for default-row factories and valid-by-default behavior. |
| src/features/customTeams/components/CustomTeamEditor/TeamInfoSection.tsx | Wires inline errors + blur tracking into team info fields with proper ARIA. |
| src/features/customTeams/components/CustomTeamEditor/SortablePlayerRow.tsx | Adds newly-added focus/select behavior, one-shot highlight, and group labeling for new rows. |
| src/features/customTeams/components/CustomTeamEditor/RosterSections.tsx | Uses default factories for add buttons; plumbs newly-added row tracking into sections. |
| src/features/customTeams/components/CustomTeamEditor/CustomTeamEditor.test.tsx | Adds unit tests for valid-by-default adds and error summary behavior. |
| src/features/careerStats/pages/CareerStatsPage/styles.ts | Adds token-aligned accessible empty state styling. |
| src/features/careerStats/pages/CareerStatsPage/index.tsx | Adds “No games yet” region + focus-managed heading + CTA to start a new game. |
| src/features/careerStats/pages/CareerStatsPage/CareerStatsPage.test.tsx | Updates tests for no-completed-games empty state and CTA navigation. |
| src/features/careerStats/hooks/useCareerStatsData.ts | Adds historyLoaded + noCompletedGames derivation for empty state routing. |
| e2e/tests/visual/home-and-dialogs.visual.spec.ts | Adjusts visual test to open “More” on mobile before clicking How to Play. |
| e2e/tests/visual/game-ui.visual.spec.ts-snapshots/player-stats-panel-iphone-15-linux.png | Updates visual snapshot baseline for mobile UI changes. |
| e2e/tests/modals.spec.ts | Updates saves import E2E to assert disabled CTA + helper guidance on invalid JSON. |
| e2e/tests/career-stats.spec.ts | Updates E2E for always-visible Career Stats entry and “no games” empty state behavior. |
| docs/style-guide.md | Documents the mobile “More” disclosure and canonical form error summary token/ARIA rules. |
Comment on lines
106
to
109
|
|
||
| let playerIdCounter = 0; | ||
| export const makePlayerId = (): string => `ep_${Date.now()}_${++playerIdCounter}`; | ||
|
|
There was a problem hiding this comment.
makePlayerId() still generates IDs via Date.now() + an incrementing counter, even though this PR introduces generatePlayerId() and the codebase treats src/storage/generateId.ts as the single source of truth for creating new IDs. This leaves the editor producing a mix of p_… and ep_… IDs and reintroduces timestamp-based IDs. Prefer replacing makePlayerId() usages with generatePlayerId() and removing the legacy generator for consistency.
Comment on lines
+88
to
+89
| $isNewlyAdded={initialNewlyAddedRef.current} | ||
| {...(initialNewlyAddedRef.current ? { role: "group", "aria-label": groupAriaLabel } : {})} |
There was a problem hiding this comment.
initialNewlyAddedRef is used not just to one-shot the focus/select effect, but also to keep role="group" + the aria-label of "New …" on the row permanently. After the 1.5s highlight window ends, this row is no longer “new”, but assistive tech will still read it that way. Consider using initialNewlyAddedRef only for the focus/select side effect, and condition the role/aria-label (and $isNewlyAdded) on the current isNewlyAdded prop so it’s removed once the new-row state clears.
Suggested change
| $isNewlyAdded={initialNewlyAddedRef.current} | |
| {...(initialNewlyAddedRef.current ? { role: "group", "aria-label": groupAriaLabel } : {})} | |
| $isNewlyAdded={isNewlyAdded} | |
| {...(isNewlyAdded ? { role: "group", "aria-label": groupAriaLabel } : {})} |
Comment on lines
40
to
+48
| <ActionButton | ||
| $paused={paused} | ||
| {...aria} | ||
| onClick={() => onDispatch({ type: "steal_attempt", payload: { base, successPct } })} | ||
| > | ||
| Yes, steal! | ||
| </ActionButton> | ||
| <SkipButton onClick={onSkip}>Skip</SkipButton> | ||
| <SkipButton $paused={paused} {...aria} onClick={onSkip}> | ||
| Skip |
There was a problem hiding this comment.
paused currently only sets aria-disabled + CSS pointer-events: none, which blocks mouse/touch but does not prevent keyboard activation (Enter/Space will still trigger onClick on a focused <button>). This allows dispatching manager decisions while the UI is paused. Add an explicit guard in the click handlers (and/or key handlers) to no-op when paused is true, or switch to native disabled with explicit focus-restore behavior after the modal closes.
Comment on lines
113
to
135
| <select | ||
| value={selectedCandidateId} | ||
| onChange={(e) => setSelectedCandidateId(e.target.value)} | ||
| aria-label="Select pinch hitter" | ||
| data-testid="pinch-hitter-select" | ||
| > | ||
| {candidates.map((c) => ( | ||
| <option key={c.id} value={c.id}> | ||
| {candidateLabel(c)} | ||
| {c.matchupDeltaPct !== undefined | ||
| ? ` [${c.matchupDeltaPct >= 0 ? "+" : ""}${c.matchupDeltaPct}%]` | ||
| : ""} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| <ActionButton onClick={handleConfirm} disabled={!selectedCandidateId}> | ||
| <ActionButton | ||
| $paused={paused} | ||
| {...aria} | ||
| onClick={handleConfirm} | ||
| disabled={!selectedCandidateId} | ||
| > | ||
| Send up pinch hitter | ||
| </ActionButton> |
There was a problem hiding this comment.
paused is implemented via aria-disabled + styling, but the <select> and action buttons still run onChange/onClick when activated from the keyboard. In the paused state this can still perform substitutions/strategy dispatches (especially handleConfirm). Gate the handlers (and consider disabling the <select> as well) so no decision actions can be taken while UI-paused.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1bd44f87a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| [handleSummaryLinkClick], | ||
| ); | ||
|
|
||
| const submitDisabled = attemptedSubmit && summaryIssues.length > 0; |
There was a problem hiding this comment.
submitDisabled is derived from summaryIssues.length, and summaryIssues includes state.error entries from async failures (createTeam/updateTeam catch path). After a transient save failure, attemptedSubmit stays true and the Save button is disabled, so users cannot immediately retry the same save action without first editing some field just to clear state.error. This regresses recoverability for temporary persistence failures.
Useful? React with 👍 / 👎.
…s modal is open Phase 1A of the UX remediation backlog (per @ux-design-lead spec). When the SavesModal is open, the Manager Mode auto-skip countdown is frozen and the autoplay scheduler is suspended. Both resume cleanly on close. The mechanism is reference-counted via a new UIPauseProvider so overlapping/nested blocking modals (modal stack) all stay paused until the last one closes. Determinism: this is a pure UI pause. No reducer dispatch, no PRNG calls (`@shared/utils/rng`), and no extra/missing pitches occur as a result of pause/resume. Existing seed-anchored regression tests are unaffected (full suite passes — 2340/2340). Visible state on pause: - Countdown bar fill recolors to `theme.colors.textMuted`, wrapper opacity 0.6, transitions disabled so the fill visibly freezes. - New 'Paused while Saves is open' pill above the DecisionPanel header using bgWarnSurface / borderWarn / textWarnBold / radii.pill / letterSpacing.wide tokens (role=status, aria-live=polite). - Decision action buttons are marked `aria-disabled=\"true\"` (NOT the native `disabled` attribute) so keyboard focus survives modal close; the standard disabled style (opacity 0.4, cursor not-allowed, pointer-events: none) is applied via styled-component prop. - Countdown progressbar exposes `aria-describedby` pointing at the pill while paused. - Hidden 'Resumed' live region (role=status, aria-live=polite) announces the transition back to active. Tests added: - UIPauseContext: push/pop, ref-counting, default no-op context, scope cleanup. - DecisionPanel paused: pill rendering, aria-disabled buttons, aria-describedby wiring, countdown freeze (no decrement, no auto-skip), and resume-from-same-remaining-seconds. - SavesModal: pushes pause on open, pops on close, releases on unmount-while-open (no leak). Out of scope: visual snapshot regeneration is left to e2e-test-runner. Co-authored-by: maniator <539579+maniator@users.noreply.github.com>
…aint
Newly added Custom Team Editor rows now instantiate with sensible defaults
so the new row passes editor validation immediately — no over-cap warning,
no aria-invalid on the name input, no error summary entries.
Defaults
- name: 'New batter N' / 'New pitcher N' (sequential, scoped per role,
skips numbers already used)
- position: lineup → next unfilled defensive slot or DH;
bench → first uncovered required field position (deviation: spec said
'BN' but no BN value exists in BATTER_POSITION_OPTIONS — picking an
uncovered required slot keeps the row immediately valid and helps
satisfy REQUIRED_FIELD_POSITIONS coverage);
pitcher → SP if rotation has < 5 starters, else RP
- handedness: R
- core stats: midpoint 50/50/50 → exactly at HITTER_STAT_CAP (150) for
hitters and well below PITCHER_STAT_CAP (160) for pitchers
- IDs from generatePlayerId() (storage util, p_ prefix) instead of
the legacy ep_ counter
UX side effects
- Newly added row container gets role='group' aria-label='New
batter/pitcher, position {pos}'
- 1500ms one-shot bgPlayerSelected → bgSurface highlight, with
prefers-reduced-motion: reduce collapsing to the final state
- Polite live region announces 'New batter/pitcher added — {name}'
- requestAnimationFrame focus + select on the new row's name input
Tests
- 8 new editorState unit tests covering createDefaultBatter/Pitcher,
position picking, sequential name indexing, and end-to-end roster
validity after add
- 5 new CustomTeamEditor component tests covering default field values,
sequential adds, live region announcement, and role=group container
Co-authored-by: maniator <539579+maniator@users.noreply.github.com>
- Add collectValidationIssues() returning all issues (not just first) with stable fieldId anchors + short inline complement copy. - Replace duplicate ErrorMsg blocks with a single canonical ErrorSummary at top of form (role=alert, focus heading on submit, anchor links that focus target inputs without polluting history). - Add InlineFieldError beneath name/abbrev inputs, gated by blur pre-submit and by submit attempt thereafter. Inline copy is the short per-field complement of the summary, never duplicating it. - Disable Save with aria-disabled + tooltip only AFTER a failed submit attempt; re-enables when issues clear. - Add 'All errors resolved' polite live announcement on errors->0 transition. - Document §12.1.a Form error summary in docs/style-guide.md. Co-authored-by: maniator <539579+maniator@users.noreply.github.com>
Co-authored-by: maniator <539579+maniator@users.noreply.github.com>
Co-authored-by: maniator <539579+maniator@users.noreply.github.com>
Phase 2C of the UX remediation backlog.
Home screen
- Career stats button is now always rendered (no longer gated on
hasCareerStats / DB probe). Reuses the existing SecondaryBtn so it
matches the rest of the menu visually.
- AppShell: dropped hasCareerStats state, the home-route DB probe, and
the related outlet-context flag (now dead code). onCareerStats is
always provided.
Career Stats destination
- New accessible 'No games yet' empty state shown when zero completed
games exist anywhere (teamsWithHistory is empty after the async
history load settles).
- Region wrapped in role=region + aria-labelledby. Heading is an h2
with tabindex=-1 and is auto-focused on first paint so SR users
land on the empty-state context.
- Body text capped at max-width: 38ch and centered. Play Ball CTA
reuses the §4.4 PlayBallButton visual treatment, navigating to
/exhibition/new.
- Existing 'no completed games yet for this team' inline state is
preserved for the case where other teams have history but the
selected team does not.
Tokens
- All new styles use existing tokens from src/shared/theme.ts:
textBody, textSubdued, textMuted, accentPrimary, btnPrimaryText,
borderForm, fontSizes.{md,h3,sub,lg,bodyLg}, spacing.{md,xxl,s10,s40},
radii.{sm,md,pill}, sizes.btnLg.
Style guide
- §12.2 Empty state: replaced hard-coded #666 / 14px / 40px with
token names (textSubdued, fontSizes.md, spacing.s40) and added a
reference to the new CareerEmptyState* components.
Tests
- Updated HomeScreen, AppShell, and CareerStatsPage suites to reflect
always-visible Career stats and the new accessible empty state.
- Added focused tests for the empty-state region (role/labelledby,
heading focus, Play Ball navigation).
- All 2361 unit tests pass; lint, format, typecheck, and build clean.
Visual snapshot baselines for the Home screen (all 6 Playwright
projects) and the Career Stats page (when empty) will need
regeneration via the e2e-test-runner agent.
Co-authored-by: maniator <539579+maniator@users.noreply.github.com>
Phase 3 of UX remediation: on mobile (≤768px), collapse non-critical game header controls (sim speed, sound, manager mode, saves, help) behind a single 'More' disclosure that opens a bottom-anchored, non-modal panel. Game continues running underneath; pause/play stays visible. Desktop/tablet layout unchanged. - New MoreMenu.tsx: compact-secondary trigger + bottom-anchored region, Escape/click-outside close, focus moves to first control on open and back to trigger on close, aria-disabled while a manager decision is pending (auto-collapses if open). - GameControls/index.tsx: branches on useMediaQuery; secondary controls rendered inline on desktop OR inside MoreMenu panel on mobile (single mount, no duplication of stateful children like SavesModal). - styles.ts: add InlineSecondaryGroup, MoreMenuMobileSlot, MoreTriggerButton (bgFormAlpha15→bgFormAlpha40, borderPanel, textBody, fontSizes.label, letterSpacing.wide, sizes.inputSm, radii.sm), MoreTriggerChevron (180° rotate, prefers-reduced-motion), MoreMenuPanel (bgSurface, top borderPanel, radii.lg radii.lg 0 0, max-height min(70dvh,520px), translateY animation). - test/setup.ts: stub window.matchMedia for jsdom (default no-match → desktop layout; tests can override). - MoreMenu.test.tsx: trigger aria-expanded toggle, panel role=region + aria-label, Escape/click-outside close, focus management, aria-disabled while decision active, auto-collapse on disable. - docs/style-guide.md §11.4: documents the compact-secondary trigger variant + bottom-anchored region pattern. Uses existing tokens only. Tested: yarn lint, yarn typecheck, yarn test (2369 passed). Co-authored-by: maniator <539579+maniator@users.noreply.github.com>
- Phase 2C: home-screen + home-league-teaser (desktop) - always-visible Career stats button shifts main menu layout above the 5% pixel tolerance on desktop. Mobile home-screen baselines remained within tolerance. - Phase 3: player-stats-panel + saves-modal-with-save (mobile/tablet WebKit) - mobile game header now collapses secondary controls behind a 'More' disclosure, which slightly reflows panels that overlay the field. Generated inside mcr.microsoft.com/playwright:v1.58.2-noble (matches CI). Co-authored-by: maniator <539579+maniator@users.noreply.github.com>
Agent-Logs-Url: https://github.com/maniator/blipit-legends/sessions/2bbc2963-7ab3-41b1-8b38-514e2e4be810 Co-authored-by: maniator <539579+maniator@users.noreply.github.com>
Co-authored-by: maniator <539579+maniator@users.noreply.github.com>
Co-authored-by: maniator <539579+maniator@users.noreply.github.com>
Co-authored-by: maniator <539579+maniator@users.noreply.github.com>
Co-authored-by: maniator <539579+maniator@users.noreply.github.com>
Agent-Logs-Url: https://github.com/maniator/blipit-legends/sessions/263c103d-1df7-4c42-b3ff-20c5a77a0c67 Co-authored-by: maniator <539579+maniator@users.noreply.github.com>
maniator
force-pushed
the
copilot/pr-241-recover-and-continue
branch
from
f069aa7 to
3b6087e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
import.spec.ts— button is now disabled for invalid JSON (Phase 2B); asserttoBeDisabled+ helper text instead of clickingqa-bugs.spec.ts(×2) —career-stats-no-teams→career-stats-empty-statewhen team exists but has no game history (Phase 2C)stat-budget.spec.ts—custom-team-save-error-hint→custom-team-editor-error-summary(Phase 2A)custom-team-editor.spec.ts— samecustom-team-save-error-hint→custom-team-editor-error-summaryfix (Phase 2A)