Multi-leaderboard display with tabs for projects#4262
Conversation
📝 WalkthroughWalkthroughFetches all project leaderboards, filters out empty ones, sorts by Changes
Sequence DiagramsequenceDiagram
participant Server as ProjectLeaderboard (server)
participant API as LeaderboardApi
participant Client as ProjectLeaderboardClient (client)
participant Table as ProjectLeaderboardTable
Server->>API: getProjectLeaderboard(projectId)
API-->>Server: LeaderboardDetails[] (all leaderboards)
Server->>Server: filter out leaderboards without entries
Server->>Server: sort by display_config.display_order
Server->>Client: pass leaderboards[]
Client->>Client: set activeLeaderboard = leaderboards[0]
alt multiple leaderboards
Client->>Client: render TabBar using display_config.display_name
Client->>Client: on tab select -> set activeLeaderboard
end
Client->>Table: render with activeLeaderboard and display_config
Table->>Table: apply column_renames for headers
Table-->>Client: rendered table with possibly renamed columns
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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
Show resolved
Hide resolved
🚀 Preview EnvironmentYour preview environment is ready!
Details
ℹ️ Preview Environment InfoIsolation:
Limitations:
Cleanup:
|
- 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
ncarazon
force-pushed
the
feat/multi-leaderboard-tabs-frontend
branch
from
64f1890 to
fa3fa21
Compare
lsabor
left a comment
lsabor
left a comment
There was a problem hiding this comment.
Great!
Works well, looks good, code is clean.
Left one non-blocking comment that should at least be considered.
| const defaultName = t(translationKey); | ||
| if (!columnRenames) { | ||
| return defaultName; | ||
| } |
There was a problem hiding this comment.
Non blocking but there is a bug. We can probably just make this a gh issue.
I noticed that the column rename doesn't work if the users are in non-english locales.
A quick fix for this would be rather than having defaultName be taken from t(translationKey), we can take the defaultName from the translation key in english.
This is ugly, but gets the job done:
import enMessages from "../../../../../../../messages/en.json";
...
const getColumnName = useCallback(
(
translationKey: Parameters<typeof t>[0],
columnRenames?: LeaderboardDisplayConfig["column_renames"]
): string => {
const getEnglishMessage = (key: string): string | undefined => {
const value = key
.split(".")
.reduce<unknown>(
(current, segment) =>
typeof current === "object" && current !== null
? (current as Record<string, unknown>)[segment]
: undefined,
enMessages as Record<string, unknown>
);
return typeof value === "string" ? value : undefined;
};
const englishName =
getEnglishMessage(String(translationKey)) ?? t(translationKey);
if (!columnRenames || !columnRenames[englishName]) {
return t(translationKey);
}
return columnRenames[englishName];
},
[t]
);
The technologically-simplest solution would be instead to store translation key names in the json on the leaderboard and add the translation keys to the en / es / ... .json files. The annoying part there is that our admins would then need to directly type in the translation keys into the field in the admin panel... I'll leave it to you and/or @elisescu to decide if this is worth doing.
3 participants
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