Skip to content

feat(predict): Adds scoreboard component#24528

Merged
matallui merged 16 commits intomainfrom
predict/nfl-scoreboard-component
Jan 15, 2026
Merged

feat(predict): Adds scoreboard component#24528
matallui merged 16 commits intomainfrom
predict/nfl-scoreboard-component

Conversation

@kevinbluer
Copy link
Copy Markdown
Contributor

@kevinbluer kevinbluer commented Jan 14, 2026 •
edited by cursor bot
Loading

Description

This PR adds sports scoreboard component that hands the various potential game states (pre-game, in-progress, half-time, delayed, and final). When a game is in-progress a possession indicator can be displayed next to a provided team. Similarly, when a game is over and a winner is specified, a trophy icon displays next to the winning team's abbreviation.

What's included:

  • PredictSportScoreboard component
  • PredictSportWinner component - trophy SVG icon component with dynamic color and size props
    • Default size: 16px (matching the original SVG viewBox)
    • Supports custom colors via hex, RGB, or other color formats
    • Includes comprehensive unit tests covering rendering, size variations, and edge cases
  • Winner enum - Added to PredictSportScoreboard.types.ts with Away, Home, and None values (mirrors the existing Possession enum pattern)
  • Scoreboard winner display logic - Updated PredictSportScoreboard to:
    • Accept optional winner prop (defaults to Winner.None)
    • Display trophy icon (14px, muted gray #9B9B9B) next to winning team's abbreviation
    • Only show trophy when gameState === GameState.Final and winner is specified
    • Trophy appears in the same position as the possession football indicator

Changelog

CHANGELOG entry: null

Related issues

Fixes:

Manual testing steps

Add the following to a page wherein you want to view the scoreboard (note the various game states are provided below for reference):

<PredictSportScoreboard
  awayTeam={{ abbreviation: 'SEA', color: '#002244' }}
  homeTeam={{ abbreviation: 'DEN', color: '#FB4F14' }}
  awayScore={109}
  homeScore={17}
  gameState={GameState.Final}
  quarter="Q3"
  time="12:02"
  date="Sun, Jan 14"
  possession={Possession.Away}
  winner={Winner.Home}
  testID="predict-positions-scoreboard"
/>
Feature: my feature name

  Scenario: user [verb for user action]
    Given [describe expected initial app state]

    When user [verb for user action]
    Then [describe expected outcome]

Screenshots/Recordings

Before

After

image image image image image

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Introduces a reusable NFL-style scoreboard with clear game-state display and indicators for possession and final winner.

  • New PredictSportScoreboard component with PreGame, InProgress, Halftime, Final states, optional eventTitle/date/time/quarter, and team helmets; shows scores except in PreGame
  • Possession indicator via PredictSportFootballIcon during InProgress; winner trophy via PredictSportWinner when Final and winner specified
  • Adds Winner enum and props in PredictSportScoreboard.types
  • Adds i18n strings predict.sports.halftime and predict.sports.final in locales/languages/en.json
  • Updates PredictSportFootballIcon default color to colors.text.alternative
  • Comprehensive unit tests for PredictSportScoreboard and PredictSportWinner

Written by Cursor Bugbot for commit a890b12. This will update automatically on new commits. Configure here.

@metamaskbot metamaskbot added the team-predict Predict team label Jan 14, 2026
@kevinbluer kevinbluer marked this pull request as ready for review January 14, 2026 17:26
@kevinbluer kevinbluer requested a review from a team as a code owner January 14, 2026 17:26
cursor[bot]
cursor bot reviewed Jan 15, 2026
View reviewed changes
app/components/UI/Predict/components/PredictSportWinner/PredictSportWinner.test.tsx
expect(svg.props.viewBox).toBe('0 0 16 16');
});
});
});
Copy link
Copy Markdown

@cursor cursor bot Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test for undefined color fallback behavior

Low Severity · Bugbot Rules

The PredictSportWinner component has a fallback for when color is undefined (color ?? colors.text.alternative), but every test passes a color prop. There is no test verifying the component renders correctly when color is omitted and uses the theme's alternative text color. This violates the unit testing guideline: "EVERY component MUST test: ✅ Edge cases - null, undefined, empty values".

Fix in Cursor Fix in Web

@matallui
Copy link
Copy Markdown
Contributor

matallui commented Jan 15, 2026

PR #24528 Review Summary

PR: feat(predict): Adds scoreboard component
Files Changed: 9 (+1,294 / -1)
Author: @kevinbluer

Overview

This PR adds a sports scoreboard component (PredictSportScoreboard) with support for multiple game states (PreGame, InProgress, Halftime, Final), a trophy icon component (PredictSportWinner), and comprehensive unit tests (996 lines of test code).

🔴 Critical Issues (0)

No critical issues found.

🟠 High Severity Issues (4)

1. Tests Don't Mock External Dependencies

Location: PredictSportScoreboard.test.tsx
Description: The test suite renders real implementations of PredictSportTeamHelmet, PredictSportFootballIcon, PredictSportWinner, and strings() instead of mocking them. This violates the repo's "mock everything external" rule and makes tests brittle.
Recommendation: Mock ../../../../../../locales/i18n and stub child components to simple placeholders.

2. "Size Variants" Tests Reference Non-Existent Prop

Location: PredictSportScoreboard.test.tsx:490-537
Description: The describe('size variants') block contains 3 tests that all do the same thing and reference a variant prop that doesn't exist in PredictSportScoreboardProps. These are misleading copy-paste tests.

// All three tests are identical - no actual variant prop exists
it('renders with default variant when variant prop omitted', () => ...);
it('renders with default variant when explicitly specified', () => ...);
it('renders with compact variant when specified', () => ...);

Recommendation: Remove this entire test block or implement the actual variant functionality.

3. Test Names Violate Guidelines

Location: PredictSportScoreboard.test.tsx:660, 682, 843
Description: Multiple test names use forbidden terms: "handles", "successfully". Per unit testing guidelines, these vague verbs should be avoided.

// ❌ Violates guidelines
it.each([...])('handles %s possession state', ...)
it.each([...])('renders %s state successfully', ...)
it.each([...])('handles %s winner state', ...)

Recommendation: Rename to explicit outcomes:

  • it('hides possession icon when possession is None', ...)
  • it('renders status label for each game state', ...)

4. Parameterized Tests Are No-Op Smoke Tests

Location: PredictSportScoreboard.test.tsx:654-695
Description: The it.each tests only assert that the root testID exists, without validating any behavior differences across states. They add maintenance cost without catching regressions.
Recommendation: Replace with assertions that vary by state (PreGame hides scores, Halftime shows i18n label, etc.).

🟡 Medium Severity Issues (3)

5. testID Creates "undefined-*" IDs When Prop Missing

Location: PredictSportScoreboard.tsx:122-176
Description: Child testIDs are built with template literals like `${testID}-away-helmet`. When testID is undefined, this creates "undefined-away-helmet".

// Current - problematic
testID={`${testID}-away-helmet`}

// Recommended fix
testID={testID ? `${testID}-away-helmet` : undefined}

Recommendation: Guard all child testIDs with conditional checks.

6. Score Defaulting to 0 May Be Misleading

Location: PredictSportScoreboard.tsx:130
Description: score ?? 0 displays 0 even when the API didn't provide a score. This could mislead users if "missing" means "not yet available" rather than "zero".
Recommendation: Confirm UX requirement. If score can be unknown, consider using a placeholder like —.

7. PredictSportWinner Tests Don't Mock useTheme

Location: PredictSportWinner.test.tsx
Description: Unlike PredictSportFootballIcon.test.tsx which mocks useTheme, the Winner tests don't. This inconsistency could cause failures if useTheme requires a provider.
Recommendation: Add mock for useTheme to match the existing pattern in similar components.

🟢 Low Severity Issues (4)

8. Football Icon Test Name Now Inaccurate

Location: PredictSportFootballIcon.test.tsx:77
Description: Test says "applies theme text default color" but implementation now uses colors.text.alternative.
Recommendation: Update test name to match actual behavior.

9. Enum Duplication (Possession & Winner)

Location: PredictSportScoreboard.types.ts:8-18
Description: Both enums have identical values (away, home, none). Minor duplication.
Recommendation: Optional - consider a shared TeamSide type if values drift becomes a concern.

10. Team Parameter Typing Could Be Cleaner

Location: PredictSportScoreboard.tsx:100
Description: team: typeof awayTeam | typeof homeTeam is harder to read than team: TeamData.
Recommendation: Use TeamData interface directly.

11. UNSAFE_getByType Usage in Tests

Location: PredictSportWinner.test.tsx:130
Description: Uses UNSAFE_getByType to access SVG viewBox, which relies on implementation details.
Recommendation: Accept as necessary for SVG testing or find alternative assertion approach.

✅ Positive Observations

Area Status Notes
Test Naming (no "should") ✅ Pass All 60+ tests follow action-oriented naming
AAA Pattern ✅ Pass Consistent Arrange-Act-Assert with proper separation
Type Safety ✅ Pass No any types, proper TypeScript interfaces
Design System Usage ✅ Pass Correctly uses Box, Text, TextVariant from design system
Tailwind Patterns ✅ Pass Uses twClassName correctly, no StyleSheet
Color Tokens ✅ Pass Uses text-default, text-alternative design tokens
Test Coverage ✅ Pass Comprehensive coverage of all game states, edge cases
File Organization ✅ Pass Proper structure (types.ts, index.ts, component, test)
Helper Functions ✅ Pass Uses factory functions (createAwayTeam, createHomeTeam)

Summary by Severity

Severity Count Action
🔴 Critical 0 -
🟠 High 4 Should fix before merge
🟡 Medium 3 Recommended to fix
🟢 Low 4 Nice to have

Recommended Actions Before Merge

  1. Remove or fix "size variants" tests (lines 490-537) - they test nothing
  2. Rename parameterized tests to be specific about what they verify
  3. Add guard for testID interpolation to prevent "undefined-*" IDs
  4. Consider mocking external dependencies in scoreboard tests

Estimated Effort: 1-4 hours for test fixes and cleanup

Verdict: The implementation code is solid and follows project conventions well. The main issues are in test quality - specifically redundant tests, guideline violations in test names, and missing mocks. Recommend addressing High severity items before merge.

Review generated by AI code review agents (Oracle, Frontend-UI-UX, Code-Review)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 15, 2026

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokePredictions
  • Risk Level: low
  • AI Confidence: 90%
click to see 🤖 AI reasoning details

The changes are isolated to the Predict feature, adding new UI components (PredictSportScoreboard and PredictSportWinner) and making a minor color change to PredictSportFootballIcon. These are purely presentational components for displaying sports game information in the predictions feature.

Key observations:

  1. All changes are within app/components/UI/Predict/components/ - a self-contained feature area
  2. New components are well-tested with comprehensive unit tests (964 lines for PredictSportScoreboard, 135 lines for PredictSportWinner)
  3. The only modification to existing code is a minor color change (text.default → text.alternative) in PredictSportFootballIcon
  4. Localization changes add two new strings ("Halftime" and "Final") for the sports feature
  5. No changes to core wallet functionality, controllers, or critical infrastructure
  6. The new components are not yet imported by any other files (only by their own index.ts and test files)

The SmokePredictions tag is the appropriate choice as it covers prediction market E2E tests. Running these tests will verify that the prediction feature continues to work correctly with the new sports-related UI components.

View GitHub Actions results

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Jan 15, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
96.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

matallui
matallui approved these changes Jan 15, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@matallui matallui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's get this in and we can refactor as we integrate this with the data logic.

@matallui matallui added this pull request to the merge queue Jan 15, 2026
Merged via the queue into main with commit 83c602a Jan 15, 2026
82 of 83 checks passed
@matallui matallui deleted the predict/nfl-scoreboard-component branch January 15, 2026 20:20
@github-actions github-actions bot locked and limited conversation to collaborators Jan 15, 2026
@metamaskbot metamaskbot added the release-7.63.0 Issue or pull request that will be included in release 7.63.0 label Jan 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@cursor cursor[bot] cursor[bot] left review comments

@matallui matallui matallui approved these changes

Assignees

No one assigned

Labels

release-7.63.0 Issue or pull request that will be included in release 7.63.0 size-XL team-predict Predict team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kevinbluer @matallui @metamaskbot