fix(leaderboard): resolve accessibility issues (W3C, Lighthouse 100%, Keyboard nav, VoiceOver)#165
Conversation
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughAdds Next.js Metadata export to the leaderboard page, introduces i18n leaderboard keys, removes the LeaderboardTabs component, updates leaderboard client/podium/table UI and styling, tweaks mobile menu aria-controls, and changes CookieBanner title from an h3 to a div. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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
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.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/components/leaderboard/LeaderboardTable.tsx (1)
13-15: Hardcoded caption should be translated for i18n consistency.The table caption provides important accessibility context for screen readers but remains hardcoded in English while the rest of the component uses translated strings. Consider adding a translation key for this caption.
Suggested fix
Add a new key to your translation files:
"tableCaption": "Leaderboard ranking for other participants"Then update the caption:
- <caption className="sr-only"> - Leaderboard ranking for other participants - </caption> + <caption className="sr-only"> + {t('tableCaption')} + </caption>
🤖 Fix all issues with AI agents
In `@frontend/components/leaderboard/LeaderboardClient.tsx`:
- Around line 44-46: Replace the hardcoded "Champions Arena" label in the
LeaderboardClient component with a translation key: add "championsArena":
"Champions Arena" to your locale JSON files and update the span in
LeaderboardClient.tsx to use the existing translation function (e.g.,
t('championsArena') / i18n.t('championsArena')) instead of the literal string;
keep the span's className and accessibility unchanged and provide a sensible
fallback if your translation helper supports one.
In `@frontend/components/leaderboard/LeaderboardPodium.tsx`:
- Around line 77-81: Replace the hardcoded "pts" suffix in the LeaderboardPodium
component with a translatable string: import and use the existing
useTranslations hook (or i18n utility used in the project) inside
LeaderboardPodium to get t('pts') and render that instead of the literal "pts";
also add the "pts" key to the locale JSON files (e.g., "pts": "pts" for en and
appropriate translations for other locales). Ensure you reference the t function
where {user.points} is rendered so the suffix becomes {t('pts')} (or the
project's equivalent lookup).
- Around line 10-12: The hardcoded aria-label on the ol in LeaderboardPodium.tsx
should be localized: import and call the project i18n hook (e.g.,
useTranslations) inside the LeaderboardPodium component to read a new key (e.g.,
"topThreeLabel") from translations and replace the static aria-label="Top 3
Leaders" with aria-label={t('topThreeLabel')}; also add "topThreeLabel": "Top 3
Leaders" to your locale translation files so the key resolves.
🧹 Nitpick comments (2)
frontend/components/leaderboard/LeaderboardTable.tsx (2)
63-66: Consider conditionally displaying the "Rising" indicator based on actual change data.The
Usertype includes achangefield, but the "Rising" indicator displays unconditionally for all users on hover. This could be misleading if a user's ranking is actually falling.Suggested conditional rendering
- <span className="flex items-center gap-1 text-[10px] text-emerald-600 dark:text-emerald-400 font-bold uppercase tracking-wide opacity-0 group-hover:opacity-100 transition-opacity"> - <TrendingUp className="w-3 h-3" aria-hidden="true" /> - {t('rising')} - </span> + {user.change > 0 && ( + <span className="flex items-center gap-1 text-[10px] text-emerald-600 dark:text-emerald-400 font-bold uppercase tracking-wide opacity-0 group-hover:opacity-100 transition-opacity"> + <TrendingUp className="w-3 h-3" aria-hidden="true" /> + {t('rising')} + </span> + )}
71-75: Consider using locale-aware number formatting for points.Raw numeric output loses locale-specific formatting (e.g.,
1,000vs1.000vs1 000). For consistency with the i18n approach, consider usingtoLocaleString()or a formatting utility.Suggested fix
- <span className="font-mono font-bold text-slate-800 dark:text-slate-100 group-hover:scale-105 inline-block transition-transform"> - {user.points} - </span> + <span className="font-mono font-bold text-slate-800 dark:text-slate-100 group-hover:scale-105 inline-block transition-transform"> + {user.points.toLocaleString()} + </span>
Summary
This PR fixes critical accessibility and semantic HTML issues on the Leaderboard page and Global Header. The goal was to achieve WCAG compliance and pass automated audits.
🛠 Key Changes
1. HTML Semantics & W3C Validation
<main>tags (replaced inner main withdivwrapper).h1->h2->h3).aria-labelattributes ondivelements.2. Accessibility (A11y)
aria-controlsand ID references (removed spaces in IDs) to ensure proper screen reader navigation.aria-controlserror where the button referenced a non-existent ID when the menu was closed.3. Metadata
export const metadatatoleaderboard/page.tsx(fixed missing<title>error).✅ Verification Results
Summary by CodeRabbit
New Features
Style
Accessibility
✏️ Tip: You can customize this high-level summary in your review settings.