feat: update sponsor button style and improve leaderboard display logic#343
Merged
ViktorSvertoka merged 3 commits intodevelopfrom Feb 18, 2026
Merged
feat: update sponsor button style and improve leaderboard display logic#343ViktorSvertoka merged 3 commits intodevelopfrom
ViktorSvertoka merged 3 commits intodevelopfrom
Conversation
Updated Sponsor button color to match GitHub style Changed leaderboard logic to display: Top 15 users Ellipsis when user is outside top 15 Contextual ranks (±2 around current user)
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughAdds sponsor color tokens and scrollbar styles to global CSS; applies sponsor tokens and responsive CTA adjustments in ProfileCard and leaderboard components; changes LeaderboardTable to show top-15 with contextual nearby rows and increases DB query limit; converts home hero wrapper to a clickable motion button that scrolls to the next section; adds localization keys. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant FrontendApp
participant API
participant Database
Browser->>FrontendApp: Load leaderboard page
FrontendApp->>API: GET /api/leaderboard?limit=1000
API->>Database: Query top scores (limit=1000)
Database-->>API: Results (ordered)
API-->>FrontendApp: Leaderboard data
FrontendApp->>FrontendApp: compute top15, isCurrentInTop?, contextRows (±2)
FrontendApp-->>Browser: Render table with top rows and optional context rows, apply `--sponsor` tokens
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/components/leaderboard/LeaderboardTable.tsx (1)
109-134:⚠️ Potential issue | 🟡 MinorContext table is missing an accessible
<caption>.The main leaderboard table has
<caption className="sr-only">{t('tableCaption')}</caption>for screen-reader identification. The context table has no caption, so assistive technologies cannot distinguish it from the main table.♿ Suggested fix
<table className="w-full table-fixed border-separate border-spacing-0 text-left"> + <caption className="sr-only">{t('contextTableCaption')}</caption> <colgroup>Add a corresponding i18n key (e.g.,
"contextTableCaption": "Your position on the leaderboard").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/leaderboard/LeaderboardTable.tsx` around lines 109 - 134, Add an accessible, screen-reader-only caption to the context table by inserting a caption element using the translation key (e.g., t('contextTableCaption')) inside the same <table> where contextRows is rendered; update the i18n files to include "contextTableCaption": "Your position on the leaderboard" (or equivalent) so the caption is available to the t() call and assistive tech can distinguish this table from the main leaderboard.frontend/db/queries/leaderboard.ts (1)
46-49:⚠️ Potential issue | 🟠 Major
idfield contains rank position, not user ID—ID-based matching will always fail silently.The returned object sets
id: index + 1(rank: 1, 2, 3…) whileCurrentUser.idis a string auth/session ID. InLeaderboardTable.tsx, the comparisonString(u.id) === normalizedCurrentUserIdconverts the rank to a string and compares it against the user's actual auth ID, which will never match. User identification falls back entirely to theusernamematch condition, leaving the ID branch as dead code across all four matching locations (lines 32, 43, 85, 119).Consider using
userId(which contains the actual DB IDu.id) instead of the rank for ID-based matching.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/db/queries/leaderboard.ts` around lines 46 - 49, The returned leaderboard item currently sets id: index + 1 (rank) which breaks ID-based matching in LeaderboardTable.tsx; change the object returned in frontend/db/queries/leaderboard.ts so that id is set to the actual DB user id (u.id or String(u.id) to match CurrentUser.id normalization) and keep a separate rank field (rank: index + 1) — ensure the properties referenced in LeaderboardTable.tsx (id and userId checks) use the real user id value rather than the rank.
🧹 Nitpick comments (5)
frontend/components/leaderboard/LeaderboardTable.tsx (1)
40-54: RedundantfindIndex— derive index frommatchedUserdirectly.
matchedUserwas already located viausers.find()(lines 30–34) with the same predicate. A second O(n)findIndexscan is unnecessary;users.indexOf(matchedUser)gives the same index in O(1), or a singlefindIndexcan replace both lookups.♻️ Proposed refactor
- const matchedUser = users.find( - u => - String(u.id) === normalizedCurrentUserId || - (currentUsername && u.username === currentUsername) - ); + const matchedUserIndex = users.findIndex( + u => + String(u.id) === normalizedCurrentUserId || + (currentUsername && u.username === currentUsername) + ); + const matchedUser = matchedUserIndex !== -1 ? users[matchedUserIndex] : undefined; const currentUserRank = matchedUser?.rank || 0; const isUserInTop = currentUserRank > 0 && currentUserRank <= TOP_COUNT; const contextRows: User[] = []; if (!isUserInTop && matchedUser) { - const userIndex = users.findIndex( - u => - String(u.id) === normalizedCurrentUserId || - (currentUsername && u.username === currentUsername) - ); - - if (userIndex !== -1) { - const start = Math.max(TOP_COUNT, userIndex - CONTEXT_RANGE); - const end = Math.min(users.length - 1, userIndex + CONTEXT_RANGE); + if (matchedUserIndex !== -1) { + const start = Math.max(TOP_COUNT, matchedUserIndex - CONTEXT_RANGE); + const end = Math.min(users.length - 1, matchedUserIndex + CONTEXT_RANGE); for (let i = start; i <= end; i++) { contextRows.push(users[i]); } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/leaderboard/LeaderboardTable.tsx` around lines 40 - 54, The code redundantly re-scans users with findIndex even though matchedUser was already found; replace the second O(n) lookup by deriving the index via users.indexOf(matchedUser) (inside the existing if (!isUserInTop && matchedUser) block) and use that userIndex for computing start/end and pushing into contextRows (preserving TOP_COUNT and CONTEXT_RANGE logic); ensure you still check userIndex !== -1 before building the contextRows.frontend/db/queries/leaderboard.ts (1)
35-35: Unbounded query — consider a reasonable upper cap.Removing
limit(50)means every cache miss (once per hour) fetches every row in the users table. The 3600 s cache mitigates DB load, but cache payload size and cold-start latency grow linearly with user count. The UI only ever renders 15 + ±2 rows, so only a bounded superset (e.g., top ~500 or top 1000) is needed.💡 Suggested cap
- .orderBy(desc(sql`COALESCE(pt_valid.total, 0)`)); + .orderBy(desc(sql`COALESCE(pt_valid.total, 0)`)) + .limit(500); // generous cap; adjust to anticipated user growth🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/db/queries/leaderboard.ts` at line 35, The query currently ends with .orderBy(desc(sql`COALESCE(pt_valid.total, 0)`)) and is unbounded; add a reasonable upper cap (e.g., .limit(500) or .limit(1000)) to the same query chain to avoid fetching the entire users table on cache miss. Locate the leaderboard query in frontend/db/queries/leaderboard.ts (the chain that calls orderBy(desc(sql`COALESCE(pt_valid.total, 0)`))) and append a .limit(...) before executing/returning the query so the cache payload stays bounded while still covering the UI’s needs.frontend/components/leaderboard/LeaderboardPodium.tsx (1)
120-120:dark:text-(--sponsor)is redundant.
--sponsoris already redefined inside.dark {}inglobals.css, sotext-(--sponsor)resolves to the dark-mode color automatically. Onlydark:bg-(--sponsor)/15(which changes the opacity) has a meaningful effect. The same redundancy appears inLeaderboardTable.tsxline 204 andProfileCard.tsxline 69.🧹 Suggested cleanup
- className="mt-0.5 inline-flex items-center gap-1 rounded-full bg-(--sponsor)/10 px-2 py-0.5 text-[10px] font-bold text-(--sponsor) transition-colors hover:bg-(--sponsor)/20 dark:bg-(--sponsor)/15 dark:text-(--sponsor) dark:hover:bg-(--sponsor)/25" + className="mt-0.5 inline-flex items-center gap-1 rounded-full bg-(--sponsor)/10 px-2 py-0.5 text-[10px] font-bold text-(--sponsor) transition-colors hover:bg-(--sponsor)/20 dark:bg-(--sponsor)/15 dark:hover:bg-(--sponsor)/25"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/leaderboard/LeaderboardPodium.tsx` at line 120, Remove the redundant "dark:text-(--sponsor)" utility from the component classNames where the CSS custom property --sponsor is already redefined inside the .dark selector; specifically edit the className in LeaderboardPodium (the inline class string containing dark:bg-(--sponsor)/15) to drop dark:text-(--sponsor), and make the same removal in LeaderboardTable (the className at the table row/cell around line with text-(--sponsor)) and ProfileCard (the className that includes text-(--sponsor)); keep the dark:bg-(--sponsor)/15 opacity modifier and any other relevant classes intact so only the redundant dark:text-(--sponsor) token is removed.frontend/components/home/WelcomeHeroSection.tsx (1)
63-75: Scroll indicator icons don't respond to button's hover color.The inner animated dot (
bg-gray-400) andChevronDown(text-gray-400 dark:text-white/50) have explicit color classes that override the parent button'shover:text-(--accent-primary). On hover, the button text color changes but the icons stay gray.✨ Proposed fix — use `currentColor`/`group-hover` instead of hard-coded grays
Add
groupto the button and replace the static color classes:- className="absolute bottom-8 left-1/2 -translate-x-1/2 flex cursor-pointer flex-col items-center gap-2 text-gray-400 transition-colors hover:text-(--accent-primary) dark:text-white/50 dark:hover:text-(--accent-primary)" + className="group absolute bottom-8 left-1/2 -translate-x-1/2 flex cursor-pointer flex-col items-center gap-2 text-gray-400 transition-colors hover:text-(--accent-primary) dark:text-white/50 dark:hover:text-(--accent-primary)"- className="mx-auto h-1.5 w-1 rounded-full bg-gray-400 dark:bg-white/70" + className="mx-auto h-1.5 w-1 rounded-full bg-current"- <ChevronDown className="h-4 w-4 text-gray-400 dark:text-white/50" /> + <ChevronDown className="h-4 w-4 text-current" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/home/WelcomeHeroSection.tsx` around lines 63 - 75, The scroll indicator's inner dot (motion.div with class "mx-auto h-1.5 w-1 rounded-full bg-gray-400 dark:bg-white/70") and the ChevronDown icon (ChevronDown element with "text-gray-400 dark:text-white/50") use hard-coded colors, so they don't inherit the button hover color; make the parent button a "group" and replace the dot's background class with a current-color style (use bg-current) and the ChevronDown's color with text-current so both inherit the parent's text color (which should already toggle via hover:text-(--accent-primary) on the button/group).frontend/app/globals.css (1)
73-75:--sponsor-hoveris defined but not consumed.All sponsor badge and button hover states in this PR use opacity modifiers on
--sponsor(e.g.,hover:bg-(--sponsor)/20) rather thanvar(--sponsor-hover). The token is unused. Either wire it up to the hover states or defer its definition until it's actually needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/globals.css` around lines 73 - 75, The CSS variable --sponsor-hover is defined but never used; update the hover rules that currently apply opacity modifiers to var(--sponsor) to instead reference var(--sponsor-hover) (or remove the --sponsor-hover declaration if you prefer the opacity approach). Locate usages of --sponsor in hover styles (e.g., hover:bg-(--sponsor)/20 patterns) and change them to use var(--sponsor-hover) for hover background/fill/border where a distinct hover color is intended, or delete the --sponsor-hover token from globals.css if you decide to keep the opacity-based hover implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@frontend/components/leaderboard/LeaderboardTable.tsx`:
- Around line 109-134: Add an accessible, screen-reader-only caption to the
context table by inserting a caption element using the translation key (e.g.,
t('contextTableCaption')) inside the same <table> where contextRows is rendered;
update the i18n files to include "contextTableCaption": "Your position on the
leaderboard" (or equivalent) so the caption is available to the t() call and
assistive tech can distinguish this table from the main leaderboard.
In `@frontend/db/queries/leaderboard.ts`:
- Around line 46-49: The returned leaderboard item currently sets id: index + 1
(rank) which breaks ID-based matching in LeaderboardTable.tsx; change the object
returned in frontend/db/queries/leaderboard.ts so that id is set to the actual
DB user id (u.id or String(u.id) to match CurrentUser.id normalization) and keep
a separate rank field (rank: index + 1) — ensure the properties referenced in
LeaderboardTable.tsx (id and userId checks) use the real user id value rather
than the rank.
---
Nitpick comments:
In `@frontend/app/globals.css`:
- Around line 73-75: The CSS variable --sponsor-hover is defined but never used;
update the hover rules that currently apply opacity modifiers to var(--sponsor)
to instead reference var(--sponsor-hover) (or remove the --sponsor-hover
declaration if you prefer the opacity approach). Locate usages of --sponsor in
hover styles (e.g., hover:bg-(--sponsor)/20 patterns) and change them to use
var(--sponsor-hover) for hover background/fill/border where a distinct hover
color is intended, or delete the --sponsor-hover token from globals.css if you
decide to keep the opacity-based hover implementation.
In `@frontend/components/home/WelcomeHeroSection.tsx`:
- Around line 63-75: The scroll indicator's inner dot (motion.div with class
"mx-auto h-1.5 w-1 rounded-full bg-gray-400 dark:bg-white/70") and the
ChevronDown icon (ChevronDown element with "text-gray-400 dark:text-white/50")
use hard-coded colors, so they don't inherit the button hover color; make the
parent button a "group" and replace the dot's background class with a
current-color style (use bg-current) and the ChevronDown's color with
text-current so both inherit the parent's text color (which should already
toggle via hover:text-(--accent-primary) on the button/group).
In `@frontend/components/leaderboard/LeaderboardPodium.tsx`:
- Line 120: Remove the redundant "dark:text-(--sponsor)" utility from the
component classNames where the CSS custom property --sponsor is already
redefined inside the .dark selector; specifically edit the className in
LeaderboardPodium (the inline class string containing dark:bg-(--sponsor)/15) to
drop dark:text-(--sponsor), and make the same removal in LeaderboardTable (the
className at the table row/cell around line with text-(--sponsor)) and
ProfileCard (the className that includes text-(--sponsor)); keep the
dark:bg-(--sponsor)/15 opacity modifier and any other relevant classes intact so
only the redundant dark:text-(--sponsor) token is removed.
In `@frontend/components/leaderboard/LeaderboardTable.tsx`:
- Around line 40-54: The code redundantly re-scans users with findIndex even
though matchedUser was already found; replace the second O(n) lookup by deriving
the index via users.indexOf(matchedUser) (inside the existing if (!isUserInTop
&& matchedUser) block) and use that userIndex for computing start/end and
pushing into contextRows (preserving TOP_COUNT and CONTEXT_RANGE logic); ensure
you still check userIndex !== -1 before building the contextRows.
In `@frontend/db/queries/leaderboard.ts`:
- Line 35: The query currently ends with
.orderBy(desc(sql`COALESCE(pt_valid.total, 0)`)) and is unbounded; add a
reasonable upper cap (e.g., .limit(500) or .limit(1000)) to the same query chain
to avoid fetching the entire users table on cache miss. Locate the leaderboard
query in frontend/db/queries/leaderboard.ts (the chain that calls
orderBy(desc(sql`COALESCE(pt_valid.total, 0)`))) and append a .limit(...) before
executing/returning the query so the cache payload stays bounded while still
covering the UI’s needs.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/components/home/WelcomeHeroSection.tsx`:
- Line 61: The aria-label on the scroll button in WelcomeHeroSection is
hardcoded; route it through the i18n layer by adding a key (e.g.
"scrollToNextSection") to the homepage messages and replacing the hardcoded
string with t('homepage.scrollToNextSection') (use the existing t() import in
WelcomeHeroSection) so the aria-label is translated like the other user-facing
strings.
In `@frontend/components/leaderboard/LeaderboardTable.tsx`:
- Around line 39-50: The current logic building contextRows can produce a
contiguous block starting at rank TOP_COUNT+1 (e.g., userIndex == TOP_COUNT),
which makes the rendered ellipsis misleading; update the rendering condition (or
the determination of whether to show the ellipsis) to only show the `• • •` when
there is a real gap between the top list and contextRows — for example check the
computed start (or contextRows[0].rank) and only render the ellipsis when start
> TOP_COUNT (or contextRows[0].rank > TOP_COUNT + 1); keep the existing
contextRows construction (variables: contextRows, isUserInTop, matchedUser,
userIndex, TOP_COUNT, CONTEXT_RANGE, start, end) but gate the ellipsis display
behind that gap check.
- Around line 114-127: In the contextRows mapping, the isMe calculation uses the
wrong property (String(user.id)) so the current user is never marked; change the
comparison to use user.userId (i.e., compare user.userId to
normalizedCurrentUserId) while keeping the existing fallback that checks
currentUsername against user.username and pass the result into TableRow via
isCurrentUser; update the isMe expression inside the contextRows.map block to
reference user.userId and leave TableRow, t, and other props unchanged.
…ter click blocking - Scrollbar: minimal custom scrollbar (no arrows, dark/light theme, Chrome/Edge/Firefox) - Horizontal scroll: overflow-x: hidden on html prevents About page overflow - Leaderboard: fix contextRows user ID matching, conditional ellipsis when no gap - Footer: fix OnlineCounterPopup blocking clicks via pointer-events-none - i18n: translate scroll indicator aria-label and context table caption
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/components/home/WelcomeHeroSection.tsx (1)
53-56: Index-based DOM lookup for the scroll target is fragile.
steps[1]implicitly relies on the rendering order of all[data-home-step]elements in the document. If any sibling component conditionally renders adata-home-stepelement before the intended target, the scroll destination shifts silently. A named attribute value (data-home-step="features") is more resilient:♻️ Suggested refactor
- const steps = document.querySelectorAll<HTMLElement>('[data-home-step]'); - if (steps[1]) steps[1].scrollIntoView({ behavior: 'smooth' }); + const target = document.querySelector<HTMLElement>('[data-home-step="features"]'); + target?.scrollIntoView({ behavior: 'smooth' });Then update the target section's attribute to
data-home-step="features"(or whichever semantic name fits).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/home/WelcomeHeroSection.tsx` around lines 53 - 56, The current onClick handler in WelcomeHeroSection uses document.querySelectorAll<HTMLElement>('[data-home-step]') and then steps[1].scrollIntoView(...), which is fragile; change it to select the target by a named attribute value (e.g. document.querySelector<HTMLElement>('[data-home-step="features"]')) and call scrollIntoView on that element instead, and update the corresponding target section to have data-home-step="features" (or another semantic name) so the selector reliably finds the correct element.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@frontend/components/home/WelcomeHeroSection.tsx`:
- Line 61: The aria-label in WelcomeHeroSection.tsx is already internationalized
by using aria-label={t('scrollToNextSection')}, so no code change is required;
leave the aria-label as-is and approve the change (verify the translation key
'scrollToNextSection' exists in the i18n files and is used by the t function).
In `@frontend/components/leaderboard/LeaderboardTable.tsx`:
- Around line 116-119: The isMe check in the contextRows.map callback correctly
uses user.userId for comparison; confirm the condition in the anonymous function
(contextRows.map -> isMe) compares user.userId to normalizedCurrentUserId and
falls back to comparing user.username to currentUsername (user.userId,
normalizedCurrentUserId, currentUsername, user.username), keep this logic as-is
and ensure there are no other places still comparing a different id field for
context rows.
---
Nitpick comments:
In `@frontend/components/home/WelcomeHeroSection.tsx`:
- Around line 53-56: The current onClick handler in WelcomeHeroSection uses
document.querySelectorAll<HTMLElement>('[data-home-step]') and then
steps[1].scrollIntoView(...), which is fragile; change it to select the target
by a named attribute value (e.g.
document.querySelector<HTMLElement>('[data-home-step="features"]')) and call
scrollIntoView on that element instead, and update the corresponding target
section to have data-home-step="features" (or another semantic name) so the
selector reliably finds the correct element.
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
2 participants
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.
Updated Sponsor button color to match GitHub style
Changed leaderboard logic to display:
Top 15 users
Ellipsis when user is outside top 15
Contextual ranks (±2 around current user)
Summary by CodeRabbit
New Features
Improvements
Other