feat: header translations, dashboard UX polish & hydration fix#360
Merged
ViktorSvertoka merged 4 commits intodevelopfrom Feb 23, 2026
Merged
feat: header translations, dashboard UX polish & hydration fix#360ViktorSvertoka merged 4 commits intodevelopfrom
ViktorSvertoka merged 4 commits intodevelopfrom
Conversation
- Translate all hardcoded header strings across en/uk/pl:
- NotificationBell: title, markAllRead, syncing, empty state, justNow, type labels
- LanguageSwitcher: title
- UserNavDropdown: "My Account" label
- Intl.RelativeTimeFormat now uses actual user locale (was hardcoded 'en')
- Add notification translations to actions (quiz achievement, profile name/password change)
- Fix AchievementBadge hydration mismatch: replace direct document.documentElement check
with useState(false) + useEffect + MutationObserver (eliminates SSR/client mismatch)
- Dashboard cards: unify all cards to use dashboard-card CSS class for consistent hover
- ProfileCard: make stat items clickable (Quizzes→quiz results, Points→stats, Rank→leaderboard)
with smooth scrollIntoView behavior
- CartButton: restyle to match LanguageSwitcher/NotificationBell minimal icon button
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
📝 WalkthroughWalkthroughAdds locale-aware UI/text and translations, introduces a ScrollWatcher client component to toggle scrolling state, swaps several mount-guards to useSyncExternalStore, tightens CSS scrollbar/perspective utilities, refines leaderboard avatar hostname detection, and applies styling/import reorderings across dashboard and shared components. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 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
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/components/dashboard/ProfileCard.tsx (1)
138-211:⚠️ Potential issue | 🟡 Minor
<dt>and<dd>elements are used outside a<dl>container — invalid HTML.The grid container was changed from
<dl>to<div>, but the children still use<dt>and<dd>elements (lines 149, 152, 168, 171, 183, 186, 198, 201). Per the HTML spec,<dt>and<dd>must be children of a<dl>(or<div>inside a<dl>). This will cause HTML validation warnings and may confuse assistive technology.Either wrap the grid in a
<dl>again, or replace<dt>/<dd>with<span>or<p>elements:Option A: Re-wrap with dl
- <div className="grid w-full grid-cols-2 gap-2 sm:grid-cols-4 sm:gap-3 xl:flex xl:w-auto xl:flex-nowrap xl:items-center xl:justify-end xl:gap-2 2xl:gap-3"> + <dl className="grid w-full grid-cols-2 gap-2 sm:grid-cols-4 sm:gap-3 xl:flex xl:w-auto xl:flex-nowrap xl:items-center xl:justify-end xl:gap-2 2xl:gap-3"> ... - </div> + </dl>Option B: Replace dt/dd with span
Replace all
<dt>with<span>and<dd>with<span>(adjusting display if needed).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/dashboard/ProfileCard.tsx` around lines 138 - 211, The component ProfileCard uses <dt> and <dd> inside a plain <div> (the grid container) which is invalid HTML; update the markup so those term/description elements are valid by either restoring the container to a <dl> around the grid or replacing all <dt> and <dd> instances with semantic inline/block elements (e.g., <span> or <p>) inside the grid; locate the occurrences in the ProfileCard component (look for statItemBase and iconBoxStyles blocks for Attempts, Points, Global rank, Joined) and implement one of the two options consistently so HTML validation and screen-reader semantics are correct.
🧹 Nitpick comments (5)
frontend/app/[locale]/leaderboard/page.tsx (1)
73-82: LGTM — hostname check is a correctness improvement overincludes.The previous
avatarBase.includes('avatars.githubusercontent.com')would yield a false positive for a URL likehttps://evil.com/avatars.githubusercontent.com/...; the hostname-based parse correctly rejects that. The try-catch also safely handles the empty-string case (new URL('')throws) without requiring a separate guard.If you'd prefer a slightly flatter shape, the IIFE can be inlined as a plain
letblock:♻️ Optional: replace IIFE with a plain let/try-catch
- const isGitHubAvatar = (() => { - try { - return new URL(avatarBase).hostname === 'avatars.githubusercontent.com'; - } catch { - return false; - } - })(); + let isGitHubAvatar = false; + try { + isGitHubAvatar = new URL(avatarBase).hostname === 'avatars.githubusercontent.com'; + } catch { + // avatarBase is empty or not a valid absolute URL + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`[locale]/leaderboard/page.tsx around lines 73 - 82, The current IIFE that computes isGitHubAvatar is correct but can be flattened for readability: replace the self-invoking function with a simple let declaration for isGitHubAvatar, then use a try-catch that sets isGitHubAvatar = new URL(avatarBase).hostname === 'avatars.githubusercontent.com' or false on error; keep the hasStarred calculation using stargazerLogins.has(nameLower) || (isGitHubAvatar && stargazerAvatars.has(avatarBase)).frontend/components/dashboard/ExplainedTermsCard.tsx (1)
275-320: Consider usingtermalone as the React key for draggable items.
key={\${term}-${index}`}ties the key to the item's position. After every reorder, all keys change and React remounts every chip unnecessarily. Becausetermvalues appear to be unique strings within the list, using justterm` as the key would keep DOM nodes stable across reorders and is the idiomatic choice for drag-and-drop lists.♻️ Proposed fix
- key={`${term}-${index}`} + key={term}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/dashboard/ExplainedTermsCard.tsx` around lines 275 - 320, The React key for each draggable chip is currently key={`${term}-${index}`}, which ties identity to position and causes unnecessary remounts on reorder; change the key to use the stable unique term string (key={term}) in the mapped div return (the element that also uses ref={setTermRef(index)} and handlers handleDragOver, handleDrop, handleDragStart, handleTouchStart, handleRemoveTerm) so DOM nodes remain stable during drag-and-drop; if term might not be globally unique, add a stable unique id to the term data and use that id instead.frontend/components/dashboard/ProfileCard.tsx (1)
140-143: Consider extracting the scroll handler to reduce inline verbosity.The same smooth-scroll pattern is repeated for two stat items (lines 143 and 162). A small helper would reduce duplication and improve readability:
Suggested extraction
+ const scrollTo = (id: string) => (e: React.MouseEvent) => { + e.preventDefault(); + document.getElementById(id)?.scrollIntoView({ behavior: 'smooth' }); + }; + <a href="#quiz-results" className={statItemBase} - onClick={e => { e.preventDefault(); document.getElementById('quiz-results')?.scrollIntoView({ behavior: 'smooth' }); }} + onClick={scrollTo('quiz-results')} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/dashboard/ProfileCard.tsx` around lines 140 - 143, Duplicate inline smooth-scroll handlers in ProfileCard.tsx make the JSX verbose; extract the logic into a small helper function (e.g., handleSmoothScroll or scrollToElementById) that takes an element id, calls e.preventDefault(), and runs document.getElementById(id)?.scrollIntoView({ behavior: 'smooth' }), then replace the inline arrow handlers on the elements using statItemBase (the anchor with href="#quiz-results" and the other stat item) with onClick={e => handleSmoothScroll(e, 'quiz-results')} (or a bound/curried version) to remove duplication and improve readability while preserving existing behavior.frontend/components/dashboard/ActivityHeatmapCard.tsx (1)
473-475: Hardcoded English strings in tooltip — consider localizing in a follow-up.The tooltip text (
"No activity","attempt","attempts"on lines 473–475) and"active day"/"active days"on line 511 are hardcoded in English. Given this PR's i18n focus, these could be localized as a follow-up to keep the dashboard fully translated.Would you like me to open an issue to track localizing these remaining hardcoded strings?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/dashboard/ActivityHeatmapCard.tsx` around lines 473 - 475, The tooltip and summary strings in ActivityHeatmapCard (the JSX that renders {tooltip.count === 0 ? 'No activity' : `${tooltip.count} ${tooltip.count === 1 ? 'attempt' : 'attempts'}`} and the "active day"/"active days" text) are hardcoded English; replace them with your i18n lookup (e.g., use the project's useTranslation/t or i18n.t function) and use pluralization keys so counts render correctly (provide keys like "heatmap.noActivity", "heatmap.attempt" with plural forms, and "heatmap.activeDay" pluralized), update the JSX to call the translator with tooltip.count to select plural forms, and ensure the component imports and uses the same translation hook/utility as other localized components (reference ActivityHeatmapCard component, tooltip.count, and the summary label rendering).frontend/components/header/NotificationBell.tsx (1)
11-25:Intl.RelativeTimeFormatinstantiated per notification per render — consider caching by locale.
getRelativeTimeis called once per notification inside.map()(Line 240), creating a freshIntl.RelativeTimeFormatfor each. ICU initialization is non-trivial; the instance is stateless after construction and safe to cache.♻️ Proposed fix — module-level cache keyed by locale
+const rtfCache = new Map<string, Intl.RelativeTimeFormat>(); +function getRtf(locale: string): Intl.RelativeTimeFormat { + if (!rtfCache.has(locale)) { + rtfCache.set(locale, new Intl.RelativeTimeFormat(locale, { numeric: 'auto' })); + } + return rtfCache.get(locale)!; +} function getRelativeTime(date: Date, locale: string, justNow: string) { - const rtf = new Intl.RelativeTimeFormat(locale, { numeric: 'auto' }); + const rtf = getRtf(locale); const now = new Date().getTime();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/header/NotificationBell.tsx` around lines 11 - 25, getRelativeTime creates a new Intl.RelativeTimeFormat on every call (invoked per-notification in the .map) causing unnecessary ICU initialization; change it to reuse a cached RelativeTimeFormat per locale by adding a module-level Map<string, Intl.RelativeTimeFormat> (or similar) and lookup/create the rtf inside getRelativeTime instead of instantiating each time, keeping the existing function signature and behavior (use the cached rtf for locale, then compute minutes/hours/daysDifference and call rtf.format as before).
🤖 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/actions/quiz.ts`:
- Around line 236-242: The code currently calls getTranslations and stores
rendered text via createNotification inside the loop over newlyEarned using
tNotify('title') and tNotify('message', { id: achievement.id }), which freezes
the notification in the user's current locale and interpolates a raw achievement
id; change this to persist a translation key and structured params instead of
pre-rendered strings (e.g., store { key:
'notifications.achievement.unlocked.title' / '.message', params: { id:
achievement.id } }) or, if you must store text, resolve the localized badge name
before interpolation by looking up the achievement display name via the i18n
lookup and pass that into tNotify (replace interpolation of achievement.id with
the localized badge name) so notifications can be re-rendered later in other
locales; update createNotification usage and any consumers to accept key+params
or the localized name accordingly.
In `@frontend/app/globals.css`:
- Around line 132-145: The WebKit/Blink scrollbar pseudo-element transition is
ineffective; remove the dead transition from *::-webkit-scrollbar-thumb and
implement the fade via a CSS custom property on html (e.g.,
--scroll-thumb-alpha) that the thumb uses like background:
rgba(0,0,0,var(--scroll-thumb-alpha)); add a transition on the html selector (or
html.is-scrolling) to animate --scroll-thumb-alpha between 0 and the target
(0.25 / 0.2 for dark) when toggling html.is-scrolling, and update the existing
html.is-scrolling and html.is-scrolling:is(.dark) / html.is-scrolling .dark
rules to set the appropriate --scroll-thumb-alpha values so the fade works
cross-browser without relying on pseudo-element transitions.
In `@frontend/components/dashboard/AchievementBadge.tsx`:
- Line 193: AchievementBadge uses a non-existent CSS utility class
perspective-midrange; either add a new CSS rule for .perspective-midrange in
frontend/app/globals.css (matching the project's utility naming and giving an
appropriate perspective value similar to .perspective-1000) or update the
AchievementBadge component to use an existing class such as perspective-1000;
locate the class reference in the AchievementBadge component and either add the
.perspective-midrange definition to globals.css or replace the className entry
with the existing perspective utility.
In `@frontend/components/header/NotificationBell.tsx`:
- Line 236: The translation lookup uses notification.type (a string) without
guarding unknown values, causing runtime errors; add a runtime guard that maps
notification.type to the allowed union before calling t — e.g., implement a
small helper (getSafeNotificationType or normalizeNotificationType) or inline
check in NotificationBell.tsx that returns one of
'SYSTEM'|'ACHIEVEMENT'|'ARTICLE'|'SHOP' if it matches, otherwise returns the
same fallback used by getIconForType/getIconStylesForType (e.g., 'SYSTEM'); then
use that safe value in t(`types.${safeType}` as const) to avoid throwing on
unexpected backend types.
In `@frontend/components/shared/ScrollWatcher.tsx`:
- Around line 18-21: The cleanup in ScrollWatcher misses removing the html
"is-scrolling" class if the component unmounts before the timeout fires; update
the effect cleanup to also remove
document.documentElement.classList.remove('is-scrolling') (and ensure any
timeout variable used by onScroll is cleared) so the class is always removed on
unmount — modify the same effect that registers onScroll and uses timeout (the
onScroll handler and its timeout variable) to perform this removal in its return
cleanup.
In `@frontend/messages/en.json`:
- Around line 1537-1540: The notification template "unlocked" currently uses
"{id}" which will show raw badge keys (e.g., first_blood); update the template
under the "unlocked" key to use a descriptive parameter like "{badgeName}" (or
"{name}") and change the message to "You just earned the {badgeName} badge!";
then update the notification creation code (where the server action assembles
notifications) to resolve the human-readable label from
dashboard.achievements.badges.{id}.name and pass that string as the badgeName
parameter instead of the badge ID so users see "First Blood" rather than
"first_blood".
In `@frontend/messages/uk.json`:
- Around line 1540-1542: The notification uses the raw internal achievement.id
(e.g., first_blood) in the message template; update the three locale templates
(en.json, pl.json, uk.json) to use a {name} placeholder instead of {id}, and
then change the notification creation in frontend/actions/quiz.ts so
tNotify('message', ...) passes the localized label by resolving
dashboard.achievements.badges.{achievement.id}.name (e.g., call the i18n lookup
for dashboard.achievements.badges.{id}.name and pass that string as the name
property) instead of passing achievement.id.
---
Outside diff comments:
In `@frontend/components/dashboard/ProfileCard.tsx`:
- Around line 138-211: The component ProfileCard uses <dt> and <dd> inside a
plain <div> (the grid container) which is invalid HTML; update the markup so
those term/description elements are valid by either restoring the container to a
<dl> around the grid or replacing all <dt> and <dd> instances with semantic
inline/block elements (e.g., <span> or <p>) inside the grid; locate the
occurrences in the ProfileCard component (look for statItemBase and
iconBoxStyles blocks for Attempts, Points, Global rank, Joined) and implement
one of the two options consistently so HTML validation and screen-reader
semantics are correct.
---
Nitpick comments:
In `@frontend/app/`[locale]/leaderboard/page.tsx:
- Around line 73-82: The current IIFE that computes isGitHubAvatar is correct
but can be flattened for readability: replace the self-invoking function with a
simple let declaration for isGitHubAvatar, then use a try-catch that sets
isGitHubAvatar = new URL(avatarBase).hostname ===
'avatars.githubusercontent.com' or false on error; keep the hasStarred
calculation using stargazerLogins.has(nameLower) || (isGitHubAvatar &&
stargazerAvatars.has(avatarBase)).
In `@frontend/components/dashboard/ActivityHeatmapCard.tsx`:
- Around line 473-475: The tooltip and summary strings in ActivityHeatmapCard
(the JSX that renders {tooltip.count === 0 ? 'No activity' : `${tooltip.count}
${tooltip.count === 1 ? 'attempt' : 'attempts'}`} and the "active day"/"active
days" text) are hardcoded English; replace them with your i18n lookup (e.g., use
the project's useTranslation/t or i18n.t function) and use pluralization keys so
counts render correctly (provide keys like "heatmap.noActivity",
"heatmap.attempt" with plural forms, and "heatmap.activeDay" pluralized), update
the JSX to call the translator with tooltip.count to select plural forms, and
ensure the component imports and uses the same translation hook/utility as other
localized components (reference ActivityHeatmapCard component, tooltip.count,
and the summary label rendering).
In `@frontend/components/dashboard/ExplainedTermsCard.tsx`:
- Around line 275-320: The React key for each draggable chip is currently
key={`${term}-${index}`}, which ties identity to position and causes unnecessary
remounts on reorder; change the key to use the stable unique term string
(key={term}) in the mapped div return (the element that also uses
ref={setTermRef(index)} and handlers handleDragOver, handleDrop,
handleDragStart, handleTouchStart, handleRemoveTerm) so DOM nodes remain stable
during drag-and-drop; if term might not be globally unique, add a stable unique
id to the term data and use that id instead.
In `@frontend/components/dashboard/ProfileCard.tsx`:
- Around line 140-143: Duplicate inline smooth-scroll handlers in
ProfileCard.tsx make the JSX verbose; extract the logic into a small helper
function (e.g., handleSmoothScroll or scrollToElementById) that takes an element
id, calls e.preventDefault(), and runs
document.getElementById(id)?.scrollIntoView({ behavior: 'smooth' }), then
replace the inline arrow handlers on the elements using statItemBase (the anchor
with href="#quiz-results" and the other stat item) with onClick={e =>
handleSmoothScroll(e, 'quiz-results')} (or a bound/curried version) to remove
duplication and improve readability while preserving existing behavior.
In `@frontend/components/header/NotificationBell.tsx`:
- Around line 11-25: getRelativeTime creates a new Intl.RelativeTimeFormat on
every call (invoked per-notification in the .map) causing unnecessary ICU
initialization; change it to reuse a cached RelativeTimeFormat per locale by
adding a module-level Map<string, Intl.RelativeTimeFormat> (or similar) and
lookup/create the rtf inside getRelativeTime instead of instantiating each time,
keeping the existing function signature and behavior (use the cached rtf for
locale, then compute minutes/hours/daysDifference and call rtf.format as
before).
frontend/actions/quiz.ts
Outdated
Comment on lines
+236
to
+242
| const tNotify = await getTranslations('notifications.achievement.unlocked'); | ||
| for (const achievement of newlyEarned) { | ||
| // Find full object to get the fancy translated string (if needed) or just generic name | ||
| await createNotification({ | ||
| userId: session.id, | ||
| type: 'ACHIEVEMENT', | ||
| title: 'Achievement Unlocked!', | ||
| message: `You just earned the ${achievement.id} badge!`, | ||
| title: tNotify('title'), | ||
| message: tNotify('message', { id: achievement.id }), |
Contributor
There was a problem hiding this comment.
Notification text is persisted in the user's current locale — future locale switches won't update it.
getTranslations resolves the locale from the current request. The translated title/message are then stored verbatim in the database via createNotification. If a user earns an achievement while using Polish and later switches to English, the notification will still appear in Polish. Consider storing a translation key + parameters instead of pre-rendered text, or accept this as a known limitation.
Additionally, tNotify('message', { id: achievement.id }) interpolates the raw achievement ID (e.g., "first_blood") into the user-facing message. In pl.json this produces something like "Właśnie zdobyłeś odznakę first_blood!". You likely want to resolve the localized badge name instead:
Suggested fix
- const tNotify = await getTranslations('notifications.achievement.unlocked');
+ const tNotify = await getTranslations('notifications.achievement.unlocked');
+ const tBadges = await getTranslations('dashboard.achievements.badges');
for (const achievement of newlyEarned) {
await createNotification({
userId: session.id,
type: 'ACHIEVEMENT',
title: tNotify('title'),
- message: tNotify('message', { id: achievement.id }),
+ message: tNotify('message', { id: tBadges(`${achievement.id}.name`) }),
metadata: { badgeId: achievement.id, icon: achievement.icon },
});
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/actions/quiz.ts` around lines 236 - 242, The code currently calls
getTranslations and stores rendered text via createNotification inside the loop
over newlyEarned using tNotify('title') and tNotify('message', { id:
achievement.id }), which freezes the notification in the user's current locale
and interpolates a raw achievement id; change this to persist a translation key
and structured params instead of pre-rendered strings (e.g., store { key:
'notifications.achievement.unlocked.title' / '.message', params: { id:
achievement.id } }) or, if you must store text, resolve the localized badge name
before interpolation by looking up the achievement display name via the i18n
lookup and pass that into tNotify (replace interpolation of achievement.id with
the localized badge name) so notifications can be re-rendered later in other
locales; update createNotification usage and any consumers to accept key+params
or the localized name accordingly.
Comment on lines
132
to
145
| *::-webkit-scrollbar-thumb { | ||
| background: rgba(0, 0, 0, 0.25); | ||
| background: rgba(0, 0, 0, 0); | ||
| border-radius: 3px; | ||
| transition: background 0.3s ease; | ||
| } | ||
|
|
||
| :is(.dark) *::-webkit-scrollbar-thumb, | ||
| .dark::-webkit-scrollbar-thumb { | ||
| html.is-scrolling *::-webkit-scrollbar-thumb { | ||
| background: rgba(0, 0, 0, 0.25); | ||
| } | ||
|
|
||
| html.is-scrolling:is(.dark) *::-webkit-scrollbar-thumb, | ||
| html.is-scrolling .dark *::-webkit-scrollbar-thumb { | ||
| background: rgba(255, 255, 255, 0.2); | ||
| } |
Contributor
There was a problem hiding this comment.
Scrollbar thumb transition won't animate in WebKit/Blink browsers.
The transition: background 0.3s ease on *::-webkit-scrollbar-thumb (line 135) is declared but WebKit/Blink browsers do not support CSS transitions on scrollbar pseudo-elements. The scrollbar will appear/disappear instantly rather than fading. This is purely cosmetic — the functionality is correct — but the transition declaration is effectively dead code in those browsers.
The Firefox scrollbar-color transition (line 151) has better support, so the fade effect may work there.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/globals.css` around lines 132 - 145, The WebKit/Blink scrollbar
pseudo-element transition is ineffective; remove the dead transition from
*::-webkit-scrollbar-thumb and implement the fade via a CSS custom property on
html (e.g., --scroll-thumb-alpha) that the thumb uses like background:
rgba(0,0,0,var(--scroll-thumb-alpha)); add a transition on the html selector (or
html.is-scrolling) to animate --scroll-thumb-alpha between 0 and the target
(0.25 / 0.2 for dark) when toggling html.is-scrolling, and update the existing
html.is-scrolling and html.is-scrolling:is(.dark) / html.is-scrolling .dark
rules to set the appropriate --scroll-thumb-alpha values so the fade works
cross-browser without relying on pseudo-element transitions.
Comment on lines
+1537
to
+1540
| "unlocked": { | ||
| "title": "Achievement Unlocked!", | ||
| "message": "You just earned the {id} badge!" | ||
| } |
Contributor
There was a problem hiding this comment.
{id} in achievement message will show raw badge IDs to users.
"You just earned the {id} badge!" substitutes {id} with the snake_case badge key (e.g., first_blood), producing "You just earned the first_blood badge!". The human-readable names are already defined under dashboard.achievements.badges.{id}.name (e.g., "First Blood").
Consider renaming the parameter and passing the display name at notification creation time:
✏️ Proposed fix
- "message": "You just earned the {id} badge!"
+ "message": "You just earned the {name} badge!"Then at the call site (server action that creates the notification), pass the resolved badge name instead of the badge ID.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "unlocked": { | |
| "title": "Achievement Unlocked!", | |
| "message": "You just earned the {id} badge!" | |
| } | |
| "unlocked": { | |
| "title": "Achievement Unlocked!", | |
| "message": "You just earned the {name} badge!" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/messages/en.json` around lines 1537 - 1540, The notification
template "unlocked" currently uses "{id}" which will show raw badge keys (e.g.,
first_blood); update the template under the "unlocked" key to use a descriptive
parameter like "{badgeName}" (or "{name}") and change the message to "You just
earned the {badgeName} badge!"; then update the notification creation code
(where the server action assembles notifications) to resolve the human-readable
label from dashboard.achievements.badges.{id}.name and pass that string as the
badgeName parameter instead of the badge ID so users see "First Blood" rather
than "first_blood".
Comment on lines
+1540
to
+1542
| "title": "Досягнення розблоковано!", | ||
| "message": "Ви щойно отримали значок {id}!" | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for achievement notification creation
rg -rn "achievement" --include="*.ts" --include="*.tsx" -B 2 -A 8 | grep -A 8 "unlocked\|notification"Repository: DevLoversTeam/devlovers.net
Length of output: 512
🏁 Script executed:
#!/bin/bash
# Search for achievement notification and unlocked pattern in TypeScript files
rg -t ts -t tsx "unlocked|achievement" -B 3 -A 5 | head -100Repository: DevLoversTeam/devlovers.net
Length of output: 98
🏁 Script executed:
#!/bin/bash
# Search for achievement unlocked patterns
rg "unlocked" -B 3 -A 5Repository: DevLoversTeam/devlovers.net
Length of output: 1962
🏁 Script executed:
#!/bin/bash
# Get the full achievement notification creation code from quiz.ts
rg "unlocked" frontend/actions/quiz.ts -B 5 -A 15Repository: DevLoversTeam/devlovers.net
Length of output: 836
🏁 Script executed:
#!/bin/bash
# Search for computeAchievements function and achievement structure
rg "computeAchievements" -B 5 -A 15Repository: DevLoversTeam/devlovers.net
Length of output: 10817
🏁 Script executed:
#!/bin/bash
# Search for dashboard.achievements.badges in language files
rg "dashboard.achievements.badges" -A 2Repository: DevLoversTeam/devlovers.net
Length of output: 362
🏁 Script executed:
#!/bin/bash
# Search for badge name definitions in JSON files
rg "first_blood" frontend/messages/ -B 2 -A 2Repository: DevLoversTeam/devlovers.net
Length of output: 861
Pass the achievement's localized name instead of the internal ID to the notification message.
The current code passes achievement.id directly to the message template:
message: tNotify('message', { id: achievement.id })
Since achievement.id contains internal snake_case identifiers (e.g., first_blood, sharpshooter), users will see unfriendly notifications like "Ви щойно отримали значок first_blood!" instead of the localized names available at dashboard.achievements.badges.{id}.name (e.g., "Перший крок").
Update the message templates in en.json, pl.json, and uk.json:
-"message": "You just earned the {id} badge!"
+"message": "You just earned the {name} badge!"Then update the notification creation in frontend/actions/quiz.ts to pass the localized name:
-message: tNotify('message', { id: achievement.id })
+message: tNotify('message', { name: achievement.name })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "title": "Досягнення розблоковано!", | |
| "message": "Ви щойно отримали значок {id}!" | |
| } | |
| "title": "Досягнення розблоковано!", | |
| "message": "Ви щойно отримали значок {name}!" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/messages/uk.json` around lines 1540 - 1542, The notification uses
the raw internal achievement.id (e.g., first_blood) in the message template;
update the three locale templates (en.json, pl.json, uk.json) to use a {name}
placeholder instead of {id}, and then change the notification creation in
frontend/actions/quiz.ts so tNotify('message', ...) passes the localized label
by resolving dashboard.achievements.badges.{achievement.id}.name (e.g., call the
i18n lookup for dashboard.achievements.badges.{id}.name and pass that string as
the name property) instead of passing achievement.id.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/app/globals.css`:
- Around line 125-129: Add a blank line before the transition declaration inside
the html block to satisfy Stylelint's empty-line-before-declaration rule; locate
the html selector in globals.css and insert a single empty line between the
--scroll-thumb-alpha declaration and the transition: --scroll-thumb-alpha 0.3s
ease; line so the declarations are separated by an empty line.
In `@frontend/components/header/NotificationBell.tsx`:
- Around line 161-165: The button in NotificationBell uses a hardcoded English
aria-label ("Notifications"); update it to use the app's i18n/localization
instead (e.g., obtain the localized string via the existing i18n hook/function
used in this component such as useTranslation or a passed prop) and replace the
static aria-label with that localized value so accessibility text is translated;
locate this in NotificationBell (the button element that toggles setIsOpen) and
use the component's translation helper (e.g., t('notifications') or equivalent)
for the aria-label.
---
Duplicate comments:
In `@frontend/actions/quiz.ts`:
- Around line 234-243: The comment promises stable English fallbacks but the
code passes achievement.id as the notification message, exposing raw IDs if
metadata or NotificationBell localization is unavailable; change the
createNotification call in the loop over newlyEarned to supply a human‑readable
fallback (e.g., use achievement.name or achievement.title, or build a string
like "Achievement unlocked: {achievement.name}") for the message field and keep
metadata.badgeId/icon for localized rendering; also update the surrounding
comment to reflect that message is a readable fallback and not the badge id.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/app/globals.css (1)
119-158:⚠️ Potential issue | 🟡 MinorThe
@property+htmltransition approach correctly resolves the WebKit scrollbar fade issue.Using
@property --scroll-thumb-alphawithinherits: truelets the transition live onhtmlrather than on the scrollbar pseudo-element itself, bypassing the WebKit/Blink restriction on pseudo-element transitions. The pseudo-elements read the interpolatedvar(--scroll-thumb-alpha)value at each paint frame via inheritance — the fade now works cross-browser.One edge case:
html.is-scrolling .dark(lines 155–157) sets--scroll-thumb-alpha: 0.2on the.darkdescendant element, which has notransitiondeclaration. When.is-scrollingis toggled,htmltransitions0 → 0.25, but the.darksubtree immediately jumps to0.2(bypassing the transition onhtml). The fade won't animate in dark mode whenever.darkis a descendant element rather than onhtmlitself. A targeted fix:✨ Proposed fix — add transition to the descendant dark override
html.is-scrolling:is(.dark), html.is-scrolling .dark { --scroll-thumb-alpha: 0.2; + transition: --scroll-thumb-alpha 0.3s ease; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/globals.css` around lines 119 - 158, The descendant dark-mode override (selector html.is-scrolling .dark) sets --scroll-thumb-alpha without a transition, causing an immediate jump when .dark is on a descendant; add the same transition used on html (transition: --scroll-thumb-alpha 0.3s ease) to the selector html.is-scrolling .dark (and/or html .dark if you also use that pattern) so the inherited --scroll-thumb-alpha change animates smoothly in descendant dark mode; leave the `@property` --scroll-thumb-alpha and the scrollbar pseudo-element rules unchanged.
🧹 Nitpick comments (2)
frontend/components/header/NotificationBell.tsx (2)
121-127: MoveKNOWN_TYPES,KnownType, andgetSafeNotificationTypeto module scope.All three are pure constants/utilities with zero dependency on component state, props, or hooks. Placing them inside the function body recreates the array and function closure on every render.
♻️ Proposed refactor
+const KNOWN_TYPES = ['SYSTEM', 'ACHIEVEMENT', 'ARTICLE', 'SHOP'] as const; +type KnownType = (typeof KNOWN_TYPES)[number]; + +const getSafeNotificationType = (type: string): KnownType => + (KNOWN_TYPES as readonly string[]).includes(type) + ? (type as KnownType) + : 'SYSTEM'; + export function NotificationBell() { // ... - const KNOWN_TYPES = ['SYSTEM', 'ACHIEVEMENT', 'ARTICLE', 'SHOP'] as const; - type KnownType = (typeof KNOWN_TYPES)[number]; - - const getSafeNotificationType = (type: string): KnownType => - (KNOWN_TYPES as readonly string[]).includes(type) - ? (type as KnownType) - : 'SYSTEM';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/header/NotificationBell.tsx` around lines 121 - 127, KNOWN_TYPES, KnownType, and getSafeNotificationType are defined inside the component and recreated on every render; move them to module scope by hoisting the const KNOWN_TYPES definition, the KnownType type alias, and the getSafeNotificationType function out of the component body to the top of the file so they are defined once. Update any references in the component to use the hoisted symbols (KNOWN_TYPES, KnownType, getSafeNotificationType) without changing their behavior, and ensure imports/types remain valid after moving them.
38-41:tAch('dashboard.achievements')introduces a cross-namespace coupling.The notification bell (a header component) now depends on the
dashboard.achievementstranslation namespace to resolve badge names. Any structural rename of that namespace silently breaks notification rendering. Consider either introducing a sharednotifications.badgesnamespace or duplicating only the needed badge name keys undernotifications.achievement, so the header has no runtime dependency on the dashboard namespace.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/header/NotificationBell.tsx` around lines 38 - 41, NotificationBell.tsx currently calls useTranslations('dashboard.achievements') via the tAch variable to resolve badge names, creating a fragile cross-namespace dependency; replace that by moving the needed badge keys into the notifications translation space (e.g. notifications.achievement or a new notifications.badges namespace) and update NotificationBell to use useTranslations('notifications.achievement' or 'notifications.badges') instead of tAch, or duplicate only the specific badge keys under notifications.achievement and switch all references from tAch(...) to t(...)/tUnlocked(...) so the header no longer depends on useTranslations('dashboard.achievements').
🤖 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/app/globals.css`:
- Around line 160-174: The Firefox `@supports` block lacks a selector for
descendant dark-mode (add a rule for html.is-scrolling .dark *), so when .dark
is applied to a child element Firefox still uses the light-mode scrollbar color;
inside the existing `@supports` (-moz-appearance: none) { ... } block add a rule
matching html.is-scrolling .dark * and set scrollbar-color to rgba(255, 255,
255, 0.2) transparent to mirror the WebKit behavior and keep dark-mode scrollbar
thumbs visible (reference selectors html.is-scrolling:is(.dark) * and
html.is-scrolling .dark *).
In `@frontend/components/header/NotificationBell.tsx`:
- Around line 113-119: In getNotificationMessage, avoid calling tAch with a
fully dynamic key built from untrusted badgeId; instead compute const key =
`badges.${n.metadata.badgeId}.name` and use tAch.has(key) as a guard: if
tAch.has(key) then call tAch(key) to get badgeName and return
tUnlocked('message', { name: badgeName }), otherwise fall back to returning
n.message (or another safe fallback) so unknown badgeIds don't throw or leak raw
keys.
---
Outside diff comments:
In `@frontend/app/globals.css`:
- Around line 119-158: The descendant dark-mode override (selector
html.is-scrolling .dark) sets --scroll-thumb-alpha without a transition, causing
an immediate jump when .dark is on a descendant; add the same transition used on
html (transition: --scroll-thumb-alpha 0.3s ease) to the selector
html.is-scrolling .dark (and/or html .dark if you also use that pattern) so the
inherited --scroll-thumb-alpha change animates smoothly in descendant dark mode;
leave the `@property` --scroll-thumb-alpha and the scrollbar pseudo-element rules
unchanged.
---
Nitpick comments:
In `@frontend/components/header/NotificationBell.tsx`:
- Around line 121-127: KNOWN_TYPES, KnownType, and getSafeNotificationType are
defined inside the component and recreated on every render; move them to module
scope by hoisting the const KNOWN_TYPES definition, the KnownType type alias,
and the getSafeNotificationType function out of the component body to the top of
the file so they are defined once. Update any references in the component to use
the hoisted symbols (KNOWN_TYPES, KnownType, getSafeNotificationType) without
changing their behavior, and ensure imports/types remain valid after moving
them.
- Around line 38-41: NotificationBell.tsx currently calls
useTranslations('dashboard.achievements') via the tAch variable to resolve badge
names, creating a fragile cross-namespace dependency; replace that by moving the
needed badge keys into the notifications translation space (e.g.
notifications.achievement or a new notifications.badges namespace) and update
NotificationBell to use useTranslations('notifications.achievement' or
'notifications.badges') instead of tAch, or duplicate only the specific badge
keys under notifications.achievement and switch all references from tAch(...) to
t(...)/tUnlocked(...) so the header no longer depends on
useTranslations('dashboard.achievements').
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
frontend/components/header/NotificationBell.tsx (2)
112-124: Minor: stray whitespace-only lines and duplicatereturn n.message.Lines 115 and 117 are whitespace-only (editing artifacts). The two
return n.messagepaths (lines 121 and 123) can also be collapsed to a single one:🧹 Proposed cleanup
const getNotificationMessage = (n: NotificationItem) => { if (n.type === 'ACHIEVEMENT' && n.metadata?.badgeId) { const key = `badges.${n.metadata.badgeId}.name`; - if (tAch.has(key as any)) { - const badgeName = tAch(key as any); return tUnlocked('message', { name: badgeName }); } - return n.message; } return n.message; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/header/NotificationBell.tsx` around lines 112 - 124, The getNotificationMessage function has stray blank lines and duplicate return n.message branches; clean it by removing the empty lines around the key lookup and the tAch check, and collapse the two n.message returns so there is a single final return for the default case. Specifically, edit getNotificationMessage (using types NotificationItem, tAch and tUnlocked) to keep the ACHIEVEMENT handling (compute key, check tAch, return tUnlocked when present) and then have one final return n.message for all other cases.
126-132: HoistKNOWN_TYPES,KnownType, andgetSafeNotificationTypeto module scope.All three are declared inside the component body but close over nothing (no hooks, state, or props). This means
KNOWN_TYPESis reallocated andgetSafeNotificationTypegets a new function identity on every render. The original fix proposal in the past review also placed them at module level.♻️ Proposed refactor — move to module scope (before `NotificationBell`)
+const KNOWN_TYPES = ['SYSTEM', 'ACHIEVEMENT', 'ARTICLE', 'SHOP'] as const; +type KnownType = (typeof KNOWN_TYPES)[number]; + +const getSafeNotificationType = (type: string): KnownType => + (KNOWN_TYPES as readonly string[]).includes(type) + ? (type as KnownType) + : 'SYSTEM'; + export function NotificationBell() { ... - const KNOWN_TYPES = ['SYSTEM', 'ACHIEVEMENT', 'ARTICLE', 'SHOP'] as const; - type KnownType = (typeof KNOWN_TYPES)[number]; - - const getSafeNotificationType = (type: string): KnownType => - (KNOWN_TYPES as readonly string[]).includes(type) - ? (type as KnownType) - : 'SYSTEM';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/header/NotificationBell.tsx` around lines 126 - 132, The constants and helper are defined inside the NotificationBell component causing them to be recreated each render; move KNOWN_TYPES, the KnownType type alias, and the getSafeNotificationType function to module scope (above the NotificationBell declaration) so they are allocated once and no longer re-created per render; update any references inside NotificationBell to use the module-scoped KNOWN_TYPES, KnownType, and getSafeNotificationType without changing their runtime behavior.frontend/app/globals.css (1)
411-413: Optional: align naming convention with.perspective-1000.
.perspective-midrangeuses a semantic name while the adjacent.perspective-1000encodes its pixel value. The semantic name obscures the actual value, making future adjustments non-obvious.♻️ Suggested rename
-.perspective-midrange { - perspective: 800px; -} +.perspective-800 { + perspective: 800px; +}Apply the corresponding rename in
AchievementBadge.tsx.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/globals.css` around lines 411 - 413, Rename the CSS class .perspective-midrange to a value-based name (e.g., .perspective-800) in globals.css and update any usage sites accordingly; specifically change the class name reference inside AchievementBadge.tsx to the new .perspective-800 so the naming convention matches .perspective-1000 and the pixel value is obvious.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/app/globals.css`:
- Around line 411-413: Rename the CSS class .perspective-midrange to a
value-based name (e.g., .perspective-800) in globals.css and update any usage
sites accordingly; specifically change the class name reference inside
AchievementBadge.tsx to the new .perspective-800 so the naming convention
matches .perspective-1000 and the pixel value is obvious.
In `@frontend/components/header/NotificationBell.tsx`:
- Around line 112-124: The getNotificationMessage function has stray blank lines
and duplicate return n.message branches; clean it by removing the empty lines
around the key lookup and the tAch check, and collapse the two n.message returns
so there is a single final return for the default case. Specifically, edit
getNotificationMessage (using types NotificationItem, tAch and tUnlocked) to
keep the ACHIEVEMENT handling (compute key, check tAch, return tUnlocked when
present) and then have one final return n.message for all other cases.
- Around line 126-132: The constants and helper are defined inside the
NotificationBell component causing them to be recreated each render; move
KNOWN_TYPES, the KnownType type alias, and the getSafeNotificationType function
to module scope (above the NotificationBell declaration) so they are allocated
once and no longer re-created per render; update any references inside
NotificationBell to use the module-scoped KNOWN_TYPES, KnownType, and
getSafeNotificationType without changing their runtime behavior.
ViktorSvertoka
approved these changes
Feb 23, 2026
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.
Summary by CodeRabbit
New Features
Improvements
Style Updates
Localization