-
Notifications
You must be signed in to change notification settings - Fork 27
Multi-leaderboard display with tabs for projects #4262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughFetches all project leaderboards, filters out leaderboards without entries, sorts by Changes
Sequence DiagramsequenceDiagram
participant Server as Server
participant API as LeaderboardApi
participant Client as ProjectLeaderboardClient
participant Table as ProjectLeaderboardTable
Server->>API: getProjectLeaderboard(projectId, params?)
API-->>Server: LeaderboardDetails[] | null
Server->>Server: filter out leaderboards with no entries
Server->>Server: sort by display_config.display_order
Server->>Client: pass leaderboards[]
Client->>Client: set activeLeaderboard = leaderboards[0] (if any)
alt multiple leaderboards
Client->>Client: render TabBar using display_config.display_name / fallback
Client->>Client: on tab select -> set activeLeaderboard
end
Client->>Table: render table with activeLeaderboard and display_config
Table->>Table: resolve headers via column_renames or translations
Table-->>Client: rendered table
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@front_end/src/app/`(main)/(leaderboards)/leaderboard/components/project_leaderboard_table/index.tsx:
- Around line 35-39: The current lookup uses the translated label (defaultName =
t(translationKey)) to index columnRenames which breaks in non-English locales;
change the lookup to use the stable translation key (translationKey) when
checking columnRenames (e.g. check columnRenames[translationKey] and if present
return that renamed string, otherwise fall back to the existing behavior of
columnRenames[defaultName] and finally defaultName) so renames are
locale-independent; update the code around defaultName, translationKey and
columnRenames to try translationKey first, then the translated label, then the
untranslated default.
In `@front_end/src/types/scoring.ts`:
- Around line 102-107: The new LeaderboardDisplayConfig field display_on_project
is never used; update the filtering in project_leaderboard.tsx (where it
currently checks entries.length > 0) to also respect config.display_on_project
(treat undefined as true for backward compatibility) so leaderboards with
display_on_project=false are hidden on project pages; reference the
LeaderboardDisplayConfig type and the display_on_project property when locating
the code to change and ensure the filter uses config.display_on_project
alongside entries.length.
🧹 Nitpick comments (2)
front_end/src/app/(main)/(leaderboards)/leaderboard/components/project_leaderboard_table/index.tsx (1)
30-42:columnRenamesparameter is redundant — simplify by closing over the outer variable.
getColumnNamealways receives the samecolumnRenamesfrom the component scope at every call site. Closing over it directly eliminates the repeated argument and simplifies all 7+ call sites.♻️ Suggested simplification
- const getColumnName = useCallback( - ( - translationKey: Parameters<typeof t>[0], - columnRenames?: LeaderboardDisplayConfig["column_renames"] - ): string => { - const defaultName = t(translationKey); - if (!columnRenames) { - return defaultName; - } - return columnRenames[defaultName] ?? defaultName; - }, - [t] - ); + const getColumnName = useCallback( + (translationKey: Parameters<typeof t>[0]): string => { + const defaultName = t(translationKey); + if (!columnRenames) { + return defaultName; + } + return columnRenames[defaultName] ?? defaultName; + }, + [t, columnRenames] + );Then simplify all call sites, e.g.:
- {getColumnName("rank", columnRenames)} + {getColumnName("rank")}front_end/src/app/(main)/(leaderboards)/leaderboard/components/project_leaderboard_client.tsx (1)
94-101:toLocaleString()called without explicit locale — inconsistent with tournament_timeline.In
tournament_timeline.tsx(Line 82),prize_poolis formatted with an explicitlocaleargument. Here it relies on the browser default, which may produce different formatting for the same user.♻️ Suggested fix
Pass locale from
next-intlfor consistent formatting:+ import { useLocale } from "next-intl"; ... + const locale = useLocale(); ... - ${activeLeaderboard.prize_pool.toLocaleString()} + ${activeLeaderboard.prize_pool.toLocaleString(locale)}
...end/src/app/(main)/(leaderboards)/leaderboard/components/project_leaderboard_table/index.tsx
Outdated
Show resolved
Hide resolved
🧹 Preview Environment Cleaned UpThe preview environment for this PR has been destroyed.
Cleanup triggered by PR close at 2026-02-09T09:08:45Z |
elisescu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
64f1890 to
fa3fa21
Compare
lsabor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
Works well, looks good, code is clean.
Left one non-blocking comment that should at least be considered.
...end/src/app/(main)/(leaderboards)/leaderboard/components/project_leaderboard_table/index.tsx
Outdated
Show resolved
Hide resolved
- Update API service to accept URLSearchParams and return LeaderboardDetails[] - Add LeaderboardDisplayConfig type with display_name, column_renames, display_order - Sort leaderboards by display_order in server component - Show TabBar when multiple leaderboards exist, hide for single leaderboard - Apply display_config.display_name for tab labels with fallback to name - Support column_renames for custom column header names in table
024997c to
2765a2e
Compare
Closes #4251
This PR adds support for displaying multiple leaderboards with tab navigation on project pages, along with custom display names and column renaming capabilities.
Multiple Leaderboards with Tabs
Tab 1: "Official Results" with renamed columns (Participant, Final Score)
Tab 3: "Pre-DQ Results" with renamed column (Raw Score)
Single Leaderboard - No Tabs (Backward Compatibility)
Bridgewater Tournament: Standard leaderboard display without tabs
Summary by CodeRabbit
New Features
Bug Fixes