Merged
Conversation
✅ 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 leaderboard achievement injection (sponsors + stargazers), a feedback POST API with multipart attachment handling and rate limiting, refactors dashboard components to use a shared Changes
Sequence DiagramssequenceDiagram
participant Client as Leaderboard Server (page)
participant DB as Database
participant GitHub as GitHub API
participant ClientRender as Client Renderer
Client->>DB: Fetch users
Client->>GitHub: getAllStargazers()
GitHub-->>Client: Return paginated stargazers
Client->>Client: Build lookup sets (logins, avatarBases)
Client->>Client: Inject achievements per user (sponsor tiers, star_gazer)
Client-->>ClientRender: Serve page with users + achievements
ClientRender->>ClientRender: Render LeaderboardTable + AchievementPips
sequenceDiagram
participant User as End User
participant UI as FeedbackForm
participant API as /api/feedback
participant SMTP as Gmail SMTP
User->>UI: Select files + fill form
UI->>UI: Accumulate files in ref, show list
User->>UI: Submit
UI->>API: POST multipart/form-data (files, fields)
API->>API: Validate fields, rate-limit, check sizes/count
API->>SMTP: Send email with attachments
SMTP-->>API: Acknowledge
API-->>UI: 200 OK
UI->>User: Show success and clear files
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/components/header/MobileMenuContext.tsx (1)
3-14:⚠️ Potential issue | 🟠 MajorUpgrade Next.js to 16.1+ — critical security CVEs patched
The library context confirms this project runs
next@16.0.7. Two vulnerabilities were identified in React Server Components — a high-severity Denial of Service (CVE-2025-55184) and a medium-severity Source Code Exposure (CVE-2025-55183); all Next.js 13.x, 14.x, 15.x and 16.x users should upgrade immediately. Additionally, a critical vulnerability (CVSS 10.0) was identified in the React Server Components protocol that can allow remote code execution — all Next.js 15.x and 16.x users should upgrade immediately. CVE-2025-55182 & CVE-2025-66478 allowed unauthenticated Remote Code Execution via insecure deserialization in the RSC "Flight" protocol; Next.js 16.1 hardens the deserialization logic.Bump
nextfrom16.0.7→16.1.x(or latest stable) before releasing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/header/MobileMenuContext.tsx` around lines 3 - 14, This repo uses next@16.0.7 which is vulnerable—update the Next.js dependency in package.json from 16.0.7 to 16.1.x (or latest stable), run your package manager (npm/yarn/pnpm install), rebuild and run tests; verify the app (including components referencing next/navigation like usePathname and useSearchParams in MobileMenuContext.tsx and any i18n routing hooks such as useRouter) still functions and update any breaking changes per Next.js 16.1 release notes before merging.frontend/components/dashboard/ProfileCard.tsx (1)
95-156:⚠️ Potential issue | 🟡 Minor
<a>/<Link>as direct children of<dl>is invalid HTML.Per the HTML spec,
<dl>only permits<div>,<dt>,<dd>, and script-supporting elements as direct children. Placing<a>elements directly inside<dl>(lines 97, 112, 127) causes a spec violation that some screen readers and validators surface as errors.🛠️ Proposed fix — wrap each group in a `
`<dl className="grid w-full ..."> {/* Attempts */} + <div> <a href="#quiz-results" onClick={scrollTo('quiz-results')} className={statItemBase}> ... - </a> + </a> + </div> {/* Points */} + <div> <a href="#stats" onClick={scrollTo('stats')} className={statItemBase}> ... - </a> + </a> + </div> {/* Global rank */} + <div> <Link href="/leaderboard" className={statItemBase}> ... - </Link> + </Link> + </div> </dl>🤖 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 95 - 156, The issue is that <a> and <Link> elements (the Attempts anchor using scrollTo('quiz-results'), the Points anchor using scrollTo('stats'), and the Link href="/leaderboard") are direct children of the <dl>, which violates the HTML spec; wrap each stat block in a neutral container (e.g., a <div>) so the <dl>'s direct children are dt/dd containers and then place the <a> or <Link> inside that wrapper (or wrap the visual content in the wrapper and move dt/dd inside it) while preserving the statItemBase class and existing click/Link behavior (refer to statItemBase, scrollTo('quiz-results'), scrollTo('stats'), and Link href="/leaderboard" to locate the elements). Ensure semantics: each wrapper contains the dt and dd pair and the interactive anchor remains accessible.
🧹 Nitpick comments (9)
frontend/components/header/MobileMenuContext.tsx (1)
49-55: Consider extracting the close-delay duration as a shared constantThe 310ms here is intentionally 10ms above the
300msCSStransformtransition inAppMobileMenu.tsx. These two values are silently coupled — if the CSS animation changes, this timeout must also change. A singleCLOSE_DELAY_MS = 310constant co-located with (or exported from) this file avoids drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/header/MobileMenuContext.tsx` around lines 49 - 55, Extract the literal 310ms into a shared constant (e.g., CLOSE_DELAY_MS = 310) and use it in the close callback instead of the magic number; update the useCallback close function (which calls setIsAnimating(false), setTimeout(... setIsOpen(false) and sets wasNavigatingRef.current = false) to reference CLOSE_DELAY_MS so the timing is maintained in one place and can be exported/co-located for reuse with AppMobileMenu.tsx.frontend/components/dashboard/ExplainedTermsCard.tsx (2)
18-33:getCachedTerms()andgetHiddenTerms()called twice unnecessarilyEach initializer independently reads the same cache and hidden-terms storage. Even if the current I/O is cheap, this is duplicated work that can be trivially collapsed.
♻️ Proposed refactor — read once, split into two arrays
- const [terms, setTerms] = useState<string[]>(() => { - if (typeof window === 'undefined') return []; - const cached = getCachedTerms(); - const hidden = getHiddenTerms(); - return sortTermsByOrder( - cached.filter(term => !hidden.has(term.toLowerCase().trim())) - ); - }); - const [hiddenTerms, setHiddenTerms] = useState<string[]>(() => { - if (typeof window === 'undefined') return []; - const cached = getCachedTerms(); - const hidden = getHiddenTerms(); - return sortTermsByOrder( - cached.filter(term => hidden.has(term.toLowerCase().trim())) - ); - }); + const [terms, setTerms] = useState<string[]>([]); + const [hiddenTerms, setHiddenTerms] = useState<string[]>([]);Then (combined with the
useEffectfix above):+ useEffect(() => { + const cached = getCachedTerms(); + const hidden = getHiddenTerms(); + setTerms(sortTermsByOrder(cached.filter(term => !hidden.has(term.toLowerCase().trim())))); + setHiddenTerms(sortTermsByOrder(cached.filter(term => hidden.has(term.toLowerCase().trim())))); + }, []);🤖 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 18 - 33, The two initializers call getCachedTerms() and getHiddenTerms() twice; instead read them once inside a single lazy initializer, compute cached = getCachedTerms() and hidden = getHiddenTerms(), then split cached into visible and hidden arrays using sortTermsByOrder and hidden.has(term.toLowerCase().trim()), and return those arrays to initialize terms/setTerms and hiddenTerms/setHiddenTerms so getCachedTerms/getHiddenTerms are only invoked once.
1-1: Upgrade Next.js to 16.1 — critical CVEs in 16.0.xTwo vulnerabilities in React Server Components are patched in 16.1: a high-severity Denial of Service (CVE-2025-55184) and a medium-severity Source Code Exposure (CVE-2025-55183). All Next.js 13.x, 14.x, 15.x, and 16.x users should upgrade immediately. Additionally, CVE-2025-55182 and CVE-2025-66478 allowed unauthenticated Remote Code Execution via insecure deserialization in the RSC "Flight" protocol; Next.js 16.1 hardens the deserialization logic.
The project currently pins
next@16.0.7, which is within the affected version range. Updatepackage.jsontonext@^16.1.0or the latest 16.x patch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/dashboard/ExplainedTermsCard.tsx` at line 1, The project is pinned to next@16.0.7 which is vulnerable; update the "next" dependency in package.json to a safe version (e.g., "next": "^16.1.0" or the latest 16.x patch), then reinstall dependencies and update lockfile (npm/yarn/pnpm) and run the test/start scripts to ensure no regressions; also scan for any custom Next.js API or RSC deserialization code that might need adjustments after the upgrade.frontend/app/api/feedback/route.ts (1)
59-62: Reuse the existingmailersingleton fromfrontend/lib/email/transporter.ts.A new
nodemailer.createTransport()is created on every request when an identical singleton already exists in the codebase. This adds unnecessary setup overhead per request and duplicates configuration.♻️ Proposed refactor
-import nodemailer from 'nodemailer'; +import { mailer } from '@/lib/email/transporter'; ... - const mailer = nodemailer.createTransport({ - service: 'gmail', - auth: { user: gmailUser, pass: gmailPass }, - }); - try { await mailer.sendMail({Note: With this change the env-var guard at lines 17–24 can be removed or relocated to
transporter.ts, since credentials are already validated there at startup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/feedback/route.ts` around lines 59 - 62, Replace the per-request creation of the mailer in route.ts (currently using nodemailer.createTransport) with the shared singleton exported from frontend/lib/email/transporter.ts: stop calling nodemailer.createTransport inside the handler and instead import the existing mailer export and use it; also remove or relocate the redundant env-var guard from this file since transporter.ts already validates credentials at startup.frontend/app/[locale]/dashboard/page.tsx (1)
1-4: Next.js 16.x has critical unpatched CVEs — upgrade to 16.1.x.Next.js 16.1 was released in December 2025 and patches a high-severity Denial of Service (CVE-2025-55184), a medium-severity Source Code Exposure (CVE-2025-55183), and a critical CVSS 10.0 remote-code-execution vulnerability (CVE-2025-66478) in React Server Components — all Next.js 16.x users are advised to upgrade immediately.
The project is pinned to
16.0.7; upgrade to at least16.1.xvianpm install next@latest.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`[locale]/dashboard/page.tsx around lines 1 - 4, Your Next.js dependency is pinned to 16.0.7 and must be upgraded to at least 16.1.x to patch critical CVEs; update package.json to bump "next" to ^16.1.0 (or run npm install next@latest), update the lockfile, then rebuild and run tests/CI to verify nothing breaks (check code using getTranslations or PostAuthQuizSync imports for any Next 16.1 breaking-change impacts), and commit the updated package.json and lockfile.frontend/components/leaderboard/LeaderboardPodium.tsx (1)
128-150: Replace the IIFE with a named helper component or a pre-computed variable.The
(() => { ... })()inside JSX is syntactically valid but unusual, hurting readability. Since the pattern is already used in another podium user in each render, extracting it keeps the JSX tree clean.♻️ Suggested refactor
+function SponsorBadge({ user, tBadges }: { user: User; tBadges: ReturnType<typeof useTranslations> }) { + const sponsorAch = getSponsorAchievement(user); + if (!sponsorAch) return null; + const tierStyle = SPONSOR_TIER_STYLE[sponsorAch.id] ?? SPONSOR_TIER_STYLE['silver_patron']; + return ( + <a + href="https://github.com/sponsors/DevLoversTeam" + target="_blank" + rel="noopener noreferrer" + aria-label={tBadges(`${sponsorAch.id}.name`)} + className={cn( + 'mt-0.5 inline-flex items-center gap-1 rounded-full px-2 py-0.5 text-[10px] font-bold transition-colors', + tierStyle + )} + > + <Heart className="h-2.5 w-2.5 fill-current" aria-hidden="true" /> + <span className="hidden md:inline">{tBadges(`${sponsorAch.id}.name`)}</span> + </a> + ); +} // In LeaderboardPodium render: - {(() => { - const sponsorAch = getSponsorAchievement(user); - if (!sponsorAch) return null; - ... - })()} + <SponsorBadge user={user} tBadges={tBadges} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/leaderboard/LeaderboardPodium.tsx` around lines 128 - 150, The JSX contains an IIFE rendering the sponsor badge; replace it with a clearer approach by extracting the logic into a small helper component or a precomputed variable: call getSponsorAchievement(user) into a const sponsorAch and if absent return null, compute tierStyle from SPONSOR_TIER_STYLE (fallback 'silver_patron'), and render the anchor with tBadges(`${sponsorAch.id}.name`) and Heart; alternatively create a SponsorBadge functional component that accepts sponsorAch and uses tBadges, SPONSOR_TIER_STYLE and Heart, then use <SponsorBadge sponsor={sponsorAch}/> in the JSX to remove the IIFE and improve readability.frontend/app/[locale]/leaderboard/page.tsx (2)
53-53:tierColormay not matchTIER_ACHIEVEMENTkeys — fails silently.If
matchedSponsor.tierColoris a value outside'gold' | 'silver' | 'bronze'(e.g. a new tier added upstream), the lookup returnsundefinedand the achievement is silently skipped. This is safe due to theif (def && ...)guard, but consider logging a warning so new tiers aren't missed indefinitely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`[locale]/leaderboard/page.tsx at line 53, The lookup of TIER_ACHIEVEMENT using matchedSponsor.tierColor can return undefined for unknown tiers (currently assigned to achievementId), so update the code around the achievementId assignment to detect when achievementId is undefined and emit a warning log (including the matchedSponsor.tierColor and matchedSponsor.id or name) so new tiers are visible; keep the existing guard logic that skips undefined values but add the warning via the same logger used in this file (or console.warn if none), referencing matchedSponsor.tierColor, TIER_ACHIEVEMENT, and achievementId to locate where to insert the check and log.
52-61: Manual field cherry-picking is fragile; prefer spreading the definition.Achievement objects are manually constructed by copying
id,icon,gradient,glow. IfAchievementgains a new field, these injection sites will silently produce incomplete objects. Spreadingdefavoids drift.Proposed fix (apply to both injection sites)
if (def && !achievements.some(a => a.id === achievementId)) { - achievements = [ - { id: def.id, icon: def.icon, gradient: def.gradient, glow: def.glow }, - ...achievements, - ]; + achievements = [{ ...def }, ...achievements]; }if (hasStarred && !achievements.some(a => a.id === 'star_gazer')) { const def = ACHIEVEMENTS.find(a => a.id === 'star_gazer'); if (def) { - achievements = [ - { id: def.id, icon: def.icon, gradient: def.gradient, glow: def.glow }, - ...achievements, - ]; + achievements = [{ ...def }, ...achievements]; } }Also applies to: 71-79
🤖 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 52 - 61, The code constructs new achievement objects by manually copying fields (id, icon, gradient, glow) from def, which is fragile; update the injection sites that handle matchedSponsor and the second injection (the similar block at lines 71-79) to spread the full def object instead of cherry-picking fields: when you detect def (from ACHIEVEMENTS.find) and decide to prepend it to achievements, use { ...def } (or simply def if immutability is guaranteed) as the inserted item so any future fields on Achievement are preserved; ensure you still avoid duplicates by checking achievements.some(a => a.id === def.id) and keep the existing logic around TIER_ACHIEVEMENT and matchedSponsor intact.frontend/components/leaderboard/LeaderboardTable.tsx (1)
120-125: Duplicated colgroup definition — keep column widths in sync via a shared constant.The
<colgroup>here (lines 120-125) duplicates lines 58-62 from the main table. If one is updated without the other, columns will visually misalign.Proposed approach
Extract the colgroup into a shared component or constant:
const COLGROUP = ( <colgroup> <col className="w-[15%] sm:w-[12%]" /> <col /> <col className="w-[25%] sm:w-[20%]" /> </colgroup> );Then use
{COLGROUP}in both tables.🤖 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 120 - 125, Extract the duplicated <colgroup> in the LeaderboardTable component into a single shared constant or small component (e.g., COLGROUP) and replace both inline <colgroup> blocks with that constant; update the LeaderboardTable.tsx component to define const COLGROUP = ( ... ) near the top of the file and use {COLGROUP} in both table renderings so the column widths (w-[15%] sm:w-[12%], default middle, w-[25%] sm:w-[20%]) stay in sync.
🤖 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/api/feedback/route.ts`:
- Around line 33-36: The email value retrieved via formData.get('email') is only
truthy-checked before being interpolated into the replyTo header (risk of header
injection); validate and sanitize it: apply a strict email-format check (e.g.,
RFC-like regex or existing validator) to the value from formData.get('email')
and reject/omit invalid addresses, then strip CRLF/control characters (same
sanitization as safeNameForHeader) before using it in the replyTo construction;
ensure replyTo uses the sanitized email (or omits replyTo) rather than the raw
email string and update the logic around replyTo creation to reference the
sanitized variable.
- Around line 16-87: The POST handler in frontend/app/api/feedback/route.ts
currently allows unlimited unauthenticated requests and can be abused to spam
the Gmail account; add IP-based rate limiting at the start of the POST flow
(before parsing formData or sending mail) by integrating a rate limiter (e.g.,
`@upstash/ratelimit` with Vercel KV or an Edge middleware) to throttle requests
per IP and return an HTTP 429 when the limit is exceeded; update the POST
function to check the limiter (use the request's IP or x-forwarded-for) and
short-circuit with NextResponse.json({ success: false }, { status: 429 }) on
limit hit, keeping the existing error handling for other cases and ensuring
limits are configurable (MAX requests/time) and logged for monitoring.
- Line 72: The subject uses escapeHtml(category) which creates HTML entities in
plain-text email subjects; change this to a plain-text sanitizer and optional
validation: remove escapeHtml and instead sanitize the category for CR/LF and
other control characters (strip or replace "\r" and "\n"), and/or validate
category against the allowed set before building subject in the mail payload
(look for the variable/field subject and the category input and the escapeHtml
call). Ensure the mail subject uses the cleaned plainTextCategory when
constructing the string (`subject`) and reserve escapeHtml only for the HTML
body.
In `@frontend/components/dashboard/ActivityHeatmapCard.tsx`:
- Around line 275-276: The Tailwind utilities h-5.5, min-w-17.5, min-w-27.5 and
z-9999 used in the ActivityHeatmapCard span (and other occurrences in
ActivityHeatmapCard.tsx) are not defined in your Tailwind config; update those
class strings to use Tailwind arbitrary values (e.g. h-[1.375rem],
min-w-[4.375rem], min-w-[6.875rem], z-[9999]) or add corresponding custom tokens
in your Tailwind config, and replace each occurrence (the span with Flame and
the other repeated locations) accordingly so the styles actually apply.
In `@frontend/components/dashboard/ExplainedTermsCard.tsx`:
- Around line 18-33: The current lazy initializers for terms and hiddenTerms in
ExplainedTermsCard cause hydration mismatches; change both useState initializers
to plain empty arrays and move the localStorage reads into a single useEffect
that runs only on the client: call getCachedTerms() and getHiddenTerms() once
inside that effect, compute the visible and hidden lists using sortTermsByOrder
and the same filtering logic, then call setTerms(...) and setHiddenTerms(...);
remove the typeof window === 'undefined' guards from the initializers so SSR
renders empty arrays and the effect hydrates client state.
In `@frontend/components/dashboard/FeedbackForm.tsx`:
- Around line 248-257: Prevent the UI from letting users accumulate more than
MAX_FILES by enforcing the limit when merging newFiles into
accumulatedFilesRef.current: in the merge logic that currently iterates newFiles
and pushes unique files, stop adding once accumulatedFilesRef.current.length
reaches MAX_FILES, update setAttachmentNames from the capped merged list, and
reset fileInputRef.current.value as before; additionally surface a client-side
validation message (e.g., set an error state or call an existing showError
method) when the user attempts to add files beyond MAX_FILES so they get
immediate, specific feedback instead of a generic server error.
- Around line 279-290: The aria-label for the remove button in FeedbackForm.tsx
is hardcoded ("Remove ${name}") and must be internationalized; replace the
template string with a localized string from next-intl (e.g., use formatMessage
/ t or the component's intl hook) passing name as a variable when rendering the
button in the map where removeFile is used, and add the corresponding key (e.g.,
"dashboard.feedback.removeFile": "Remove {name}") to en.json, pl.json and
uk.json under dashboard.feedback so screen readers show the translated label.
In `@frontend/components/header/AppMobileMenu.tsx`:
- Around line 121-134: The current useEffect that toggles
document.documentElement.style.overflowY when AppMobileMenu's open changes won't
stop background scrolling on iOS Safari; update the useEffect (the hook watching
open in AppMobileMenu) to also prevent touchmove propagation when open by
adding/removing a touchmove listener on document (or integrate body-scroll-lock
and call disableBodyScroll/enableBodyScroll on the nav element) so touches
outside the nav don't scroll the background while preserving overscroll-contain
inside the nav; ensure you restore the previous overflowY and remove the
touchmove listener (or call enableBodyScroll) in the cleanup to avoid leaks.
In `@frontend/components/leaderboard/AchievementPips.tsx`:
- Around line 100-112: The tooltip position is computed once in handleMouseEnter
using getBoundingClientRect and never updated, so it can stay misaligned on
scroll/resize and overflow the viewport; update the implementation to (1)
recompute or hide the tooltip on window scroll and resize by adding listeners
that call the same positioning logic used in handleMouseEnter (or clear tooltip
via handleMouseLeave) and remove those listeners on mouse leave, and (2) clamp
the computed x coordinate against window.innerWidth (and optionally y against
window.innerHeight) before calling setTooltip so the tooltip cannot overflow
horizontally; reference handleMouseEnter, handleMouseLeave, setTooltip and the
tooltip state when adding these listeners and clamp logic.
In `@frontend/components/leaderboard/LeaderboardTable.tsx`:
- Around line 106-138: The current implementation maps contextRows to individual
<table> elements causing accessibility and alignment issues; change this to
render a single table wrapper for the entire context section (reuse the same
colgroup as the main table) and render each context entry as a <tr> rather than
a separate <table>; apply the highlight/glow for the current user by setting the
box-shadow/outline class on the <tr> (the element produced by the TableRow
render) when isMe is true (keep the isMe logic and pass isCurrentUser/inContext
to TableRow so the row can receive the visual highlight) to preserve semantics
and consistent column widths.
In `@frontend/lib/github-stars.ts`:
- Around line 54-74: getAllStargazers currently swallows failures by breaking
the pagination loop on !res.ok and in the catch block; update the loop in
getAllStargazers to log failures instead of silently breaking: when res.ok is
false, call console.warn (including URL, res.status and res.statusText) before
breaking, and in the catch handler call console.error with the caught error and
the URL, then break; this mirrors the logging style used in checkHasStarredRepo
and ensures transient GitHub API errors are visible while preserving existing
pagination break behavior.
---
Outside diff comments:
In `@frontend/components/dashboard/ProfileCard.tsx`:
- Around line 95-156: The issue is that <a> and <Link> elements (the Attempts
anchor using scrollTo('quiz-results'), the Points anchor using
scrollTo('stats'), and the Link href="/leaderboard") are direct children of the
<dl>, which violates the HTML spec; wrap each stat block in a neutral container
(e.g., a <div>) so the <dl>'s direct children are dt/dd containers and then
place the <a> or <Link> inside that wrapper (or wrap the visual content in the
wrapper and move dt/dd inside it) while preserving the statItemBase class and
existing click/Link behavior (refer to statItemBase, scrollTo('quiz-results'),
scrollTo('stats'), and Link href="/leaderboard" to locate the elements). Ensure
semantics: each wrapper contains the dt and dd pair and the interactive anchor
remains accessible.
In `@frontend/components/header/MobileMenuContext.tsx`:
- Around line 3-14: This repo uses next@16.0.7 which is vulnerable—update the
Next.js dependency in package.json from 16.0.7 to 16.1.x (or latest stable), run
your package manager (npm/yarn/pnpm install), rebuild and run tests; verify the
app (including components referencing next/navigation like usePathname and
useSearchParams in MobileMenuContext.tsx and any i18n routing hooks such as
useRouter) still functions and update any breaking changes per Next.js 16.1
release notes before merging.
---
Nitpick comments:
In `@frontend/app/`[locale]/dashboard/page.tsx:
- Around line 1-4: Your Next.js dependency is pinned to 16.0.7 and must be
upgraded to at least 16.1.x to patch critical CVEs; update package.json to bump
"next" to ^16.1.0 (or run npm install next@latest), update the lockfile, then
rebuild and run tests/CI to verify nothing breaks (check code using
getTranslations or PostAuthQuizSync imports for any Next 16.1 breaking-change
impacts), and commit the updated package.json and lockfile.
In `@frontend/app/`[locale]/leaderboard/page.tsx:
- Line 53: The lookup of TIER_ACHIEVEMENT using matchedSponsor.tierColor can
return undefined for unknown tiers (currently assigned to achievementId), so
update the code around the achievementId assignment to detect when achievementId
is undefined and emit a warning log (including the matchedSponsor.tierColor and
matchedSponsor.id or name) so new tiers are visible; keep the existing guard
logic that skips undefined values but add the warning via the same logger used
in this file (or console.warn if none), referencing matchedSponsor.tierColor,
TIER_ACHIEVEMENT, and achievementId to locate where to insert the check and log.
- Around line 52-61: The code constructs new achievement objects by manually
copying fields (id, icon, gradient, glow) from def, which is fragile; update the
injection sites that handle matchedSponsor and the second injection (the similar
block at lines 71-79) to spread the full def object instead of cherry-picking
fields: when you detect def (from ACHIEVEMENTS.find) and decide to prepend it to
achievements, use { ...def } (or simply def if immutability is guaranteed) as
the inserted item so any future fields on Achievement are preserved; ensure you
still avoid duplicates by checking achievements.some(a => a.id === def.id) and
keep the existing logic around TIER_ACHIEVEMENT and matchedSponsor intact.
In `@frontend/app/api/feedback/route.ts`:
- Around line 59-62: Replace the per-request creation of the mailer in route.ts
(currently using nodemailer.createTransport) with the shared singleton exported
from frontend/lib/email/transporter.ts: stop calling nodemailer.createTransport
inside the handler and instead import the existing mailer export and use it;
also remove or relocate the redundant env-var guard from this file since
transporter.ts already validates credentials at startup.
In `@frontend/components/dashboard/ExplainedTermsCard.tsx`:
- Around line 18-33: The two initializers call getCachedTerms() and
getHiddenTerms() twice; instead read them once inside a single lazy initializer,
compute cached = getCachedTerms() and hidden = getHiddenTerms(), then split
cached into visible and hidden arrays using sortTermsByOrder and
hidden.has(term.toLowerCase().trim()), and return those arrays to initialize
terms/setTerms and hiddenTerms/setHiddenTerms so getCachedTerms/getHiddenTerms
are only invoked once.
- Line 1: The project is pinned to next@16.0.7 which is vulnerable; update the
"next" dependency in package.json to a safe version (e.g., "next": "^16.1.0" or
the latest 16.x patch), then reinstall dependencies and update lockfile
(npm/yarn/pnpm) and run the test/start scripts to ensure no regressions; also
scan for any custom Next.js API or RSC deserialization code that might need
adjustments after the upgrade.
In `@frontend/components/header/MobileMenuContext.tsx`:
- Around line 49-55: Extract the literal 310ms into a shared constant (e.g.,
CLOSE_DELAY_MS = 310) and use it in the close callback instead of the magic
number; update the useCallback close function (which calls
setIsAnimating(false), setTimeout(... setIsOpen(false) and sets
wasNavigatingRef.current = false) to reference CLOSE_DELAY_MS so the timing is
maintained in one place and can be exported/co-located for reuse with
AppMobileMenu.tsx.
In `@frontend/components/leaderboard/LeaderboardPodium.tsx`:
- Around line 128-150: The JSX contains an IIFE rendering the sponsor badge;
replace it with a clearer approach by extracting the logic into a small helper
component or a precomputed variable: call getSponsorAchievement(user) into a
const sponsorAch and if absent return null, compute tierStyle from
SPONSOR_TIER_STYLE (fallback 'silver_patron'), and render the anchor with
tBadges(`${sponsorAch.id}.name`) and Heart; alternatively create a SponsorBadge
functional component that accepts sponsorAch and uses tBadges,
SPONSOR_TIER_STYLE and Heart, then use <SponsorBadge sponsor={sponsorAch}/> in
the JSX to remove the IIFE and improve readability.
In `@frontend/components/leaderboard/LeaderboardTable.tsx`:
- Around line 120-125: Extract the duplicated <colgroup> in the LeaderboardTable
component into a single shared constant or small component (e.g., COLGROUP) and
replace both inline <colgroup> blocks with that constant; update the
LeaderboardTable.tsx component to define const COLGROUP = ( ... ) near the top
of the file and use {COLGROUP} in both table renderings so the column widths
(w-[15%] sm:w-[12%], default middle, w-[25%] sm:w-[20%]) stay in sync.
Comment on lines
+275
to
276
| <span className="hidden sm:inline-flex items-center gap-1.5 rounded-full bg-orange-500/10 px-2 py-0.5 text-[10px] sm:px-2.5 sm:py-1 sm:text-[11px] h-5.5 sm:h-auto font-bold tracking-wide text-orange-600 dark:bg-orange-500/20 dark:text-orange-400 border border-orange-500/20"> | ||
| <Flame className="h-2.5 w-2.5 sm:h-3 sm:w-3" /> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and inspect Tailwind config for custom spacing/zIndex tokens
configs=$(fd -t f 'tailwind\.config\.(js|ts|cjs|mjs)' .)
if [ -z "$configs" ]; then
echo "No tailwind config found."
exit 0
fi
for f in $configs; do
echo "== File: $f =="
cat -n "$f"
echo ""
doneRepository: DevLoversTeam/devlovers.net
Length of output: 96
🏁 Script executed:
#!/bin/bash
# Verify the specific classes are used in ActivityHeatmapCard.tsx
file="frontend/components/dashboard/ActivityHeatmapCard.tsx"
if [ -f "$file" ]; then
echo "=== Lines 275-276 ==="
sed -n '275,276p' "$file"
echo ""
echo "=== Lines 291-292 ==="
sed -n '291,292p' "$file"
echo ""
echo "=== Lines 349-350 ==="
sed -n '349,350p' "$file"
echo ""
echo "=== Lines 455-456 ==="
sed -n '455,456p' "$file"
echo ""
echo "=== Lines 462-463 ==="
sed -n '462,463p' "$file"
echo ""
echo "=== All occurrences of h-5.5, min-w-17.5, min-w-27.5, z-9999 ==="
rg "(h-5\.5|min-w-17\.5|min-w-27\.5|z-9999)" "$file" -n
else
echo "File not found: $file"
fiRepository: DevLoversTeam/devlovers.net
Length of output: 1788
🏁 Script executed:
#!/bin/bash
# Check for Tailwind config in common alternative locations
echo "=== Checking next.config.js ==="
[ -f next.config.js ] && cat -n next.config.js || echo "Not found"
echo -e "\n=== Checking postcss.config.js ==="
[ -f postcss.config.js ] && cat -n postcss.config.js || echo "Not found"
echo -e "\n=== Checking for CSS/SCSS files with custom token definitions ==="
fd -t f '\.(css|scss)$' . | head -20
echo -e "\n=== Checking for tailwind directives in CSS files ==="
rg "@apply|@tailwind|theme\(" --type css --type scss | head -30
echo -e "\n=== Check package.json for Tailwind setup ==="
cat package.json | jq '.devDependencies, .dependencies' 2>/dev/null | grep -i tailwind || echo "No Tailwind in dependencies"Repository: DevLoversTeam/devlovers.net
Length of output: 473
🏁 Script executed:
#!/bin/bash
# Search for theme or spacing definitions that might define these custom values
rg "(spacing|zIndex|extend)" --type js --type ts | grep -E "(5\.5|17\.5|27\.5|9999)" | head -20Repository: DevLoversTeam/devlovers.net
Length of output: 53
🏁 Script executed:
#!/bin/bash
# Check globals.css
echo "=== frontend/app/globals.css ==="
cat -n frontend/app/globals.css | head -50
echo -e "\n=== Find package.json ==="
fd -t f 'package\.json' . | head -5
echo -e "\n=== Project structure ==="
find . -maxdepth 2 -type f -name '*.json' | grep -E '(package|tsconfig|tailwind)' | head -10Repository: DevLoversTeam/devlovers.net
Length of output: 2417
🏁 Script executed:
#!/bin/bash
# Check if Tailwind classes would be recognized even without explicit config
# Look for any tailwind setup in the project
echo "=== Search for tailwind references in all files ==="
rg -i "tailwind" --type ts --type tsx --type js --type json | head -20
echo -e "\n=== Check actual dimensions - is 5.5 even a valid Tailwind default? ==="
# 5.5 in spacing would be 22px (5.5 * 4px), but Tailwind default spacing goes 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10...
# 5.5 is NOT in the default scale
echo "Tailwind default spacing scale: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10"
echo "5.5 is not in default scale - would need custom config"Repository: DevLoversTeam/devlovers.net
Length of output: 360
🏁 Script executed:
#!/bin/bash
# Check globals.css
echo "=== frontend/app/globals.css ==="
cat -n frontend/app/globals.css
echo -e "\n=== Find package.json ==="
fd -t f 'package\.json' .
echo -e "\n=== Check tailwind references ==="
rg -i "tailwind" -t ts -t tsx -t js -t json | head -30Repository: DevLoversTeam/devlovers.net
Length of output: 17919
🏁 Script executed:
#!/bin/bash
# Check if Tailwind is setup as dependency elsewhere
echo "=== Check all package.json files ==="
find . -name 'package.json' -type f -exec sh -c 'echo "File: $1"; cat "$1" | jq ".devDependencies | keys[] | select(. | contains(\"tailwind\"))" 2>/dev/null || echo "Cannot parse or no tailwind"' _ {} \;Repository: DevLoversTeam/devlovers.net
Length of output: 232
🏁 Script executed:
#!/bin/bash
# Double-check: verify if any custom spacing values are defined in globals.css
echo "=== Search for spacing or spacing-related definitions in globals.css ==="
grep -n "spacing\|--spacing\|--width\|--height" frontend/app/globals.css || echo "No spacing definitions found"
echo -e "\n=== Search for zIndex definitions ==="
grep -n "zIndex\|--z-\|z-index" frontend/app/globals.css || echo "No zIndex definitions found"
echo -e "\n=== Confirm Tailwind v4 syntax in use ==="
head -10 frontend/app/globals.cssRepository: DevLoversTeam/devlovers.net
Length of output: 596
Replace non-existent Tailwind utility classes with arbitrary values or define custom tokens.
Classes h-5.5, min-w-17.5, min-w-27.5, and z-9999 are not defined in the Tailwind configuration (checked in frontend/app/globals.css). These styles will not apply without custom token definitions. Use arbitrary values as a fallback.
🔧 Suggested fix using arbitrary values
- <span className="hidden sm:inline-flex items-center gap-1.5 rounded-full bg-orange-500/10 px-2 py-0.5 text-[10px] sm:px-2.5 sm:py-1 sm:text-[11px] h-5.5 sm:h-auto font-bold tracking-wide text-orange-600 dark:bg-orange-500/20 dark:text-orange-400 border border-orange-500/20">
+ <span className="hidden sm:inline-flex items-center gap-1.5 rounded-full bg-orange-500/10 px-2 py-0.5 text-[10px] sm:px-2.5 sm:py-1 sm:text-[11px] h-[22px] sm:h-auto font-bold tracking-wide text-orange-600 dark:bg-orange-500/20 dark:text-orange-400 border border-orange-500/20">
...
- <span className="hidden sm:block min-w-17.5 text-center">
+ <span className="hidden sm:block min-w-[70px] text-center">
...
- className="fixed z-9999 pointer-events-none"
+ className="fixed z-[9999] pointer-events-none"
...
- <div className="bg-gray-900 dark:bg-white rounded-xl px-3 py-2 shadow-xl shadow-black/30 text-center min-w-27.5">
+ <div className="bg-gray-900 dark:bg-white rounded-xl px-3 py-2 shadow-xl shadow-black/30 text-center min-w-[110px]">Also applies to: lines 291–292, 349–350, 455–456, 462–463
🤖 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 275 -
276, The Tailwind utilities h-5.5, min-w-17.5, min-w-27.5 and z-9999 used in the
ActivityHeatmapCard span (and other occurrences in ActivityHeatmapCard.tsx) are
not defined in your Tailwind config; update those class strings to use Tailwind
arbitrary values (e.g. h-[1.375rem], min-w-[4.375rem], min-w-[6.875rem],
z-[9999]) or add corresponding custom tokens in your Tailwind config, and
replace each occurrence (the span with Flame and the other repeated locations)
accordingly so the styles actually apply.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/components/dashboard/FeedbackForm.tsx (2)
166-173:⚠️ Potential issue | 🟡 Minor
readOnlyemail field silently breaks submission whenuserEmailis null.When
userEmailisnullorundefined, the email input receives an emptydefaultValueand is read-only — the user has no way to fill it in. The server requires a non-empty, valid email and will return a 400, which the UI surfaces only as the generic "something went wrong" message with no actionable recovery path.Either conditionally allow editing when
userEmailis absent, or show a visible prompt explaining that an email must be set before submitting feedback:🛠️ Proposed fix
- readOnly - className={`${inputStyles} cursor-default select-none`} + readOnly={!!userEmail} + className={userEmail ? `${inputStyles} cursor-default select-none` : inputStyles}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/dashboard/FeedbackForm.tsx` around lines 166 - 173, In FeedbackForm, the email input (name="email", defaultValue={userEmail ?? ''}) is currently always readOnly which prevents users from entering an address when userEmail is null; change the logic so the input is editable when userEmail is null/undefined (e.g., set readOnly={!userEmail} or render an editable input when !userEmail), ensure the field is required/validated before submission, and/or display a clear inline prompt/error near the email input telling users they must provide an email before submitting so the server won't return a 400.
108-119:⚠️ Potential issue | 🟡 Minor
tooLargeflag from the 413 response is silently ignored — user sees a generic error.The server returns
{ success: false, tooLarge: true }with status 413 when a file exceeds 5 MB, but the client only checksresult.success, so a file-size rejection is indistinguishable from any other failure. Users have no way to know they need to remove a large file.🛠️ Proposed fix
+ if (!res.ok && res.status === 413) { + const r = await res.json().catch(() => ({})); + if (r.tooLarge) { + setStatus('error'); // or a dedicated 'tooLarge' status + message + // surface t('tooLargeFile') or similar translation key + setLoading(false); + return; + } + } const result = await res.json();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/dashboard/FeedbackForm.tsx` around lines 108 - 119, In FeedbackForm.tsx update the submit response handling (where result is read and setStatus is called) to detect the server file-size rejection (check result.tooLarge === true or res.status === 413) and set a distinct client state (e.g., setStatus('file-too-large') or setStatus('tooLarge')) so the UI can show a specific message; keep the existing success branch (setStatus('success'), clear accumulatedFilesRef.current and setAttachmentNames([]), reset the form and successTimerRef) and only run the generic setStatus('error') for other failures.
🧹 Nitpick comments (1)
frontend/components/header/AppMobileMenu.tsx (1)
121-144: The iOS Safari scroll-lock concern has been addressed — one optional cleanup.The
preventTouchMovehandler callsdocument.getElementById('app-mobile-nav')on everytouchmoveevent fired during a scroll gesture (can be 60+ calls/sec). The nav element is guaranteed to be in the DOM when this effect runs (React flushes DOM before effects), so the reference can be captured once.♻️ Hoist the nav query outside the handler
- const preventTouchMove = (e: TouchEvent) => { - const nav = document.getElementById('app-mobile-nav'); - if (nav && nav.contains(e.target as Node)) return; - e.preventDefault(); - }; + const nav = document.getElementById('app-mobile-nav'); + const preventTouchMove = (e: TouchEvent) => { + if (nav && nav.contains(e.target as Node)) return; + e.preventDefault(); + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/header/AppMobileMenu.tsx` around lines 121 - 144, The touchmove handler repeatedly calls document.getElementById('app-mobile-nav'); hoist that query out of the preventTouchMove closure inside the useEffect so you capture the nav element once when the effect runs (e.g., const nav = document.getElementById('app-mobile-nav')), then use that captured nav in the preventTouchMove handler to check nav.contains(e.target as Node); keep the same preventTouchMove function reference for addEventListener/removeEventListener and preserve restoring document.documentElement.style.overflowY in the cleanup; reference symbols: useEffect, preventTouchMove, and the 'app-mobile-nav' id.
🤖 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/api/feedback/route.ts`:
- Around line 71-79: In the loop that builds attachments (for (const f of files)
{ … }) sanitize and validate incoming file metadata before pushing to
attachments: strip CRLF (\r, \n), null bytes and other control characters from
f.name (and fallback to a generated safe filename if the result is empty) and
validate f.type against an allowlist or a strict MIME token/type pattern (e.g.
/^[a-z0-9.+-]+\/[a-z0-9.+-]+$/i); if f.type is invalid, replace it with
"application/octet-stream" or omit the media-type, then push the sanitized
values into attachments instead of raw f.name/f.type to prevent MIME header
injection when nodemailer constructs headers.
In `@frontend/components/dashboard/FeedbackForm.tsx`:
- Around line 110-116: On successful submission (inside the block handling
result.success in FeedbackForm, where setStatus('success'), setCategory(''),
setAttachmentNames([]), accumulatedFilesRef.current = [], and form reset are
performed), ensure the file-limit error state is cleared by calling
setFileLimitError(false) so any previous fileLimitError banner does not reappear
when the form returns to 'idle'; add this call alongside the other cleanup
actions (and consider also clearing any related fileCount/attachment state if
present).
In `@frontend/components/leaderboard/AchievementPips.tsx`:
- Line 197: Replace the non-standard Tailwind class z-9999 with Tailwind's
arbitrary value syntax z-[9999] wherever it's used so the tooltip gets an actual
z-index; update the className string in AchievementPips (the JSX element with
className="... z-9999 ...") and the analogous class in ActivityHeatmapCard to
use z-[9999] instead of z-9999.
In `@frontend/components/leaderboard/LeaderboardTable.tsx`:
- Around line 106-131: The context table rendered in the LeaderboardTable
component (the table that maps over contextRows and returns TableRow) is missing
an accessible label; add a visually hidden <caption> with the translated string
(e.g. t('contextTableCaption')) inside that table element (or, if you prefer,
add an aria-label to the table) so screen readers won't announce an unlabeled
second table — update the table that contains the colgroup and tbody where
contextRows are mapped (the TableRow usage and isCurrentUser logic stays the
same).
---
Outside diff comments:
In `@frontend/components/dashboard/FeedbackForm.tsx`:
- Around line 166-173: In FeedbackForm, the email input (name="email",
defaultValue={userEmail ?? ''}) is currently always readOnly which prevents
users from entering an address when userEmail is null; change the logic so the
input is editable when userEmail is null/undefined (e.g., set
readOnly={!userEmail} or render an editable input when !userEmail), ensure the
field is required/validated before submission, and/or display a clear inline
prompt/error near the email input telling users they must provide an email
before submitting so the server won't return a 400.
- Around line 108-119: In FeedbackForm.tsx update the submit response handling
(where result is read and setStatus is called) to detect the server file-size
rejection (check result.tooLarge === true or res.status === 413) and set a
distinct client state (e.g., setStatus('file-too-large') or
setStatus('tooLarge')) so the UI can show a specific message; keep the existing
success branch (setStatus('success'), clear accumulatedFilesRef.current and
setAttachmentNames([]), reset the form and successTimerRef) and only run the
generic setStatus('error') for other failures.
---
Nitpick comments:
In `@frontend/components/header/AppMobileMenu.tsx`:
- Around line 121-144: The touchmove handler repeatedly calls
document.getElementById('app-mobile-nav'); hoist that query out of the
preventTouchMove closure inside the useEffect so you capture the nav element
once when the effect runs (e.g., const nav =
document.getElementById('app-mobile-nav')), then use that captured nav in the
preventTouchMove handler to check nav.contains(e.target as Node); keep the same
preventTouchMove function reference for addEventListener/removeEventListener and
preserve restoring document.documentElement.style.overflowY in the cleanup;
reference symbols: useEffect, preventTouchMove, and the 'app-mobile-nav' id.
Comment on lines
+71
to
+79
| const attachments: { filename: string; content: Buffer; contentType: string }[] = []; | ||
|
|
||
| for (const f of files) { | ||
| if (f.size > MAX_FILE_SIZE) { | ||
| return NextResponse.json({ success: false, tooLarge: true }, { status: 413 }); | ||
| } | ||
| const buffer = Buffer.from(await f.arrayBuffer()); | ||
| attachments.push({ filename: f.name, content: buffer, contentType: f.type }); | ||
| } |
Contributor
There was a problem hiding this comment.
MIME header injection: sanitize f.name and f.type before passing to nodemailer.
f.name comes from the filename parameter of the multipart Content-Disposition header — a crafted HTTP client can set it to anything, including a value containing \r\n. Similarly, f.type is the raw Content-Type of the individual form part. Nodemailer places both into outgoing MIME headers (Content-Type: …; name="…" / Content-Disposition: attachment; filename="…"). If nodemailer doesn't encode CRLF in these values, a malicious actor can inject arbitrary MIME headers into the forwarded email.
Strip at minimum CRLF and null bytes from f.name, and validate f.type against an allowlist or a basic MIME token pattern before use:
🛡️ Proposed fix
- const buffer = Buffer.from(await f.arrayBuffer());
- attachments.push({ filename: f.name, content: buffer, contentType: f.type });
+ const buffer = Buffer.from(await f.arrayBuffer());
+ const safeFilename = f.name.replace(/[\r\n\0"\\]/g, '').trim().slice(0, 255) || 'attachment';
+ const MIME_TYPE_RE = /^[a-zA-Z0-9][a-zA-Z0-9!#$%&'*+\-.^_`|~]*\/[a-zA-Z0-9][a-zA-Z0-9!#$%&'*+\-.^_`|~]*$/;
+ const safeContentType = MIME_TYPE_RE.test(f.type) ? f.type : 'application/octet-stream';
+ attachments.push({ filename: safeFilename, content: buffer, contentType: safeContentType });📝 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
| const attachments: { filename: string; content: Buffer; contentType: string }[] = []; | |
| for (const f of files) { | |
| if (f.size > MAX_FILE_SIZE) { | |
| return NextResponse.json({ success: false, tooLarge: true }, { status: 413 }); | |
| } | |
| const buffer = Buffer.from(await f.arrayBuffer()); | |
| attachments.push({ filename: f.name, content: buffer, contentType: f.type }); | |
| } | |
| const attachments: { filename: string; content: Buffer; contentType: string }[] = []; | |
| for (const f of files) { | |
| if (f.size > MAX_FILE_SIZE) { | |
| return NextResponse.json({ success: false, tooLarge: true }, { status: 413 }); | |
| } | |
| const buffer = Buffer.from(await f.arrayBuffer()); | |
| const safeFilename = f.name.replace(/[\r\n\0"\\]/g, '').trim().slice(0, 255) || 'attachment'; | |
| const MIME_TYPE_RE = /^[a-zA-Z0-9][a-zA-Z0-9!#$%&'*+\-.^_`|~]*\/[a-zA-Z0-9][a-zA-Z0-9!#$%&'*+\-.^_`|~]*$/; | |
| const safeContentType = MIME_TYPE_RE.test(f.type) ? f.type : 'application/octet-stream'; | |
| attachments.push({ filename: safeFilename, content: buffer, contentType: safeContentType }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/api/feedback/route.ts` around lines 71 - 79, In the loop that
builds attachments (for (const f of files) { … }) sanitize and validate incoming
file metadata before pushing to attachments: strip CRLF (\r, \n), null bytes and
other control characters from f.name (and fallback to a generated safe filename
if the result is empty) and validate f.type against an allowlist or a strict
MIME token/type pattern (e.g. /^[a-z0-9.+-]+\/[a-z0-9.+-]+$/i); if f.type is
invalid, replace it with "application/octet-stream" or omit the media-type, then
push the sanitized values into attachments instead of raw f.name/f.type to
prevent MIME header injection when nodemailer constructs headers.
Comment on lines
110
to
116
| if (result.success) { | ||
| setStatus('success'); | ||
| setCategory(''); | ||
| setAttachmentName(null); | ||
| setAttachmentNames([]); | ||
| accumulatedFilesRef.current = []; | ||
| (e.target as HTMLFormElement).reset(); | ||
| successTimerRef.current = setTimeout(() => setStatus('idle'), 5000); |
Contributor
There was a problem hiding this comment.
fileLimitError not reset on successful submission — stale error banner reappears.
On a successful submit the form transitions to status === 'success' (hiding the form), then setStatus('idle') fires after 5 seconds, making the form visible again. Because setFileLimitError(false) is never called, a previously shown "too many files" banner will reappear alongside an empty attachment list, which is confusing.
🛠️ Proposed fix
if (result.success) {
setStatus('success');
setCategory('');
setAttachmentNames([]);
+ setFileLimitError(false);
accumulatedFilesRef.current = [];📝 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
| if (result.success) { | |
| setStatus('success'); | |
| setCategory(''); | |
| setAttachmentName(null); | |
| setAttachmentNames([]); | |
| accumulatedFilesRef.current = []; | |
| (e.target as HTMLFormElement).reset(); | |
| successTimerRef.current = setTimeout(() => setStatus('idle'), 5000); | |
| if (result.success) { | |
| setStatus('success'); | |
| setCategory(''); | |
| setAttachmentNames([]); | |
| setFileLimitError(false); | |
| accumulatedFilesRef.current = []; | |
| (e.target as HTMLFormElement).reset(); | |
| successTimerRef.current = setTimeout(() => setStatus('idle'), 5000); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/dashboard/FeedbackForm.tsx` around lines 110 - 116, On
successful submission (inside the block handling result.success in FeedbackForm,
where setStatus('success'), setCategory(''), setAttachmentNames([]),
accumulatedFilesRef.current = [], and form reset are performed), ensure the
file-limit error state is cleared by calling setFileLimitError(false) so any
previous fileLimitError banner does not reappear when the form returns to
'idle'; add this call alongside the other cleanup actions (and consider also
clearing any related fileCount/attachment state if present).
| tooltip && | ||
| createPortal( | ||
| <div | ||
| className="pointer-events-none fixed z-9999 whitespace-nowrap rounded-lg border border-gray-200 bg-white px-2 py-1 text-[10px] font-semibold text-gray-700 shadow-md dark:border-white/10 dark:bg-neutral-800 dark:text-gray-200" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the Tailwind configuration file
fd -type f -name "tailwind.config.*" -o -name "tailwind.config.*"Repository: DevLoversTeam/devlovers.net
Length of output: 241
🏁 Script executed:
# Check for CSS files with theme or z-index definitions
fd -type f \( -name "*.css" -o -name "*.scss" -o -name "*.postcss" -o -name "*.pcss" \)Repository: DevLoversTeam/devlovers.net
Length of output: 241
🏁 Script executed:
# Search for any explicit definition of z-9999
rg "z-9999|z-\[9999\]|zIndex.*9999" --type tsx --type ts --type js --type cssRepository: DevLoversTeam/devlovers.net
Length of output: 98
🏁 Script executed:
# Check if Tailwind config extends or defines zIndex
rg "zIndex|z-" tailwind.config.* 2>/dev/null || echo "No tailwind.config found or no matches"Repository: DevLoversTeam/devlovers.net
Length of output: 109
🏁 Script executed:
# Find tailwind config files
find . -name "tailwind.config.*" 2>/dev/nullRepository: DevLoversTeam/devlovers.net
Length of output: 53
🏁 Script executed:
# Search for z-9999 or z-[9999] in the entire repo
rg "z-9999|z-\[9999\]"Repository: DevLoversTeam/devlovers.net
Length of output: 471
🏁 Script executed:
# Look for any Tailwind theme configuration mentioning zIndex
rg "zIndex"Repository: DevLoversTeam/devlovers.net
Length of output: 132
🏁 Script executed:
# Check the package.json to understand the project structure
head -50 package.jsonRepository: DevLoversTeam/devlovers.net
Length of output: 142
🏁 Script executed:
# Search for CSS files that might define z-9999
find . -name "*.css" -o -name "*.scss" 2>/dev/null | xargs grep -l "z-9999" 2>/dev/null || echo "No CSS files define z-9999"Repository: DevLoversTeam/devlovers.net
Length of output: 97
🏁 Script executed:
# Look for next.config or other config files
find . -maxdepth 2 -name "next.config.*" -o -name "tsconfig.json" 2>/dev/nullRepository: DevLoversTeam/devlovers.net
Length of output: 144
🏁 Script executed:
# Check for any `@layer` or custom utilities defining z-9999
rg "@layer|@apply.*z-9999|--z-9999"Repository: DevLoversTeam/devlovers.net
Length of output: 198
🏁 Script executed:
# View the actual AchievementPips.tsx file to see context
cat frontend/components/leaderboard/AchievementPips.tsx | head -200 | tail -50Repository: DevLoversTeam/devlovers.net
Length of output: 2155
🏁 Script executed:
# Check the next.config.ts for Tailwind configuration
cat ./frontend/next.config.tsRepository: DevLoversTeam/devlovers.net
Length of output: 2625
🏁 Script executed:
# Check the globals.css utilities layer for z-9999 definition
rg "@layer utilities" frontend/app/globals.css -A 30Repository: DevLoversTeam/devlovers.net
Length of output: 832
🏁 Script executed:
# Check for tailwind.config in the entire repo
find . -name "tailwind.config.*" 2>/dev/null | head -20Repository: DevLoversTeam/devlovers.net
Length of output: 53
🏁 Script executed:
# Check if tailwind is even installed
grep -r "tailwindcss" . 2>/dev/null | grep -E "package.json|dependencies" | head -5Repository: DevLoversTeam/devlovers.net
Length of output: 251
🏁 Script executed:
# Look for tailwind configuration in package.json or any config files in frontend/
find frontend -maxdepth 2 -name "*.json" -o -name "*.config.*" 2>/dev/nullRepository: DevLoversTeam/devlovers.net
Length of output: 471
🏁 Script executed:
# Check postcss.config.mjs for Tailwind configuration
cat frontend/postcss.config.mjsRepository: DevLoversTeam/devlovers.net
Length of output: 164
🏁 Script executed:
# Check if tailwind config is in globals.css
head -50 frontend/app/globals.cssRepository: DevLoversTeam/devlovers.net
Length of output: 1784
🏁 Script executed:
# Check if there's a tailwind.config in the CSS files
rg "theme.*zIndex|extend.*zIndex|z-9999" frontend/Repository: DevLoversTeam/devlovers.net
Length of output: 471
z-9999 is not a standard Tailwind utility — tooltip may silently get no z-index.
Tailwind CSS v4 with the @theme inline syntax used in this project defines z-index values via CSS custom properties, but does not extend them to include z-9999. The standard Tailwind z-index scale (z-0, z-10, z-20, z-30, z-40, z-50, z-auto) does not include 9999, and the codebase does not add it to the theme. Without this definition, the z-9999 class is silently ignored, leaving the tooltip fixed element with no z-index property applied. This causes it to stack in natural document order and may appear beneath table rows or other overlaid content.
Use Tailwind's arbitrary value syntax z-[9999] instead:
Proposed fix
- className="pointer-events-none fixed z-9999 whitespace-nowrap rounded-lg ..."
+ className="pointer-events-none fixed z-[9999] whitespace-nowrap rounded-lg ..."This affects both frontend/components/leaderboard/AchievementPips.tsx and frontend/components/dashboard/ActivityHeatmapCard.tsx.
📝 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
| className="pointer-events-none fixed z-9999 whitespace-nowrap rounded-lg border border-gray-200 bg-white px-2 py-1 text-[10px] font-semibold text-gray-700 shadow-md dark:border-white/10 dark:bg-neutral-800 dark:text-gray-200" | |
| className="pointer-events-none fixed z-[9999] whitespace-nowrap rounded-lg border border-gray-200 bg-white px-2 py-1 text-[10px] font-semibold text-gray-700 shadow-md dark:border-white/10 dark:bg-neutral-800 dark:text-gray-200" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/leaderboard/AchievementPips.tsx` at line 197, Replace the
non-standard Tailwind class z-9999 with Tailwind's arbitrary value syntax
z-[9999] wherever it's used so the tooltip gets an actual z-index; update the
className string in AchievementPips (the JSX element with className="... z-9999
...") and the analogous class in ActivityHeatmapCard to use z-[9999] instead of
z-9999.
Comment on lines
+106
to
131
| <div className="overflow-hidden rounded-3xl border border-gray-200 bg-white/10 shadow-sm backdrop-blur-md dark:border-neutral-800 dark:bg-neutral-900/10"> | ||
| <table className="w-full table-fixed border-separate border-spacing-0 text-left"> | ||
| <colgroup> | ||
| <col className="w-[15%] sm:w-[12%]" /> | ||
| <col /> | ||
| <col className="w-[25%] sm:w-[20%]" /> | ||
| </colgroup> | ||
| <tbody> | ||
| {contextRows.map(user => { | ||
| const isMe = | ||
| user.userId === normalizedCurrentUserId || | ||
| (currentUsername && user.username === currentUsername); | ||
|
|
||
| return ( | ||
| <TableRow | ||
| key={user.id} | ||
| user={user} | ||
| isCurrentUser={!!isMe} | ||
| t={t} | ||
| /> | ||
| ); | ||
| })} | ||
| </tbody> | ||
| </table> | ||
| </div> | ||
| return ( | ||
| <TableRow | ||
| key={user.id} | ||
| user={user} | ||
| isCurrentUser={!!isMe} | ||
| inContext | ||
| t={t} | ||
| /> | ||
| ); | ||
| })} | ||
| </tbody> | ||
| </table> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Context table is missing a <caption> — screen readers will announce an unlabeled second table.
The main table at line 57 has <caption className="sr-only"> for accessibility. The context table at line 107 has no caption, so assistive technologies will announce a separate, unnamed table immediately after the main leaderboard table. Add a visually hidden caption (e.g. {t('contextTableCaption')}) or at minimum an aria-label on the wrapping element.
♿ Proposed fix
<table className="w-full table-fixed border-separate border-spacing-0 text-left">
+ <caption className="sr-only">{t('contextTableCaption')}</caption>
<colgroup>📝 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
| <div className="overflow-hidden rounded-3xl border border-gray-200 bg-white/10 shadow-sm backdrop-blur-md dark:border-neutral-800 dark:bg-neutral-900/10"> | |
| <table className="w-full table-fixed border-separate border-spacing-0 text-left"> | |
| <colgroup> | |
| <col className="w-[15%] sm:w-[12%]" /> | |
| <col /> | |
| <col className="w-[25%] sm:w-[20%]" /> | |
| </colgroup> | |
| <tbody> | |
| {contextRows.map(user => { | |
| const isMe = | |
| user.userId === normalizedCurrentUserId || | |
| (currentUsername && user.username === currentUsername); | |
| return ( | |
| <TableRow | |
| key={user.id} | |
| user={user} | |
| isCurrentUser={!!isMe} | |
| t={t} | |
| /> | |
| ); | |
| })} | |
| </tbody> | |
| </table> | |
| </div> | |
| return ( | |
| <TableRow | |
| key={user.id} | |
| user={user} | |
| isCurrentUser={!!isMe} | |
| inContext | |
| t={t} | |
| /> | |
| ); | |
| })} | |
| </tbody> | |
| </table> | |
| </div> | |
| <div className="overflow-hidden rounded-3xl border border-gray-200 bg-white/10 shadow-sm backdrop-blur-md dark:border-neutral-800 dark:bg-neutral-900/10"> | |
| <table className="w-full table-fixed border-separate border-spacing-0 text-left"> | |
| <caption className="sr-only">{t('contextTableCaption')}</caption> | |
| <colgroup> | |
| <col className="w-[15%] sm:w-[12%]" /> | |
| <col /> | |
| <col className="w-[25%] sm:w-[20%]" /> | |
| </colgroup> | |
| <tbody> | |
| {contextRows.map(user => { | |
| const isMe = | |
| user.userId === normalizedCurrentUserId || | |
| (currentUsername && user.username === currentUsername); | |
| return ( | |
| <TableRow | |
| key={user.id} | |
| user={user} | |
| isCurrentUser={!!isMe} | |
| inContext | |
| t={t} | |
| /> | |
| ); | |
| })} | |
| </tbody> | |
| </table> | |
| </div> |
🤖 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 106 - 131,
The context table rendered in the LeaderboardTable component (the table that
maps over contextRows and returns TableRow) is missing an accessible label; add
a visually hidden <caption> with the translated string (e.g.
t('contextTableCaption')) inside that table element (or, if you prefer, add an
aria-label to the table) so screen readers won't announce an unlabeled second
table — update the table that contains the colgroup and tbody where contextRows
are mapped (the TableRow usage and isCurrentUser logic stays the same).
ViktorSvertoka
approved these changes
Feb 22, 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
UI/UX Improvements
Bug Fixes
Internationalization