attendance qr fix - #13
Closed
nyxb1t wants to merge 62 commits into
Closed
Conversation
Pixel-for-pixel port of FED-Frontend (React 18 + Vite) and FED-Backend (Express) into a single Next.js 16 App Router application, running against the same MongoDB. The original SCSS modules are carried over verbatim (84 stylesheets, 24 image assets) and components keep their original markup and class names, which is what makes the replica indistinguishable from the original rather than merely similar. URLs keep the original casing (/Events, /Team, /Login). Verified against both apps running side by side: - /Events and /Team render at identical total page height (6109px, 9461px) with matching element counts and computed styles - All five public API responses are byte-identical to the Express server - typecheck clean, eslint 0 errors, build 68 routes, npm audit 0 vulns Includes all 48 Express endpoints, ported as Route Handlers preserving the original JSON contracts. Security fixes (npm audit 44 -> 0): - xlsx moved to the official SheetJS distribution (prototype pollution) - react-share-social removed for a local react-share component (-530 pkgs) - postcss and sharp pinned to patched versions via overrides - JWT_SECRET was 4 characters; replaced with 64 bytes of CSPRNG entropy, which rotates existing sessions - BCRYPT_SALT_ROUNDS held a bcrypt salt string rather than a cost factor - contact endpoint now validates email format and message length See MIGRATION.md for the full record, including known issues.
The original never navigated from inside Login.jsx — it called
authCtx.login(...) and let the React Router table react:
<Route path="/Login" element={authCtx.isLoggedIn ? <LoginRedirect /> : <Login />} />
App Router routes are files, so nothing observed isLoggedIn, and proxy.ts
only runs on a server request — which a client-side sign-in never makes.
A correct login showed "Login successful" and then sat on /Login forever.
AuthRouteGuard reinstates the guard for all five auth routes, honouring
both the original sessionStorage.prevPage and the ?next= the proxy
appends. It rejects protocol-relative return URLs, and cannot ping-pong
with the proxy when localStorage holds a session the cookie no longer
backs.
The layout-level guard added in 1b17715 was the wrong shape. SignUp and CompleteProfile sign the user in and then navigate themselves to "/", and a guard reacting to isLoggedIn cancels that in-flight router.push before it commits — measured on the signup flow, the push never reached history and a new account landed on /profile instead of "/". No delay fixes that reliably, since the push only commits once its RSC payload arrives. Login.jsx, GoogleLogin.jsx and GoogleSignup.jsx already carry the shouldNavigate/navigatePath state and the effect that acts on it — dead in the original precisely because the route table did the job. Setting setShouldNavigate(true) after authCtx.login(...) brings it to life, with no cross-component race. SendOtp.jsx already did this and is unchanged. postAuthRedirect() resolves the destination the way LoginRedirect did, plus the ?next= proxy.ts appends, discarding anything that is not a plain internal path. Verified against the running server, signed out and signed in: all five auth pages, all seven /profile pages, casing redirects, a tampered cookie (rejected and cleared), and the seven auth endpoints on an empty body. Two pre-existing gaps documented in MIGRATION.md rather than silently changed: /ForgotPassword submits natively (identical in the original), and /profile/members and /profile/BlogForm are not access-gated in the UI.
A correct login showed "Login successful" and then sat on /Login forever.
App.jsx never navigated from inside Login.jsx — it called authCtx.login(...)
and let the route table react:
<Route path="/Login" element={authCtx.isLoggedIn ? <LoginRedirect /> : <Login />} />
App Router routes are files, so nothing observes isLoggedIn, and proxy.ts only
runs on a server request, which a client-side sign-in never makes.
The redirect belongs in the components rather than in a layout wrapper. A guard
in app/(auth)/layout.jsx reacting to isLoggedIn was tried first and is wrong:
SignUp and CompleteProfile sign the user in and then navigate themselves to
"/", and a layout guard cancels that in-flight router.push before it commits.
Measured on the signup flow, the push never reached history at all and a new
account landed on /profile instead of "/". No delay fixes that reliably, since
the push only commits once its RSC payload arrives.
Login.jsx, GoogleLogin.jsx and GoogleSignup.jsx already carry the
shouldNavigate/navigatePath state and the effect that acts on it — dead in the
original precisely because the route table did the job. Setting
setShouldNavigate(true) after authCtx.login(...) brings it to life, with no
cross-component race. SendOtp.jsx already did this and is unchanged.
postAuthRedirect() resolves the destination the way LoginRedirect did, plus the
?next= proxy.ts appends. That value comes off the query string and is therefore
attacker-supplied, so anything that is not a plain internal path is discarded.
Verified in the browser by driving the real forms with the API stubbed:
Login /Login -> /profile
Login /Login?next=/Events -> /Events
Login /Login?next=//example.com/phish -> /profile, origin preserved
Login blocked page -> login -> back to it, prevPage cleared
Signup /SignUp -> /, matching the original
Login stale localStorage, no cookie -> login form, one bounce, no loop
Also verified every auth route against the running server, signed out and
signed in: all five auth pages, all seven /profile pages, the lowercase casing
redirects, a tampered cookie (rejected and cleared), and the seven auth
endpoints on an empty body.
Two pre-existing gaps are documented in MIGRATION.md rather than silently
changed: /ForgotPassword submits natively (identical in the original), and
/profile/members and /profile/BlogForm are not access-gated in the UI.
typecheck clean, eslint 0 errors, build 68 routes.
The landing page shipped 2.3 MB of JavaScript. Home.jsx imported its four
sections from the `sections` barrel, which also re-exports `sections/Profile` —
the whole admin panel. All of them are client components, so the bundler pulled
that entire graph onto the landing page: certificate tooling, admin tables, the
avatar editor, event analytics. The `features` barrel did the same for
LiveEventPopup. Under Vite this cost nothing noticeable; under Next, where each
route gets its own bundle, a barrel import silently undoes the code splitting.
Importing those four components directly, leaving the barrels in place for
other call sites:
/ 2317 KB -> 1082 KB
/Events 2063 KB -> 1082 KB
/Team 2014 KB -> 1082 KB
/Login 1248 KB -> 920 KB
Over the wire the landing page is now 327 KB of JS; locally TTFB 38ms,
DOMContentLoaded 135ms, load 536ms.
EventCard wrapped its meta row in a <p> containing div.price, which contained
another <p>. Client-rendered that was invisible, because React builds the DOM
node by node and nothing reparents it. Server-rendered it is real markup, so
the parser closed the <p> early and div.price came out a sibling instead of a
child — a different layout, reported as a hydration mismatch on every card. The
wrapper is a <div className={style.meta}> and the two `.eventname p` rules list
`.eventname .meta` alongside, so computed styles are unchanged: verified in the
browser at font-size 14.4px, display flex, align-items center, margin-top 1.6px
with div.price a real child, console clean across 580 cards.
Also: darken() -> color.adjust() in VerifyCertificate.module.scss, confirmed
identical by compiling both and diffing (rgb(80%, 43.2941176471%, 0%) either
way), which clears the Dart Sass deprecation warnings; and <html> now carries
data-scroll-behavior="smooth" to acknowledge the globals.scss rule.
The <Fit /> warnings are left alone — react-fit is a transitive dependency of
react-date-picker and uses the `warning` package, a no-op in production.
typecheck clean, eslint 0 errors, build 68 routes, all pages 200, auth route
matrix unchanged.
Four components wrapped block-level content in a <p>: EventCard and EventModal (<p> -> div.price -> <p>), Hero (<p> -> <span> -> <h3>) and Social (<p> -> div.fed). Client-rendered under Vite none of it mattered, because React builds the DOM node by node and nothing reparents an existing tree. Server-rendered it is real markup, so the parser closes the <p> at the first block child and the content lands as a sibling — a different layout, reported as a hydration mismatch. EventCard was fixed in 9a1073e; this covers the other three. Each wrapper is a <div> carrying a class listed alongside the original `p` selector, so computed styles are unchanged. Verified in the browser: EventModal .meta 14.4px / flex / center / 1.6px / #fff Hero .tagline 39.2px / 700 / #fff Social .content 40px / 600 / #fff / center Hero keeps its <h3> rather than downgrading it to a span — the wrapper changed instead, so the heading still counts as a heading. Social's `styles.content` had no rule in the stylesheet at all, so the className resolved to undefined and the element was styled purely by `.text p`; `.content` now exists and carries those declarations. Fixing Social surfaced a second, unrelated mismatch on that page: react-social-media-embed mints a fresh UUID per render into both `id` and `className`, and the embed sizes come from useDimensions(), which measures 0 on the server. Both embeds now load via next/dynamic with ssr: false — nothing is lost, since the visible post is drawn by Instagram's and LinkedIn's own scripts after mount. Same four embeds, same 1674px page height, console clean. npm run audit:nesting keeps this from recurring. It walks a tag stack through every JSX file and reports anything the HTML parser would reparent, skipping comments and string/regex literals — without that a comment mentioning <p>, or `.replace(/<a\s[^>]*>/gi, '')`, is read as markup. It was validated against the pre-fix files: it reports all four real cases and nothing for the two files that produced false positives. Current tree: 153 files, zero findings. typecheck clean, eslint 0 errors, build 68 routes, no Sass deprecations.
The calendar on the admin form page had no styling at all. The original
index.scss imported three vendor stylesheets and only one was carried over, so
react-date-picker's popup — which it draws with react-calendar — came out as an
unstyled column of numbers. Confirmed by grepping the emitted CSS: zero
.react-calendar rules shipped, now 50. In the browser the popup measures
350x248 with a #a0a096 border, react-calendar's own defaults.
Re-reading every form/event controller against its port turned up more:
getFormAnalytics returned the wrong shape entirely. EventStats.jsx reads
response.data.form.formAnalytics, .form.info and .yearCounts; the port returned
{ success, message, data }, so response.data.form was undefined and the admin
analytics panel threw. It now returns { message, form, yearCounts }, rebuilds
the yearCounts histogram, and restores the 404 for a form nobody has registered
to. Access is the controller's own allowlist (ADMIN/PRESIDENT/VICEPRESIDENT/
DIRECTOR_*) plus the srex@fedkiit.com escape hatch, answering 401 "Access
Denied", not the isMember test the port had invented.
The attendance flow did not work. The QR carries a signed JWT: attendanceCode
must return { message, attendanceToken } with a 20-minute expiry, and
markAttendance takes { formId, token } and verifies it. The port returned the
raw record id under a different key, so QRCodeModal generated no QR, and
markAttendance rejected the scanned JWT for not being a 24-character id. Both
now match, including the formId binding that stops one event's QR checking
someone in at another, and the ?teamCode= branch. Verified both directions: a
token minted by the original's jsonwebtoken call verifies with jose here, and
vice versa; tampered tokens are rejected.
Access levels corrected both ways: export-attendance is ADMIN again (was any
club member, which handed every executive the full attendee list), while
markAttendance drops to signed-in, matching the Express route whose checkAccess
is commented out — the door volunteer signs in as a plain USER, so requiring
member access locked the door staff out.
Cloudinary dimensions were wrong: addForm/editForm resize FormImages at
h350.67 x w196.37 and QR media at h400 x w150 and h150 x w400 respectively, not
the 1000x1000 / 500x500 the port used. The two QR rows are transposed against
each other in the original and are reproduced per call site rather than
reconciled. addForm also returns 200 "Form created successfully" and editForm
"Form info and sections updated successfully".
One divergence kept deliberately: Express's addForm computes
Boolean(isPublic) || false over a multipart field, and Boolean("false") is
true, so every event created through the admin form was public,
registration-closed and past regardless of the toggles. editForm already used
=== "true". The port uses editForm's form in both so the switches work;
restoring byte-parity would re-break them. Documented in MIGRATION.md along
with checkAccess("USER") being stricter in Express than here.
typecheck clean, eslint 0 errors, build 68 routes, nesting audit clean.
Introduces a broad UI refresh across auth, events, team, profile, blog, chatbot, and modal surfaces using shared design tokens, updated layouts, and improved accessibility/interaction patterns (including a reusable CloseButton and new event artwork/disclosure components). Adds certificate backend support with new routes (`createOrganisationEvent`, `getEventByFormId`, `myCertificates`) and matching service logic, plus related flow fixes (team management query cleanup, event/certificate lookup behavior) and `prisma generate` on postinstall.
The password-reset email carried a 6-digit code while every OTP screen renders four boxes, so the code could not be entered at all. The reset flow and signup share components/OtpInput, so both were affected. The UI was not at fault — it is four boxes in the original too, character for character. This port had raised OTP_LENGTH from 4 to 6 as hardening, without a consumer that could accept six. Reverted to 4, matching generateOtp(4, false, false, false). The argument for 6 does not hold here: 10,000 combinations are brute-forceable against the Express backend, which had no throttling at all, but RATE_LIMITS.passwordReset allows 6 attempts per 15 minutes and a code expires after 15, so an attacker gets at most 6 guesses out of 10,000 before the code stops existing. The rest of the OTP hardening is untouched: SHA-256 digest at rest, constant-time comparison, single-use, and expiry derived from createdAt rather than a setTimeout that never fires on a serverless host. Codes issued before this change are 6 digits and cannot be entered; they expire on their own within 15 minutes. typecheck clean, eslint 0 errors, build 68 routes.
The disabled "Resend OTP" button rendered with a grey box. Measured against the
original running side by side, it had background-color rgba(19,1,1,0.3) and a
2px outset border where the original has rgba(0,0,0,0) and 0px none — the
user-agent's disabled-button chrome showing through.
The cause is structural, not a mistranslation. TeamCard.module.scss has a
top-level bare `button { }` rule; Vite does not hash element selectors in CSS
Modules and bundles every module into one stylesheet for the SPA, so that rule
was live on every page of the original. Next code-splits CSS per route, so once
ported it only loaded where TeamCard did — /Team and /profile/members — and
buttons everywhere else lost their reset.
Declared in app/globals.scss, which reproduces the original cascade. At
specificity 0-0-1 every component's own class rules still win: verified by
diffing all three buttons on /Login between the two apps, identical background,
font-size, margins and box sizes, and the resend button on /otp now matches the
original on every property (transparent, 0px none, 14.4px, 16px margin-top,
126x19).
Ten other top-level :global(...) rules sit in the same trap and are listed in
MIGRATION.md. They are left alone deliberately — three are :global(*) and would
move layout on every page, so they want reviewing one at a time rather than
hoisting wholesale.
Separately, App.jsx renders <Chatbot /> above <Routes>, so it is on every route.
The port mounted it in the (main) layout, hiding it on /Login, /SignUp, /otp,
/ForgotPassword and /completeProfile. Moved to the root layout, inside Providers
since it reads AuthContext. Confirmed in the server-rendered HTML for all six
routes, and the toggle measures 72x72 on /Login in both apps.
typecheck clean, eslint 0 errors, build 68 routes.
/otp rejected a correct code with "Email, otp and password are required".
OtpInput.jsx posts { newPassword, confirmPassword, otp, email }, matching
Express; this handler destructured `password`, so the check failed before the
code was ever looked at.
Restored the rest of that controller's contract too, which had been simplified
away: 400 "Missing fields." when any of the four is absent, 409 on a
confirm-password mismatch, 404 "User not found!" (Express's checkAccess looks
the account up by the body's email, which is also why the endpoint works without
a session), 400 when the new password equals the old one, and
200 { status: "OK", message: "Password has been changed successfully !!" }.
This was the third contract break found by report rather than by testing, so
npm run audit:contracts now walks every api.*() call in the components, resolves
it to its Route Handler and flags payload keys the handler never reads. It found
two more live ones:
renameTeam UI sends newTeamName, handler read teamName
sendJoinRequest UI sends teamRegistrationId, handler read teamCode
Every rename failed on an empty name and every join request was rejected. Both
now match Express, including sendJoinRequest identifying the target by
registration row id — the value searchTeams already returns to the UI as
teamRegistrationId — with the explicit team.formId !== formId check that a
global row id makes necessary.
The audit also surfaced that the admin certificate tooling calls five endpoints
that do not exist here (getEvent, getEventByFormId, createOrganisationEvent,
sendBatchMails, sendCertViaEmail), out of eleven missing overall. That is a gap
rather than a regression — it was never built — and is documented in
MIGRATION.md rather than fixed in passing, since the certificate flow already
carries a deliberate architectural deviation.
typecheck clean, eslint 0 errors, build 68 routes.
React Router returns [params, setParams]; Next returns the params object itself.
Two components outside the team feature kept the array destructuring:
EventForm.jsx crashed opening the event registration form
VerifyCertificate.jsx crashed opening certificate verification
ReadonlyURLSearchParams is iterable, so destructuring it as an array does not
throw — it quietly yields the first [key, value] entry, or undefined when the
URL has no query string. The next .get() then fails with "Cannot read
properties of undefined (reading 'get')".
EventForm is the one that hid: it reads searchParams.get("teamCode") during
render, but ProtectedRoute redirects signed-out visitors first, so an anonymous
smoke test returns 200 and only a signed-in user reaches the crash.
Swept the tree for the rest of the React Router surface (useNavigate,
useLocation, Outlet, Navigate, to=): only comments remain.
TeamManagement.jsx has the same bug and is fixed on feature/team-member-fix,
where the rest of the team work lives.
GET|HEAD /api/health returns 200 when every dependency answers and 503 when one does not. There was no equivalent in the Express backend, so uptime monitors had to be pointed at a real endpoint like /api/form/getAllForms — which reads the whole forms collection on every poll and still answers 200 when the database is unreachable but the response is cached. The database check uses MongoDB's `ping`, the cheapest command the driver exposes: it touches no collection and reads no documents, so polling costs nothing. It is bounded at 3s. A hung TCP connection to Atlas does not reject — it waits for the driver's own much longer timeout — and a probe that hangs reads to a monitor as a timeout rather than a clean 503. The endpoint is unauthenticated (proxy.ts already excludes /api/), so the error body carries only "unreachable" or the timeout message. Driver errors can embed the connection string, which holds the Atlas password; the detail goes to the server log instead. HEAD is exported explicitly because Next does not derive it from GET for route handlers, and a monitor configured for HEAD would otherwise get 405 and report the site as down. force-dynamic + no-store: a health check answering from build output or a cache would report "ok" for a process that is not actually serving. Verified both paths against the real database: 200 with a 36ms ping, and 503 in 3.03s with DATABASE_URL pointed at an unreachable host (HEAD likewise 503).
The five components were already ported line for line; the backend behind them
was not. It had been written against a different data model, so most of the page
did not work.
The model: a team is one formRegistration row holding every member's address in
regTeamMemEmails and every member's answers in value, with userId as the leader.
Leaving or being removed lifts a person's entries out of that row into their own
UNAFFILIATED row, so they stay registered and can join or start another team.
The port assumed a row per member sharing a teamCode, earliest row as leader.
That assumption produced most of the faults:
teamDetails looked up by userId, so only the leader could load the page.
Confirmed on live data: the ownership query returns NULL for a
real member of team "ABC"; the membership query finds it.
teamDetails returned registrationId/regTeamMemEmails/isLeader, while the UI
reads eventTitle, leaderEmail, max/minTeamSize,
isRegistrationClosed, isEventPast and data.isTeamless — and
members lacked college and year.
teamDetails had no UNAFFILIATED branch, so TeamlessState — the entire
create/join flow — could never render.
searchTeams returned a flat array of {teamName,teamCode,size,maxSize,isFull}
and read ?q=, where the picker reads data.data.teams with
teamRegistrationId/teamSize/maxTeamSize/leaderName/
spotsRemaining/hasPendingRequest and sends ?search=. The list
rendered empty and the search box filtered nothing.
leaveTeam keyed on userId with no leader/member distinction: a member
could not leave, answers were not carried across, and the
tracker's regTeamNames was never released.
removeTeamMember assumed row-per-member, so removal failed and no email went
out.
Also restored: leaveTeam blocks a leader who still has members; the
closed-registration guard compares the flags as the strings "true"/"false" the
data actually stores; joinTeam returns eventId (falling back to formId when
relatedEvent is absent or the literal "null"); joinRequestUpdates returns
pendingCount; the removed-member email is sent, following
emailTemplates/removedMember.html. Every success message matches the original,
since the components put response.data.message straight into a toast.
Verified against the live database, read paths only: teamDetails as leader and
as member return identical full payloads, the teamless branch returns
isTeamless with the event's limits, searchTeams returns data.teams with every
field the picker needs and ?search= filters correctly, inviteLink and
joinRequestUpdates match.
The mutations are deliberately not exercised: this database holds real
registrations and running them would rewrite other people's teams. They are
matched to the controllers line by line and typecheck clean, but want a
run-through on a scratch database before release.
typecheck clean, eslint 0 errors, build 68 routes, audit:contracts clean for
every team endpoint.
TeamManagement.jsx carried React Router's [params, setParams] destructuring.
Next returns the params object itself, so opening any team page failed with
"Cannot read properties of undefined (reading 'get')".
ReadonlyURLSearchParams is iterable, so destructuring it as an array does not
throw — it quietly yields the first [key, value] entry, or undefined when the
URL has no query string. The next .get() is what fails.
TeamManagement also cleaned the URL after an email-redirect toast by mutating
the params and calling the setter. Next's object is read-only with no setter,
so that is now a copy plus router.replace(..., { scroll: false }) — the toast
still does not re-fire on refresh and no history entry is added.
EventForm.jsx and VerifyCertificate.jsx had the same bug from the base
migration; they are fixed on auth-redirect-fix since neither is team work.
Verified in the browser against live data: the team page renders in full for a
non-leader member (team, code, 2/3 members with year and college, "You" marker,
Leave Team) and for the leader (invite panel, share link, per-member remove);
?toast=joined&name=... is consumed and stripped from the URL; console clean on
both.
inviteLink is built from the request rather than hardcoded, so production traffic already produced production URLs — the localhost in a dev screenshot is dev's own origin. But the origin was taken verbatim, and these URLs go into email: the team invitation, and the accept/reject buttons sent to a team leader. Origin and Host are set by the caller. Reflected unchecked, anyone able to create a team could have FED KIIT send a message, from its own address, containing a link to a domain they chose. Measured before the change, a request carrying `Origin: https://evil.example` came back with `https://evil.example/Events/.../Form?teamCode=...`. An origin is now trusted when it matches NEXT_PUBLIC_SITE_URL, or is localhost so a developer's copied link still works locally; anything else falls back to NEXT_PUBLIC_SITE_URL. After the change: Origin: https://www.fedkiit.com -> https://www.fedkiit.com/Events/... Origin: http://localhost:3999 -> http://localhost:3999/Events/... Origin: https://evil.example -> falls back to the real host Host: evil.example -> https://www.fedkiit.com/Events/... The Express controller reflected these headers (req.headers.origin || process.env.FRONTEND_URL || "https://fedkiit.com"), so this is a deliberate deviation, noted in MIGRATION.md. typecheck clean, eslint 0 errors, build 68 routes.
Clicking an invite while signed out sent you to the login page and then dropped
you on the team-finding page instead of joining the team.
The auto-join was never the problem — it was ported and works. The destination
was thrown away during authentication, in four places:
Login.jsx "Sign Up" link overwrote prevPage with /Login, discarding the
invite ProtectedRoute had just saved
SignUP.jsx router.push("/") after signing up
GoogleSignup.jsx cleared prevPage, then resolved to /profile
CompleteProfile.jsx router.push("/")
So the very case an invite exists for — someone without an account — lost it
before signing up. All four now resolve through postAuthRedirect(), which takes
a fallback argument so the signup screens still default to "/" as before.
Measured through the real UI with the login and join calls stubbed, so nothing
was written to the database:
open invite signed out -> /Login, prevPage = /Events/<id>/Form?teamCode=...
click "Sign Up" -> prevPage unchanged (previously became "/Login")
sign in -> POST /api/auth/login
POST /api/form/joinTeam {formId, teamCode}
-> /Events/<id>/team
Email and WhatsApp links carry the same ?teamCode=, so both behave identically.
A brand-new account still fills in the event's registration form first: team
membership is a formRegistration row, so there is nothing to move onto a team
until the person has registered, and that form is where required details and
payment are collected. The invite is carried through it — EventForm passes the
code to PreviewForm, which joins the team as soon as registration succeeds.
typecheck clean, eslint 0 errors, build 68 routes.
PreviewForm joins the invited team the moment registration succeeds. When that
join was rejected it only logged to the console and fell through to
router.push("/Events") — the person ended up on the events listing, registered
but teamless, with nothing on screen explaining that the invite had not been
honoured.
Two paths were silent, not one. Besides the catch, a 200 carrying
success: false also fell through untouched. Both now redirect to
/Events/<formId>/team?toast=join_failed&reason=<api message>
The team page is the right destination: they are registered by then, just
unaffiliated, so TeamManagement renders TeamlessState — the team search. They
land on the screen that lets them recover instead of the events listing.
The reason is forwarded rather than hard-coded, because the same path catches
four different rejections from joinTeam: "This team is full", "Invalid team
code", "Registration is closed for this event" and "You are already in a team".
TeamManagement appends "You can join another team below." and strips reason from
the URL alongside toast and name, so the toast does not re-fire on refresh.
Also dropped a dead `const eventId` in the success branch — it was computed and
never read; the redirect uses form.id directly.
The same silent fall-through is in FED-Frontend PreviewForm.jsx:191-194, so the
behaviour is inherited, not a port defect.
Not exercised against the database: joinTeam is a mutation and the configured
Atlas instance holds real registrations. eslint 0 errors, build clean.
The mutations had only ever been matched to the controllers by eye. Running them end to end against the database with two real accounts found four defects. joinTeam failed outright with a 500. formRegistration carries @@unique([formId, teamCode]): the model is one row per TEAM, holding the whole roster in regTeamMemEmails, not one row per member. The port stamped the team's code onto the joiner's own row, colliding with the team row on that constraint, so every join died with a Prisma P2002. Joining is a merge — the joiner's email and value entry move onto the team row and their solo row is deleted. This is what the entire invite-link flow depends on, so that flow could not have worked. respondJoinRequest failed the same way for the same reason, so a leader accepting from their email got the generic error page and the request stayed PENDING for ever. It also now handles the requester having joined elsewhere in the meantime, and a team that filled up, as expiries rather than errors. renameTeam had no leader check at all — any member could rename the team. The controller looks the row up by membership and compares its owner to the caller; this loaded the caller's own row and compared nothing. Also missing: the registration-closed check, the "name unchanged" no-op, and the tracker's name swap, so a rename left the old name reserved for ever. Team codes were built from a slug of the team name (CLAUDETE-7130) instead of the controller's <2-letter event code>-<3-digit index>-<4 digits> (AR-003-8793). Every code already in the database uses the latter and people share them by hand, so the wrong shape makes a valid code look fake. Two messages were wrong with them: createTeam used a commented-out v1 string from addRegistration.js rather than the live one, and renameTeam invented its own. Both now match the controllers. One deliberate deviation kept: the controller lets a teamless registrant rename their own UNAFFILIATED placeholder, producing a named "team" with a SOLO- code and absent from the tracker. Guarded here. Verified on the "Team Test" event with two accounts belonging to the repo owner, restored to their original state afterwards; no team containing anyone else was touched. 20/20 asserted steps pass across createTeam, duplicate name, searchTeams, joinTeam (valid/invalid/already-joined), renameTeam (leader/non-leader/unchanged), inviteTeamMember, removeTeamMember, leaveTeam (member and leader-last), sendJoinRequest and respondJoinRequest (accept and replay). Both accounts end TEAMLESS with no leftover PENDING requests. typecheck clean, eslint 0 errors, build 68 routes.
A pending join request survived the dissolution of the team it pointed at, and came back to life the moment that leader created their next team. A leader could disband "ABC", create "BCD", and the person who asked to join ABC would be pulled into BCD without ever agreeing to it. teamRegistrationId cannot tell the two apart: the registration row is reused, so its id survives a rename and a disband alike. Confirmed against the database rather than assumed — after a disband and a fresh createTeam, the request's teamRegistrationId still equals the new team's row id. teamCode is the discriminator. A rename leaves it untouched; disbanding resets it to a SOLO- code and the next createTeam mints a fresh one. teamJoinRequest now pins the code at the time of asking, and acceptance compares it: renames ABC -> BCD code unchanged request accepted, link works disbands ABC, creates BCD code new request auto-expired, link 404 The invite-link path already had the right semantics, since those links carry ?teamCode=, but it was verified rather than taken on trust. The field is optional: rows written before it existed carry no code, and a request that cannot be verified is treated as stale. There were 45 requests in the database and none pending when this shipped, so nothing was grandfathered. MongoDB needs no migration for an optional scalar with no index — prisma generate is enough, no db push against production. Verified on the live database, 9/9 assertions across all three cases, plus the 14-step lifecycle re-run to confirm nothing regressed. Both test accounts end TEAMLESS with no leftover PENDING requests. typecheck clean, eslint 0 errors, build 68 routes.
upstream/main carries PR #3 as a squash commit (8a4b896 / f5e6387), which has no ancestry link to the 596c5f6 it was squashed from. Git therefore saw the same sign-in redirect work as two unrelated changes and conflicted on three files. Resolutions, all of them "ours supersedes upstream" rather than a merge of two live variants: src/utils/postAuthRedirect.js add/add. Ours is a superset: it takes a `fallback` argument, which the signup screens pass as "/". Upstream's is the earlier hardcoded /profile form, and keeping it would send new accounts to the wrong page. src/authentication/SignUp/ Upstream still calls GoogleSignup.jsx sessionStorage.removeItem("prevPage") before redirecting. That is the exact line that discarded a team invite when someone signed up with Google, removed in 98848cb. MIGRATION.md Both hunks are additions on our side with nothing opposite them; markers stripped so upstream's non-conflicting edits elsewhere in the file are kept, which --ours would have discarded. Checked that nothing upstream-only was lost: the only lines dropped are the four superseded ones above. app/(auth)/layout.jsx is byte-identical to upstream. typecheck clean, eslint 0 errors, build 68 routes.
beta and main each carry their own squash of PR #3 — 8a4b896 on beta, f5e6387 on main — as separate commits with the same content and no ancestry link to the 596c5f6 they came from. So merging beta conflicted on the same three files that merging main did, and resolved the same way: src/utils/postAuthRedirect.js ours keeps the `fallback` argument the signup screens pass as "/" src/authentication/SignUp/ ours keeps the removal of GoogleSignup.jsx sessionStorage.removeItem("prevPage"), the line that discarded a team invite during Google sign-up MIGRATION.md additions on our side only; markers stripped rather than taking --ours Confirmed again that the only upstream lines dropped are those four superseded ones. The branch now descends from both upstream/beta and upstream/main, so a PR opened against either target merges without conflicts. beta is the one that matters here: work lands there first and reaches main from beta. typecheck clean, build 68 routes.
The generated client lives in node_modules/.prisma/client, which is gitignored, so it never travels with a commit. @prisma/client has its own postinstall hook, but npm only fires that during an install — and a schema change on its own does not touch package.json, so nobody has a reason to reinstall. That combination is the trap: pulling a branch that changed schema.prisma leaves a client that no longer matches it, the app starts perfectly, and the mismatch only surfaces as "Unknown argument" on whichever request happens to touch the new field. npm run build and npm run typecheck do fail loudly, but npm run dev does not type-check up front, so the usual workflow hides it. The teamCode field added for the rename-vs-disband fix is the first change with this hazard. scripts/ensure-prisma.mjs runs the check where it cannot be skipped: with-env.mjs calls it, and dev, build and start all go through that. It regenerates only when the schema actually changed, comparing a SHA-256 of schema.prisma against a stamp written beside the generated client. Generating unconditionally would add seconds to every dev start; hashing one file is imperceptible, so the common case is free. The stamp sits inside the client directory deliberately — deleting node_modules takes it along, so a wiped install cannot look up to date — and is written only after a successful generate, so a failure retries instead of marking a client that was never produced as current. postinstall runs the same script with --optional, which tolerates a missing CLI: prisma is a devDependency, so npm install --omit=dev on a deploy host legitimately has none and failing there would break the install. A dev server without one is not legitimate, so that path leaves the flag off and fails. npm run prisma:generate is the manual escape hatch. package-lock records hasInstallScript for the new hook, and npm re-resolved the Prisma engines from dev to devOptional. Verified all three paths: no stamp generates; unchanged schema is silent with no generate spawned; a changed schema is detected and regenerated, both standalone and through a real npm run dev, which then served /api/health 200. npm install still exits 0. eslint clean, typecheck clean, build 68 routes.
The merged PR read `response.data.data.attendanceId` from
/api/form/attendanceCode, but that endpoint returns
`{ message, attendanceToken }` at the top level — the same shape the
Express controller returns (markAttendance.js:99). `response.data.data`
is undefined, so reading `.attendanceId` off it threw a TypeError and QR
generation failed outright.
Its duplicate-scan branch was unreachable for the same reason: a rescan
comes back as 400 "Attendance already marked." (markAttendance.js:154),
never a 200 carrying `data.alreadyMarked`. That intent is kept, moved to
the catch block and keyed off the response the API actually sends, so a
second scan reads as information rather than a red error.
The double-scan guard from the PR is untouched — it fixed a real bug.
Also stop reporting a lapsed volunteer session as a bad QR: both are 401,
so the API's own wording is now shown.
/profile/attendance was reachable by any signed-in participant. The sidebar only ever showed the link to admins, but that is presentation: proxy.ts guards /profile by checking for a valid session, not a role, and the route entry had no check of its own. Confirmed against the live app — both test accounts (access=USER) got a working scanner by typing the path. Since a participant can also mint their own QR through /api/form/attendanceCode, which exists precisely so they can display it, that combination let anyone mark themselves present. The gate that matters is on the API: markAttendance now returns 403 to non-admins, so the page is a UI over a call that refuses. Issuing a code stays open to any signed-in user; only redeeming one is restricted. This diverges from the Express route, which has its checkAccess commented out entirely and accepts unauthenticated calls. Note: the sidebar's attendance@fedkiit.com special case is dead code — no such account exists.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/views/AttendancePage/AttendancePage.jsx`:
- Around line 162-170: Update the scanner lifecycle in the AttendancePage
component to store the active scanner in a ref before render callbacks can
execute, rather than relying on the scanner state captured by onScanSuccess. Use
that ref in both the duplicate-attendance handling and cleanup paths to call
clear() on the active scanner, and keep the ref synchronized when the scanner is
created or reset.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dacb78dd-1ae9-4dd6-89fa-32b3c08d8487
📒 Files selected for processing (5)
app/(main)/profile/attendance/page.jsxapp/api/form/markAttendance/route.tsscripts/audit-nesting.mjssrc/features/Modals/Event/QRCodeModal/QRCodeModal.jsxsrc/views/AttendancePage/AttendancePage.jsx
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/audit-nesting.mjs
Comment on lines
+162
to
+170
| if (scanner) { | ||
| try { | ||
| scanner.clear(); | ||
| } catch (clearError) { | ||
| console.error("Error clearing scanner:", clearError); | ||
| } | ||
| } | ||
| setShowScanner(false); | ||
| setScanner(null); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 \
'qrScanner\.render|setScanner\(|onScanSuccess|scanner\.clear|scannerRef|useEffect\(' \
src/views/AttendancePage/AttendancePage.jsxRepository: AnshRaj112/fedkiit
Length of output: 3852
🏁 Script executed:
#!/bin/bash
set -euo pipefail
awk 'BEGIN{RS=""; FS="\n"} {print NR": "length }' src/views/AttendancePage/AttendancePage.jsx
cat -n src/views/AttendancePage/AttendancePage.jsx | sed -n '70,180p'
cat -n src/views/AttendancePage/AttendancePage.jsx | sed -n '310,370p'
printf '\n--- package files mentioning Html5QrcodeScanner/html5-qrcode ---\n'
rg -n "Html5QrcodeScanner|html5-qrcode|qrscanner|Scanner" -S . --glob '!node_modules' --glob '!dist' --glob '!build' --glob '!coverage'Repository: AnshRaj112/fedkiit
Length of output: 8421
🌐 Web query:
html5-qrcode 2.3.8 Html5QrcodeScanner render source register callback
💡 Result:
In html5-qrcode version 2.3.8, the render method of the Html5QrcodeScanner class is used to initialize and display the scanner interface [1][2][3]. The method signature is: render(qrCodeSuccessCallback, qrCodeErrorCallback) [1][3] Parameters: 1. qrCodeSuccessCallback (Required): A callback function executed when a QR code or supported barcode is successfully detected [1][3]. It receives two arguments: decodedText (string) and decodedResult (object containing metadata) [4][3]. 2. qrCodeErrorCallback (Optional): A callback function executed when no code is found during a scan attempt [1][3]. This is typically used for logging or handling continuous scan failures [5][3]. Example implementation: function onScanSuccess(decodedText, decodedResult) { console.log(Code matched = ${decodedText}, decodedResult); } function onScanFailure(error) { // Handle scan failure, often ignored to keep scanning console.warn(Code scan error = ${error}); } let html5QrcodeScanner = new Html5QrcodeScanner( "reader", { fps: 10, qrbox: { width: 250, height: 250 } }, /* verbose= */ false); html5QrcodeScanner.render(onScanSuccess, onScanFailure); The render method handles the creation of the user interface within the DOM element specified by the elementId passed to the constructor [3]. For further API details, refer to the official documentation at scanapp.org/html5-qrcode-docs [1][4][6].
Citations:
- 1: https://scanapp.org/html5-qrcode-docs/docs/apis/classes/Html5QrcodeScanner
- 2: https://npmx.dev/package/html5-qrcode/v/2.3.8
- 3: https://github.com/mebjas/html5-qrcode/blob/master/src/html5-qrcode-scanner.ts
- 4: https://github.com/mebjas/html5-qrcode
- 5: https://scanapp.org/html5-qrcode-docs/docs/intro
- 6: https://www.npmjs.com/package/html5-qrcode
Store the active scanner outside the callback closure.
scanScanner.render(onScanSuccess, onScanFailure) runs before setScanner(qrScanner), so onScanSuccess closes over scanner === null. The duplicate-attendance path and cleanup can then skip scanner.clear() while the camera remains active. Store the scanner in a ref and clear it from that ref in both paths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/views/AttendancePage/AttendancePage.jsx` around lines 162 - 170, Update
the scanner lifecycle in the AttendancePage component to store the active
scanner in a ref before render callbacks can execute, rather than relying on the
scanner state captured by onScanSuccess. Use that ref in both the
duplicate-attendance handling and cleanup paths to call clear() on the active
scanner, and keep the ref synchronized when the scanner is created or reset.
Taken selectively rather than wholesale. The source branch carries merges
made with "accepting all incoming changes", which had reverted the
migration work it was branched from — a plain merge applied cleanly and
silently rolled back 79 backend files (+743/-1368), including the team
mutation fixes, the Google sign-in fix and the env schema.
Merged with -s ours and then brought his work in deliberately, so nothing
of ours could be lost by omission.
Taken:
- Home revamp: Home.jsx, Hero, About, Feedback, Sponser, Contact and a
new EventsSection, plus HeroGallery, CustomCursor, CloseButton and
the Event Artwork component
- The revamped navbar (app/components/Navbar.tsx) and its design
tokens (app/globals.css)
- SCSS restyling across ~100 existing stylesheets
- New image assets, and dark-theme overrides for the date pickers
Not taken:
- Every backend file. All 76 of his modifications there were reverts.
- app/events, app/team, app/login, app/insights and app/data — static
mock-ups reading hardcoded data, unreachable behind the existing
proxy redirects. Wiring them up would have replaced the working
Events page with a prototype.
- app/sections/*.tsx and app/components/Footer.tsx, which nothing
imports even on his own branch.
- His package.json, which dropped the audit and prisma scripts and
added "saas" — a typo for the already-present "sass"; the real
package has no repository and has not been published since 2015.
- 207 files of editor/agent configuration and committed temp
artifacts (.tmp-shots, skills-lock.json).
Adjusted on the way in:
- Root layout keeps the Chatbot. His moved it into the main layout,
which hides it on Login, SignUp and OTP — the reason it was moved out
in the first place — and replaced the metadata title template with
hardcoded strings.
- The navbar's mobile backdrop was the one element written in Tailwind
utilities. Translated to plain CSS so no second styling system is
needed; nothing else in the merged set uses one.
- The navbar reset its state inside an effect, which the lint config
rejects as an error. Moved to the adjust-during-render pattern.
- The mobile blog menu no longer offers Attendance to
SENIOR_EXECUTIVE_CREATIVE, whom the new ADMIN gate would bounce.
His navbar links to /Events, /Team and /Blog, so it drives the existing
pages rather than the mock-ups.
PR #12 was squash-merged into upstream main, so beta's lineage was severed from it again and beta no longer merged cleanly. Nine files conflicted; all resolved in favour of this branch, which already contains everything beta has: - markAttendance: keeps the ADMIN gate added here. - audit-nesting.mjs: keeps the CodeQL ReDoS fix, which came from beta and is already merged. - The Home revamp and its stylesheets: beta predates them. Beta did contribute two things worth keeping. It restores the global `button {}` rule in globals.scss, which the incoming revamp had dropped and which exists so buttons outside /Team and /profile/members do not fall back to the user-agent appearance. The duplicated vendor @import lines the merge produced are collapsed back to one each. Deliberately not taken: beta's GoogleSignup.jsx, which lost the invite-carry fix when PR #8 was resolved into it. That regression is the reason this branch keeps its own copy.
Taking the revamp's stylesheets while keeping our own components broke the pairing between them: a CSS Module's exported class names are part of its component's contract, and the revamp renamed them. On /Team that left 448 elements rendering with class="undefined" — the page loaded but was entirely unstyled. Reverted the 63 stylesheets whose component we kept. What remains from the revamp is the 10 stylesheets that ship alongside their own components: the Home sections (Hero, About, Feedback, Sponser, Contact, EventsSection) and the new HeroGallery, CustomCursor and CloseButton. /Team now renders 482 styled cards. All ten main routes return 200 with no unresolved module classes. Pre-existing and left alone: a handful of unresolved classes on /Login, /SignUp and in TeamCard. Those files are untouched by this merge and behave the same before it.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 21325564 | Triggered | Generic High Entropy Secret | 4ac0762 | src/sections/Home/Events/EventsSection.jsx | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
The site rendered `class="undefined"` on a large number of elements. Two
separate causes, both inherited from the original app:
1. `styles.X` where the module never defined `.X`. 62 such references
across 33 files. Scanning the original Vite app found the same set —
these are dead references to rules deleted at some point, so the
elements were already unstyled and removing the reference changes
nothing visually.
2. A `className` prop interpolated into a template literal with no
default. `Core/Input` and `Core/Text` both did this, so every caller
that omitted the prop contributed a literal "undefined" to the class
list. This is what produced the counts on /Login and /SignUp.
Handled the second by defaulting the props to "", and swept for the same
pattern everywhere else — no other component has it.
Three of the first group were real defects rather than dead weight:
- PreviewForm added `styles.noScroll` to <body> to lock scrolling while
the modal is open. No such rule existed, so the page behind the modal
stayed scrollable. Defined it, matching the declaration the original
uses for this class in its other modules.
- Footer set `id={styles.footer}` with no `#footer` rule, rendering
id="undefined" — invalid as soon as any other such element was on the
page. Dropped.
- EventsSection referenced `.loadingGrid` for its skeleton row, which
was never defined, so the placeholders stacked instead of sitting in
the events grid. Added alongside `.eventsGrid` so the layout does not
shift when real cards replace them.
Also removed two files the revamp brought in that nothing imports
(Event/components/Artwork and Disclosure) and the superseded
src/layouts/Navbar.
All ten main routes now render with no unresolved classes, server-side and
after hydration. /Team goes from 448 such elements to none.
GitGuardian flagged a Generic High Entropy Secret in EventsSection.jsx. It was the `apiKey` query parameter on a cdn.builder.io image URL used as the fallback banner when an event carries no image of its own — a key belonging to a third-party Builder.io account, not ours, almost certainly carried over from a design-tool export. Low severity in itself: Builder.io public keys are meant to be visible in image URLs, and the asset is read-only. But it is a third-party credential hardcoded in our source, it fails the secret scan on every PR, and it makes the home page depend on an outside host that is not even in `next.config.ts` remotePatterns. Replaced with a local asset behind a named constant. The same URL also appeared 10 times in src/data/FormData.json, the mock data several components fall back to, so those are replaced too — GitGuardian only saw the one in the PR diff. Verified no other occurrence of the key or host remains, and swept everything the revamp merge added for similar patterns: nothing else.
These were dropped by the earlier stylesheet correction, not by intent. When taking the revamp's stylesheets while keeping our own components turned out to break the class-name contract, the fix was to revert every stylesheet whose component we had kept — which caught Footer, the Events page and EventCard along with the rest, so only Home and the navbar ended up restyled. Taking these three as matched JSX+SCSS pairs instead. Checked first that nothing of ours is lost: - Footer.jsx and Event.jsx: no commit of ours has ever touched them. - EventCard.jsx: one, 9a1073e, which replaced a <p> that wrapped `div.price` because the invalid nesting broke SSR hydration. The incoming version does not have that nesting — confirmed with scripts/audit-nesting.mjs, which was written for exactly this bug and reports clean across all 160 files. Event.jsx imports Artwork and Disclosure, so those come back too. They were removed earlier as dead code, which they were under our Event.jsx; under this one they are used, and their classes resolve against the incoming Event.module.scss. Verified: /Events renders 634 cards from the live API with 33 register buttons, the footer marquee is present, and no route emits an unresolved class either server-side or after hydration. Backend still untouched.
Several screens quietly fell back to JSON fixtures when an API call
failed, so an outage did not show an error — it showed fabricated
records that looked real:
- Home events section and /Events/pastEvents fell back to
FormData.json, putting old test rows ("Test Payment 3", "QR TEST")
on the public site under an error banner.
- /Alumni listed sample people out of Team.json as though they were
real alumni.
- The admin member view fell back to Team.json for the roster.
- /Events counted a member's registrations against the sample file
rather than the events actually fetched.
All four now render an empty state and surface the error instead.
Two role lists were worse than stale fallbacks. ViewMember fetched
/api/user/fetchAccessTypes and then discarded the response in favour of
a hardcoded `testAccess` array, and AddMemberForm fell back to
Access.json. That file had drifted to 15 entries against the schema's
30 — missing every SENIOR_EXECUTIVE and DEPUTY_DIRECTOR role, and
offering DIRECTOR_SPONSORSHIP, OPERATION and SPONSORSHIP, none of which
exist. Both now use the enum the API returns.
Deleted FormData.json, Team.json, Access.json and user.json, along with
the dead imports of them in EventForm, EventModal, ViewEvent, Login,
GoogleLogin and GoogleSignup.
Kept: Sponser.json, Feedback.json, Carousel.json, SocialLink.json and
the live-event content files. Those are curated site copy, not stand-ins
for backend records.
Verified every route returns 200 and no page renders a sample record.
/Events shows 634 cards and 33 register buttons from the live API.
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/LiveEvents/Accordian/Accordian.jsx (1)
100-111: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd an explicit
backgroundto the "Show More" button.
showMoreButtonis not defined in this module or global styles. The inline button styles setcolor: "white"with no background, so make the background explicit to avoid user-agent fallback rendering.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/LiveEvents/Accordian/Accordian.jsx` around lines 100 - 111, Add an explicit background value to the inline style object of the Show More/Show Less button, alongside the existing color and border styles. Update only this button’s styling so its appearance does not depend on user-agent defaults.src/sections/LiveEvents/Omega/Accordion/Accordion.jsx (1)
95-105: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRestore styles for the Show More button.
styles.showMoreButtonis no longer insrc/sections/LiveEvents/Omega/Accordion/styles/Accordion.module.scss, and this button now uses only inline styles. If the previous class included responsive, hover, or focus styles, add equivalent inline/global styles so the control does not lose its presentation or interaction feedback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sections/LiveEvents/Omega/Accordion/Accordion.jsx` around lines 95 - 105, Restore the Show More button styling in the button rendered by the Accordion component, replacing the lost styles from styles.showMoreButton with equivalent supported inline or global styles. Preserve responsive presentation and hover/focus interaction feedback, while keeping the existing showAll-based click handler and label behavior unchanged.src/components/EventCard/EventCard.jsx (1)
626-666: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMake the admin controls reachable without a pointer.
The admin bar renders only when
isHoveredis true.onMouseEnterandonMouseLeavenever fire for keyboard or touch users, so Edit, Delete, and analytics are unreachable for them. Keep the bar mounted and control its visibility with CSS, and reveal it on focus as well as hover.♿ Proposed fix
- {enableEdit && isHovered && authCtx.user.access === "ADMIN" && ( - <div className={style.adminBar}> + {enableEdit && authCtx.user.access === "ADMIN" && ( + <div className={style.adminBar} data-visible={isHovered || undefined}>Then gate visibility in
EventCard.module.scssso focus also reveals it:.adminBar { opacity: 0; transition: opacity 0.2s ease; &[data-visible], &:focus-within { opacity: 1; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/EventCard/EventCard.jsx` around lines 626 - 666, Update the admin bar rendering in EventCard so it remains mounted whenever admin controls are enabled, rather than being gated by isHovered. Pass a visibility data attribute based on isHovered and update the adminBar styles in EventCard.module.scss to show it on that attribute or via :focus-within, preserving the existing hover behavior while enabling keyboard access.
🟡 Minor comments (24)
src/layouts/Profile/Sidebar/Sidebar.jsx-85-85 (1)
85-85: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPrevent duplicate Attendance entries for administrators.
When
authCtx.user.access === "ADMIN"on mobile, this branch renders Attendance again.renderAdminMenu()already renders Attendance at Lines 193-213, and both menus render at Lines 323-326. Mobile administrators therefore see two Attendance entries. Keep Attendance in one renderer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/layouts/Profile/Sidebar/Sidebar.jsx` at line 85, Update the mobile administrator branch in Sidebar so Attendance is rendered by only one menu path; remove or exclude its duplicate rendering from the branch guarded by isMobile and authCtx.user.access === "ADMIN", while preserving renderAdminMenu() and the shared menu rendering behavior.app/globals.css-744-744 (1)
744-744: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the Stylelint errors in this file.
Stylelint reports four errors. The keyframe names
fadeSlideUp(line 830) andfadeUp(line 1155) violatekeyframes-name-pattern, which requires kebab-case. ThecurrentColorvalues on lines 744 and 943 violatevalue-keyword-case, which requirescurrentcolor. If Stylelint runs in CI, the build fails.Rename the keyframes together with their
animationreferences (.testimonial-card-animateat line 827 and.animate-fade-upat line 1168).🔧 Proposed fix
-.status-dot { - background: currentColor; +.status-dot { + background: currentcolor;-.hamburger span { - background: currentColor; +.hamburger span { + background: currentcolor;-.testimonial-card-animate { - animation: fadeSlideUp 0.5s ease forwards; +.testimonial-card-animate { + animation: fade-slide-up 0.5s ease forwards; } -@keyframes fadeSlideUp { +@keyframes fade-slide-up {-@keyframes fadeUp { +@keyframes fade-up {-.animate-fade-up { - animation: fadeUp 0.6s ease forwards; +.animate-fade-up { + animation: fade-up 0.6s ease forwards; }Also applies to: 830-830, 943-943, 1155-1155
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/globals.css` at line 744, Update app/globals.css to satisfy Stylelint: change both currentColor values to lowercase currentcolor, rename the fadeSlideUp and fadeUp keyframes to kebab-case, and update the animation references in .testimonial-card-animate and .animate-fade-up to match the new names.Source: Linters/SAST tools
src/features/Modals/Profile/Admin/styles/Preview.module.scss-150-160 (1)
150-160: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the two Stylelint errors in the new comment block.
Stylelint reports
scss/double-slash-comment-empty-line-beforeat line 151 andscss/comment-no-emptyat line 153. Line 153 contains only//. If Stylelint runs in CI, the build fails.🔧 Proposed fix
} + // Applied to <body> while the preview modal is open, via // `document.body.classList.add(styles.noScroll)` in PreviewForm.jsx. -// // The component has always referenced it, but no rule existed here — in the // original app either — so `styles.noScroll` was `undefined` and the call added // a literal "undefined" class. The page behind the modal stayed scrollable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/Modals/Profile/Admin/styles/Preview.module.scss` around lines 150 - 160, Remove the standalone `//` line in the comment block above `.noScroll` and ensure the block follows the required spacing before its first comment. Preserve the explanatory comments and the `.noScroll` rule unchanged.Source: Linters/SAST tools
app/globals.scss-142-142 (1)
142-142: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the reported Stylelint errors.
app/globals.scss#L142-L142: Remove the empty//comment.app/globals.scss#L264-L264: ChangeoptimizeLegibilitytooptimizelegibility.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/globals.scss` at line 142, Fix the Stylelint violations in app/globals.scss: remove the empty comment at lines 142-142 and change optimizeLegibility to optimizelegibility at lines 264-264.Source: Linters/SAST tools
src/components/TeamCard/TeamCard.jsx-131-133 (1)
131-133: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep default TeamCard styles when no override is supplied.
src/views/Alumni/Alumni.jsxrendersTeamCardwithoutcustomStyles. These removals leave the designation, social links, and buttons without their default module classes. Append optional custom classes instead of replacing the defaults.Also applies to: 149-151, 161-163, 173-173, 242-242
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/TeamCard/TeamCard.jsx` around lines 131 - 133, Update the TeamCard className expressions for the designation, social links, and buttons to retain their existing default CSS module classes, appending customStyles overrides only when provided. Apply this consistently to the referenced elements, including the teamMemberBackh5 styling, while keeping TeamCard rendering without customStyles fully styled.src/sections/Profile/Admin/View/ViewEvent/VIewEvent.jsx-111-113 (1)
111-113: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRestore visual styling for the API-error message.
The error message renders in an unstyled
<div>becausestyles.erroris not applied and there is noerrorclass inViewEvent.module.scss. Use an existing error/error-container class or restore the styling so the warning remains user-visible and consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sections/Profile/Admin/View/ViewEvent/VIewEvent.jsx` around lines 111 - 113, Update the error rendering in ViewEvent to apply the existing error or error-container styling to the element displaying error.message. If no suitable class exists, add or restore the corresponding error style in ViewEvent.module.scss, ensuring the API warning remains visually prominent and consistent.src/views/Event/Event.jsx-34-34 (1)
34-34: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInitialize
parentEventCountas a number.Line 34 initializes the state with
[], and Line 139 assigns a number. On the first renderparentEventCount === 0is false, soshowPrerequisiteNoticeis false until the effect runs.🔧 Proposed fix
- const [parentEventCount, setParentEventCount] = useState([]); + const [parentEventCount, setParentEventCount] = useState(0);Also applies to: 139-139
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/Event/Event.jsx` at line 34, Initialize the parentEventCount state in Event using the numeric default expected by showPrerequisiteNotice, rather than an empty array, so the initial render matches the number assigned by the effect at line 139.src/views/Event/styles/Event.module.scss-1-5 (1)
1-5: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the two Stylelint errors.
Stylelint reports an empty comment at Line 2 and the deprecated
clipproperty at Line 26. Useclip-pathfor the screen-reader-only helper.🎨 Proposed fixes
// Events listing. -// // Mobile-first: a single column of cards, widening to a fluid grid. Depth comesoverflow: hidden; - clip: rect(0, 0, 0, 0); + clip-path: inset(50%); white-space: nowrap;Also applies to: 20-29
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/Event/styles/Event.module.scss` around lines 1 - 5, Remove the empty comment line in the header of Event.module.scss, and update the screen-reader-only helper’s deprecated clip declaration to use the clip-path property while preserving its existing clipping behavior.Source: Linters/SAST tools
src/views/Event/Event.jsx-213-216 (1)
213-216: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThe page renders no heading, and the new masthead styles are unused.
The shell contains only blank lines before the loading branch.
Event.module.scssadds.masthead,.mastheadText,.mastheadArt,.backLink,.eyebrow,.title, and.lede, but no element in this file uses them. The events page therefore has noh1and no page title, so the document outline starts ath2. Render the masthead, or remove the unused rules.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/Event/Event.jsx` around lines 213 - 216, Update the Event page render inside the style.shell container to use the new masthead styles by adding a masthead with its associated text, eyebrow, h1 title, lede, artwork, and back-link elements before the loading branch. Ensure the page has a descriptive h1 and the existing style symbols are applied, rather than leaving the shell blank or removing the styles.src/components/EventCard/styles/EventCard.module.scss-1-6 (1)
1-6: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the three Stylelint errors.
Stylelint reports an empty comment at Line 2, a non-kebab-case keyframe name at Line 474, and
currentColorcasing at Line 106. If Stylelint runs in CI, these fail the lint job. The same keyframe name pattern applies tosrc/views/Event/styles/Event.module.scss, which usespulse.🎨 Proposed fixes
// Event card. -// // One surface, one hairline border, one image. Depth comes from an inset bevel.metaDot { - background-color: currentColor; + background-color: currentcolor;.skeleton { - animation: cardPulse 1.6s var(--ease) infinite; + animation: card-pulse 1.6s var(--ease) infinite; }-@keyframes cardPulse { +@keyframes card-pulse {Also applies to: 106-106, 474-482
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/EventCard/styles/EventCard.module.scss` around lines 1 - 6, Fix the three Stylelint violations in the EventCard styles: remove the empty comment line, use the linter-approved lowercase form of currentColor, and rename the pulse keyframe to a kebab-case name while updating every reference. Apply the same keyframe naming change to the corresponding pulse animation in Event.module.scss.Source: Linters/SAST tools
src/views/Event/Event.jsx-334-359 (1)
334-359: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the event route segment for past event cards.
/Events/pastEvents/page.jsxexists for the “View all” page, but there is no/pastEvents/[eventId]route segment because event details use/Events/[eventId]. Setmodalpath="/Events/"in bothsrc/views/Event/Event.jsxandsrc/views/Event/PastEvent.jsxso past event detail links resolve.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/Event/Event.jsx` around lines 334 - 359, Update the past-event EventCard configuration in the Event view and PastEvent view to use modalpath="/Events/" instead of the past-events listing path. Keep the existing past-event rendering and detail-link behavior unchanged so cards resolve through the shared /Events/[eventId] route.src/components/HeroGallery/styles/HeroGallery.module.scss-242-251 (1)
242-251: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDisable navigation-control motion for reduced-motion users.
The reduced-motion rule does not include
.navBtn. Its hover and active states still animate and translate. Disable its transitions and transform changes in this media query.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/HeroGallery/styles/HeroGallery.module.scss` around lines 242 - 251, Update the prefers-reduced-motion block in HeroGallery styles to include .navBtn, disabling its transitions and preventing hover/active transform changes while preserving the existing reduced-motion behavior for .card, .card img, .railDot, and .active img.src/components/CustomCursor/CustomCursor.jsx-23-35 (1)
23-35: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winApply translation and scale in one transform.
Line 24 sets an inline
transform. It overrides thetransform: scale(1.6)rule inCustomCursor.module.scss, so the cursor dot does not grow on hover. Compose both effects in one transform, or use CSS custom properties for the coordinates and scale.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/CustomCursor/CustomCursor.jsx` around lines 23 - 35, Update the cursor transform logic in the CustomCursor mouse-move handler so position translation and hover scaling are composed rather than letting the inline transform override the stylesheet scale. Preserve the existing coordinate updates and ensure the isHovered state controls the cursor’s scale.app/components/Navbar.tsx-30-33 (1)
30-33: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPrevent false active states for prefix routes.
Line 33 marks
/events-archiveas active for the/Eventslink. Remove the finalstartsWith(cleanHref)check. The preceding child-route check already handles/events/....Proposed fix
- return cleanPath === cleanHref || cleanPath.startsWith(`${cleanHref}/`) || cleanPath.startsWith(cleanHref); + return cleanPath === cleanHref || cleanPath.startsWith(`${cleanHref}/`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/Navbar.tsx` around lines 30 - 33, Update the active-route logic in the Navbar component by removing the final `cleanPath.startsWith(cleanHref)` condition from the return expression. Preserve exact matches and child-route matching via `cleanPath.startsWith(`${cleanHref}/`)`, along with the existing blog-specific handling.app/components/Navbar.tsx-131-149 (1)
131-149: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExpose the mobile menu state.
The toggle button changes the menu state but does not expose that state to assistive technology. Add
aria-expanded={mobileOpen}to the button.Proposed fix
<button className="hamburger-button" onClick={() => setMobileOpen(!mobileOpen)} aria-label="Toggle mobile navigation menu" + aria-expanded={mobileOpen} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/Navbar.tsx` around lines 131 - 149, Add aria-expanded={mobileOpen} to the hamburger button in the Navbar component so assistive technology reflects the current mobile menu state.src/components/CustomCursor/CustomCursor.jsx-70-72 (1)
70-72: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winKeep the initial render independent of
window.Hydration renders the cursor markup server-side, but a coarse-pointer client returns
nulland skips attaching the mouse event listeners. Render the same initial output on the server and client, then store the pointer capability after mount before applyingmatchMedia.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/CustomCursor/CustomCursor.jsx` around lines 70 - 72, Update the CustomCursor initial render so it does not read window.matchMedia or return null based on pointer capability during render. Store the pointer capability in state initialized consistently for server and client, determine it inside the mount effect, and only then apply the coarse-pointer behavior and mouse event listeners.Source: Linters/SAST tools
src/sections/Home/Events/EventsSection.jsx-47-54 (1)
47-54: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe heading contradicts the past-event fallback.
If no live or upcoming events exist, lines 47-54 select past events and the section still renders the heading "LIVE & UPCOMING EVENTS". Visitors then read past events as upcoming. Track which branch produced the list and change the heading accordingly.
🐛 Proposed fix
const [upcomingEvents, setUpcomingEvents] = useState([]); const [loading, setLoading] = useState(true); + const [showingPast, setShowingPast] = useState(false);} else { // Sort past events by date descending (most recent / newest past dates first) + setShowingPast(true); sortedEvents = [...pastEvents].sort((a, b) => {<div className={styles.heading}> <h2> - LIVE <span className={styles.highlight}>& UPCOMING</span> EVENTS + {showingPast ? ( + <>PAST <span className={styles.highlight}>EVENTS</span></> + ) : ( + <>LIVE <span className={styles.highlight}>& UPCOMING</span> EVENTS</> + )} </h2>Also applies to: 108-113
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sections/Home/Events/EventsSection.jsx` around lines 47 - 54, Track whether the displayed list comes from the past-events fallback in the EventsSection component, and update the section heading to reflect that source. Keep “LIVE & UPCOMING EVENTS” for live or upcoming results, but render an appropriate past-events heading when the fallback branch selects pastEvents, including the corresponding heading logic at the other referenced location.src/sections/Home/Contact/Contact.jsx-68-68 (1)
68-68: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the
idvalue that contains a space.
id="Contact Us"is not addressable by a URL fragment and is not a valid CSS identifier.src/views/Home/Home.jsxalready wraps this component in<section id="Contact">, so anchor navigation targets#Contact. Remove the inneridto avoid a second, unusable anchor.🐛 Proposed fix
- <section id="Contact Us" className={styles.section} aria-labelledby="contact-heading"> + <section className={styles.section} aria-labelledby="contact-heading">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sections/Home/Contact/Contact.jsx` at line 68, Remove the inner id attribute from the section in the Contact component, preserving the existing className and aria-labelledby. Rely on the parent Home section’s id="Contact" as the sole anchor target.app/components/ScrollRevealWrapper.tsx-14-33 (1)
14-33: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLatch the revealed state instead of re-hiding content.
The callback assigns
entry.isIntersectingdirectly. When a wrapped section scrolls out of view,isVisiblereturns tofalseand the section fades back toopacity-0with a blur. The negativerootMarginmakes this happen while parts of the section are still on screen. The wrapped Contact form and Feedback list then animate repeatedly during normal scrolling.Set the state once, then stop observing.
🐛 Proposed fix to latch visibility
const observer = new IntersectionObserver( ([entry]) => { - setIsVisible(entry.isIntersecting); + if (!entry.isIntersecting) return; + setIsVisible(true); + observer.unobserve(entry.target); }, { threshold: 0.05, rootMargin: "-6% 0px -6% 0px", } ); const current = domRef.current; if (current) observer.observe(current); return () => { - if (current) observer.unobserve(current); + observer.disconnect(); }; }, [instant]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/ScrollRevealWrapper.tsx` around lines 14 - 33, Update the IntersectionObserver callback in ScrollRevealWrapper’s useEffect to latch visibility by setting isVisible to true only when entry.isIntersecting, then stop observing the current element. Preserve the existing initial hidden state and cleanup behavior while preventing sections from re-hiding or animating repeatedly after first reveal.src/sections/Home/Sponser/styles/Sponser.module.scss-115-115 (1)
115-115: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStylelint errors across the new stylesheets will fail the lint job. The new files use camelCase keyframe names and leave blank lines before declarations. The project's Stylelint config enforces
keyframes-name-pattern(kebab-case) anddeclaration-empty-line-before. Rename each keyframe in both the@keyframesrule and everyanimationshorthand that references it, and delete the stray blank lines.
src/sections/Home/Sponser/styles/Sponser.module.scss#L115-L115: renamesponsorScrolltosponsor-scrolland update theanimationdeclarations at lines 85 and 151.src/layouts/Footer/styles/Footer.module.scss#L242-L242: renamefooterMarqueetofooter-marqueeand update theanimationdeclaration at line 215.src/sections/Home/Events/styles/EventsSection.module.scss#L206-L206: renamepulseSkeletontopulse-skeletonand update theanimationdeclaration at line 203.src/sections/Home/Hero/styles/Hero.module.scss#L24-L26: remove the blank lines before the declarations at lines 26, 48, and 105.src/sections/Home/Contact/styles/Contact.module.scss#L57-L59: remove the blank line before the declaration at line 59.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sections/Home/Sponser/styles/Sponser.module.scss` at line 115, Update src/sections/Home/Sponser/styles/Sponser.module.scss at lines 115 and 85/151 by renaming the sponsorScroll keyframe and animation references to sponsor-scroll; update src/layouts/Footer/styles/Footer.module.scss at lines 242 and 215 from footerMarquee to footer-marquee; update src/sections/Home/Events/styles/EventsSection.module.scss at lines 206 and 203 from pulseSkeleton to pulse-skeleton. Remove the blank lines before declarations in src/sections/Home/Hero/styles/Hero.module.scss at lines 26, 48, and 105, and in src/sections/Home/Contact/styles/Contact.module.scss at line 59.Source: Linters/SAST tools
src/sections/Home/Contact/styles/Contact.module.scss-94-98 (1)
94-98: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep a visible focus indicator on the form controls.
The rule removes the native outline and signals focus only through
border-color. A 1px border-color change is easy to miss, and its contrast against--borderis not guaranteed. Keyboard users then lose track of the focused field. Add an offset outline or a ring.♻️ Proposed fix
.formGroup input:focus, .formGroup textarea:focus { border-color: var(--border-focus); - outline: none; + outline: 2px solid var(--border-focus); + outline-offset: 2px; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sections/Home/Contact/styles/Contact.module.scss` around lines 94 - 98, Update the `.formGroup input:focus, .formGroup textarea:focus` rule to retain a clearly visible keyboard-focus indicator by adding a sufficiently contrasting offset outline or focus ring alongside the existing border-color change, while preserving the current focus styling.src/sections/Home/Feedback/styles/Feedback.module.scss-79-85 (1)
79-85: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMove marquee gaps and horizontal track padding onto each half so
-50%remains on a copy boundary. All three tracks use duplicated content, but.feedbacks,.track, and.marqueeTrackinclude the spacing between the copies inmax-content;translateX(-50%)therefore lands inside the duplicated gap/padding and the first frame jumps. Move the inter-copy spacing onto.feedbackCard/.sponser_card/.marqueeItem, or wrap the duplicated halves and animate by the width/value of a single copy.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sections/Home/Feedback/styles/Feedback.module.scss` around lines 79 - 85, Update the duplicated marquee tracks in src/sections/Home/Feedback/styles/Feedback.module.scss lines 79-85, src/sections/Home/Sponser/styles/Sponser.module.scss lines 79-86, and src/layouts/Footer/styles/Footer.module.scss lines 208-217 so inter-copy gaps and horizontal padding are applied per item/card (.feedbackCard, .sponser_card, and .marqueeItem) or otherwise excluded from the animated track width; preserve -50% as a boundary between identical copies in all three tracks.src/sections/Home/Sponser/styles/Sponser.module.scss-107-113 (1)
107-113: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
object-fit: containfor sponsor logos.
.sponser_cardis a 5.5rem circle withoverflow: hidden. Withobject-fit: cover, non-square logos are cropped and wordmarks become unreadable. Usecontainwith padding so the full logo stays visible. Thetransition: opacitydeclaration has no matching hover rule, so it has no effect.♻️ Proposed fix
.SponserCard_image { height: 100%; width: 100%; - object-fit: cover; - opacity: 1; - transition: opacity 0.3s var(--ease); + object-fit: contain; + padding: 0.65rem; + box-sizing: border-box; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sections/Home/Sponser/styles/Sponser.module.scss` around lines 107 - 113, Update the .SponserCard_image styles to use object-fit: contain and add padding so complete non-square sponsor logos remain visible within the circular card. Remove the unused transition: opacity declaration because no matching hover state exists.src/sections/Home/Hero/styles/Hero.module.scss-108-119 (1)
108-119: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd
-webkit-background-clip: textto gradient text classes. Three affected rules only usebackground-clip: text; add the WebKit prefix alongside it for Safari 15.4 and earlier support wherecolor: transparentwould otherwise make the text disappear.
src/sections/Home/Hero/styles/Hero.module.scss#L108-L119: add-webkit-background-clip: textto.typing.src/sections/Home/Hero/styles/Hero.module.scss#L130-L135: add-webkit-background-clip: textto.accent.src/sections/Home/Feedback/styles/Feedback.module.scss#L51-L55: add-webkit-background-clip: textto.heading span.src/sections/Home/Feedback/styles/Feedback.module.scss#L165-L172: add-webkit-background-clip: textto.feedbackAuthor.src/sections/Home/Contact/styles/Contact.module.scss#L45-L49: add-webkit-background-clip: textto.highlight.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sections/Home/Hero/styles/Hero.module.scss` around lines 108 - 119, Add the WebKit-prefixed background clipping declaration alongside background-clip: text in .typing and .accent in src/sections/Home/Hero/styles/Hero.module.scss (lines 108-119 and 130-135), .heading span and .feedbackAuthor in src/sections/Home/Feedback/styles/Feedback.module.scss (lines 51-55 and 165-172), and .highlight in src/sections/Home/Contact/styles/Contact.module.scss (lines 45-49).
🧹 Nitpick comments (15)
src/components/LiveEvents/Accordian/Accordian.jsx (1)
41-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify the class expression to
styles.card.The template literal interpolates an empty string and adds a trailing space to the rendered
classattribute. Move the explanation to a normal code comment.♻️ Proposed cleanup
<motion.div key={index} - className={`${styles.card} ${"" /* .activeCard is not defined by this module, as in the original */}`} + // `.activeCard` is not defined by this module, as in the original. + className={styles.card} ref={ref}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/LiveEvents/Accordian/Accordian.jsx` around lines 41 - 43, Update the motion.div className in the Accordian component to use styles.card directly instead of an interpolated template literal. Move the explanation about the undefined activeCard style to a regular code comment outside the class expression.src/views/Blog/FullBlog.jsx (1)
110-124: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReturn the loading and error content without the empty wrapper divs.
Both wrappers now carry no class and no attributes. They add markup without purpose.
♻️ Proposed cleanup
if (isLoading) { - return ( - <div> - <ComponentLoading /> - </div> - ); + return <ComponentLoading />; } if (error) { - return ( - <div> - <p>{error}</p> - </div> - ); + return <p>{error}</p>; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/Blog/FullBlog.jsx` around lines 110 - 124, Remove the unnecessary wrapper divs from the isLoading and error branches in FullBlog, returning ComponentLoading and the error paragraph directly while preserving their existing conditional behavior and content.src/components/BlogCard/BlogCard.jsx (1)
88-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDelete the no-op
expandDescriptionbranch.Line 90 appends an empty template literal, so the statement has no effect. It also makes
expandDescriptionlook like it still affects the card class. Remove the line. Keep theexpandDescriptionprop only if a caller still needs it for another purpose.♻️ Proposed cleanup
const getCardClass = () => { let cardClass = styles.card; - if (expandDescription) cardClass += ``; if (props.isRecentCard) cardClass += ` ${styles.recentCard}`;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/BlogCard/BlogCard.jsx` around lines 88 - 96, Remove the no-op expandDescription conditional from getCardClass, leaving the existing card-type and recent-card class logic unchanged. Retain the expandDescription prop only if it is used elsewhere in BlogCard for a separate purpose.app/globals.css (1)
1154-1194: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a
prefers-reduced-motionguard for the infinite animations.This file adds three animations that never stop:
pulse-dot(line 549),live-pulse(lines 681 and 1031), andscroll-left(line 766). Continuous motion can cause discomfort for users who request reduced motion. It also blocks reading of the sponsor carousel content.Add a media query at the end of the file.
♿ Proposed addition
+ +/* ─── Reduced Motion ────────────────────────────────────────────── */ +@media (prefers-reduced-motion: reduce) { + + html { + scroll-behavior: auto; + } + + .fed-label-dot, + .status-dot--live, + .fed-event-card--live, + .event-grid-card--live, + .sponsor-carousel-track, + .testimonial-card-animate, + .animate-fade-up { + animation: none !important; + } + + .animate-delay-100, + .animate-delay-200, + .animate-delay-300, + .animate-delay-400, + .animate-delay-500 { + opacity: 1; + animation-delay: 0s; + } +}Note that
.animate-delay-*setopacity: 0and rely on the animation to reveal content. If the animation is disabled without theopacity: 1override, the content stays invisible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/globals.css` around lines 1154 - 1194, Add an end-of-file prefers-reduced-motion media query covering the infinite animation selectors pulse-dot, live-pulse, and scroll-left, disabling their animation while preserving readable sponsor content. Within the same guard, override .animate-delay-100 through .animate-delay-500 opacity to 1 so delayed content remains visible when motion is reduced.src/features/Modals/Profile/Admin/styles/Preview.module.scss (1)
158-160: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the dead
.no-scrollrule.
PreviewForm.jsxusesstyles.noScroll, and thenoScrollrule is already defined. The kebab-case rule is not referenced and should be deleted.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/Modals/Profile/Admin/styles/Preview.module.scss` around lines 158 - 160, Remove the unused kebab-case .no-scroll rule from the stylesheet, while preserving the referenced .noScroll rule used by PreviewForm.jsx.src/components/EventCard/styles/EventCard.module.scss (1)
346-349: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
.techAccentrule.The featured SVG in
EventCard.jsxdoes not referencestyle.techAccent. The rule is dead code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/EventCard/styles/EventCard.module.scss` around lines 346 - 349, Remove the unused .techAccent rule from the EventCard styles, since the featured SVG in EventCard.jsx does not reference style.techAccent.src/views/Event/components/Artwork.jsx (1)
1-1: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueThe
"use client"directive is not needed here.These components render static SVG markup with no hooks, state, or event handlers. They can render as server components. Removing the directive keeps them out of the client bundle. Client parents can still import them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/Event/components/Artwork.jsx` at line 1, Remove the "use client" directive from Artwork.jsx, keeping the static SVG component implementation unchanged so it remains a server component and is excluded from the client bundle.src/components/EventCard/EventCard.jsx (1)
144-149: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRemove the duplicate countdown interval.
The effect at Lines 75-81 already calls
calculateRemainingTimeevery second wheninfo.regDateAndTimeexists. This second effect starts another 1-second interval for every card, and it runs even wheninfo.regDateAndTimeis missing. In that caseparsereturns an invalid date and the countdown state is set to an empty string on every tick.♻️ Proposed cleanup
- useEffect(() => { - calculateRemainingTime(); // Initial calculation - const intervalId = setInterval(calculateRemainingTime, 1000); - - return () => clearInterval(intervalId); - }, []); -🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/EventCard/EventCard.jsx` around lines 144 - 149, Remove the later useEffect that invokes calculateRemainingTime and creates intervalId, since the existing effect already manages the countdown when info.regDateAndTime is available. Preserve the existing cleanup and calculation behavior in the original effect.src/sections/Home/Sponser/Sponser.jsx (1)
7-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused
loadedstate.
setLoadedruns for every image, butloadedis never read. The className expression concatenates an empty string with an explanatory comment. Each image load therefore triggers a re-render with no visual effect. Delete the state and simplify the class name.♻️ Proposed cleanup
const SponserCard = ({ image }) => { - const [loaded, setLoaded] = useState(false); - return ( <div className={styles.sponser_card}> <img src={image.image} - className={`${styles.SponserCard_image} ${"" /* .loaded is not defined by this module, as in the original */}`} + className={styles.SponserCard_image} alt={image.title || "Sponsor logo"} - onLoad={() => setLoaded(true)} loading="lazy" draggable={false} /> </div> ); };Remove
useStatefrom the React import if no other code in the file uses it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sections/Home/Sponser/Sponser.jsx` around lines 7 - 22, Remove the unused loaded state and its setLoaded onLoad handler from SponserCard, simplify the image className to styles.SponserCard_image, and remove useState from the React import if no other code in the file uses it.src/sections/Home/Feedback/Feedback.jsx (1)
12-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove
FeedbackCardout ofFeedback.
FeedbackCardis redefined on every render ofFeedback. React treats each definition as a new component type, so it unmounts and remounts all cards and resetsisExpanded. Any expanded testimonial collapses when the parent re-renders. Declare the component at module scope.♻️ Proposed refactor
-const Feedback = () => { - const feedbacksRef = useRef(null); - const containerRef = useRef(null); - - const FeedbackCard = ({ quote }) => { +const FeedbackCard = ({ quote }) => { const [isExpanded, setIsExpanded] = useState(false); const long = quote.quote.length > 160; const truncatedQuote = long ? `${quote.quote.substring(0, 150)}…` : quote.quote; return ( ... ); - }; +}; + +const Feedback = () => { + const feedbacksRef = useRef(null); + const containerRef = useRef(null);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sections/Home/Feedback/Feedback.jsx` around lines 12 - 41, Move the FeedbackCard component definition from inside Feedback to module scope so React preserves its component identity across parent renders. Keep its quote prop, expansion state, truncation logic, and rendering behavior unchanged.app/components/ScrollRevealWrapper.tsx (1)
38-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRespect
prefers-reduced-motion.This wrapper animates transform, opacity, and blur for 1000ms on four homepage sections.
src/sections/Home/About/About.jsxalready checksprefers-reduced-motion. Align this component with that behavior by using themotion-reducevariant.♻️ Proposed reduced-motion handling
className={`transition-all duration-1000 ease-[cubic-bezier(0.16,1,0.3,1)] transform will-change-[transform,opacity,filter] ${ isVisible ? "opacity-100 translate-y-0 scale-100 filter blur-0" - : "opacity-0 translate-y-16 scale-[0.97] filter blur-[4px]" + : "opacity-0 translate-y-16 scale-[0.97] filter blur-[4px] motion-reduce:opacity-100 motion-reduce:translate-y-0 motion-reduce:scale-100 motion-reduce:blur-0 motion-reduce:transition-none" }`}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/ScrollRevealWrapper.tsx` around lines 38 - 42, Update the className in ScrollRevealWrapper to apply the Tailwind motion-reduce variant, disabling transition, transform, and blur effects for users who prefer reduced motion while preserving the existing animation classes and visibility behavior otherwise.src/components/HeroGallery/HeroGallery.jsx (2)
101-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the tablist roles or complete the pattern.
The rail uses
role="tablist"androle="tab", but no element hasrole="tabpanel"and no button hasaria-controls. Screen readers then announce tabs that control nothing. Plain buttons witharia-labelandaria-currentdescribe this pagination correctly.♻️ Proposed markup change
- <div className={styles.rail} role="tablist" aria-label="Gallery slides"> + <div className={styles.rail} role="group" aria-label="Gallery slides"> {images.map((_, index) => ( <button key={index} type="button" - role="tab" - aria-selected={index === current} + aria-current={index === current ? "true" : undefined} className={`${styles.railDot} ${ index === current ? styles.railDotActive : "" }`}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/HeroGallery/HeroGallery.jsx` around lines 101 - 115, Update the pagination rail in HeroGallery by removing the tablist and tab roles, replacing aria-selected with aria-current on the active button, and retaining the existing labeled button and goTo behavior.
139-146: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the stale
propTypesdeclaration with static types.This component is a function component in React 19.2.4, so
HeroGallery.propTypesis no longer validated. Remove the unusedprop-typesimport or use TypeScript/JSDoc types to keep the props contract in a maintained runtime-checked path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/HeroGallery/HeroGallery.jsx` around lines 139 - 146, Replace the obsolete HeroGallery.propTypes declaration with a maintained static props contract, preferably TypeScript or JSDoc, covering required images and each image’s required image string plus optional title. Remove the prop-types import if it is only used by this declaration, and keep the HeroGallery component’s existing behavior unchanged.src/sections/Home/About/About.jsx (1)
54-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCouple
STAGEStoblocks.
STAGESandblocksare parallel arrays with no enforced relationship. If a fourth block is added,STAGES[3]isundefinedand the destructure insidegetPanelStylethrows a TypeError. Derive the stages fromblocks.length, or guard the lookup.♻️ Minimal guard
panelRefs.current.forEach((p, i) => { if (!p) return; - const { opacity, ty, pe } = getPanelStyle(STAGES[i], progress); + const stage = STAGES[i]; + if (!stage) return; + const { opacity, ty, pe } = getPanelStyle(stage, progress);Also applies to: 122-129
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sections/Home/About/About.jsx` around lines 54 - 58, Update the STAGES lookup used by getPanelStyle so it remains safe when blocks contains more entries than STAGES, deriving or selecting stage data from blocks.length or guarding missing entries before destructuring. Preserve the existing animation behavior for defined stages and ensure added blocks do not cause a TypeError.src/sections/Home/Events/styles/EventsSection.module.scss (1)
199-210: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a reduced-motion guard for the skeleton pulse.
.skeletonCardruns an infinite opacity animation. Other sections in this PR disable their animations underprefers-reduced-motion. Apply the same rule here for consistency.♻️ Proposed addition
`@keyframes` pulseSkeleton { 0% { opacity: 0.4; } 50% { opacity: 0.8; } 100% { opacity: 0.4; } } + +@media (prefers-reduced-motion: reduce) { + .skeletonCard { + animation: none; + opacity: 0.6; + } +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sections/Home/Events/styles/EventsSection.module.scss` around lines 199 - 210, Update the .skeletonCard animation styles to disable the pulseSkeleton animation when the user’s prefers-reduced-motion setting is reduce, matching the reduced-motion handling used by other sections while preserving the existing animation for users without that preference.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0c187cec-280f-49e4-96b2-4399479975b6
⛔ Files ignored due to path filters (19)
app/icon.pngis excluded by!**/*.pngpublic/assets/design-2.pngis excluded by!**/*.pngpublic/assets/design-3.pngis excluded by!**/*.pngpublic/assets/design-4.pngis excluded by!**/*.pngpublic/assets/design.pngis excluded by!**/*.pngpublic/assets/fedrick.pngis excluded by!**/*.pngpublic/contact-envelope.pngis excluded by!**/*.pngpublic/fedkiit-logo.pngis excluded by!**/*.pngpublic/fedkiit-logo.svgis excluded by!**/*.svgpublic/fedkiit-mascot.pngis excluded by!**/*.pngpublic/grid-bg.pngis excluded by!**/*.pngpublic/to-fed-envelope-clean-transparent-fixed.pngis excluded by!**/*.pngpublic/to-fed-envelope-clean-transparent.pngis excluded by!**/*.pngpublic/to-fed-envelope-fixed.pngis excluded by!**/*.pngpublic/to-fed-envelope-seamless.pngis excluded by!**/*.pngpublic/to-fed-envelope-transparent-fixed.pngis excluded by!**/*.pngpublic/to-fed-envelope-transparent.pngis excluded by!**/*.pngpublic/to-fed-envelope.pngis excluded by!**/*.pngsrc/assets/images/contact.pngis excluded by!**/*.png
📒 Files selected for processing (74)
.gitignoreapp/(main)/layout.jsxapp/components/Navbar.tsxapp/components/ScrollRevealWrapper.tsxapp/globals.cssapp/globals.scssapp/layout.tsxapp/not-found.tsxsrc/assets/styles/Global.scsssrc/authentication/SignUp/SignUP.jsxsrc/components/BlogCard/BlogCard.jsxsrc/components/Carousel/Carousel.jsxsrc/components/CloseButton/CloseButton.jsxsrc/components/CloseButton/styles/CloseButton.module.scsssrc/components/Core/Input.jsxsrc/components/Core/Text.jsxsrc/components/CustomCursor/CustomCursor.jsxsrc/components/CustomCursor/styles/CustomCursor.module.scsssrc/components/EventCard/EventCard.jsxsrc/components/EventCard/styles/EventCard.module.scsssrc/components/HeroGallery/HeroGallery.jsxsrc/components/HeroGallery/styles/HeroGallery.module.scsssrc/components/LiveEvents/Accordian/Accordian.jsxsrc/components/TeamCard/TeamCard.jsxsrc/components/index.jsxsrc/data/FormData.jsonsrc/features/Modals/EditProfile/EditProfile.jsxsrc/features/Modals/Event/EventModal/EventModal.jsxsrc/features/Modals/Event/EventStats/EventStats.jsxsrc/features/Modals/Profile/Admin/SectionModal.jsxsrc/features/Modals/Profile/Admin/styles/Preview.module.scsssrc/layouts/Blog/LeftSidebar/LeftSidebar.jsxsrc/layouts/Blog/RightSidebar/RightSidebar.jsxsrc/layouts/Footer/Footer.jsxsrc/layouts/Footer/styles/Footer.module.scsssrc/layouts/Navbar/Navbar.tsxsrc/layouts/Navbar/styles/Navbar.module.scsssrc/layouts/Profile/ProfileLayout/ProfileLayout.jsxsrc/layouts/Profile/Sidebar/Sidebar.jsxsrc/sections/Home/About/About.jsxsrc/sections/Home/About/styles/About.module.scsssrc/sections/Home/Contact/Contact.jsxsrc/sections/Home/Contact/styles/Contact.module.scsssrc/sections/Home/Events/EventsSection.jsxsrc/sections/Home/Events/styles/EventsSection.module.scsssrc/sections/Home/Feedback/Feedback.jsxsrc/sections/Home/Feedback/styles/Feedback.module.scsssrc/sections/Home/Hero/Hero.jsxsrc/sections/Home/Hero/styles/Hero.module.scsssrc/sections/Home/Sponser/Sponser.jsxsrc/sections/Home/Sponser/styles/Sponser.module.scsssrc/sections/Home/index.jsxsrc/sections/LiveEvents/Omega/Accordion/Accordion.jsxsrc/sections/LiveEvents/Omega/Event/Event.jsxsrc/sections/LiveEvents/Omega/FedShow/FedShow.jsxsrc/sections/LiveEvents/Omega/Sponsors/Sponsors.jsxsrc/sections/Profile/Admin/Form/MemberForm/AddMemberForm.jsxsrc/sections/Profile/Admin/View/ViewEvent/VIewEvent.jsxsrc/sections/Profile/Admin/View/ViewMember/ViewMember.jsxsrc/sections/Profile/General/CertificatesView/CertificatesView.jsxsrc/sections/Profile/General/EventsView/EventsView.jsxsrc/views/Alumni/Alumni.jsxsrc/views/AttendancePage/AttendancePage.jsxsrc/views/Blog/Blog.jsxsrc/views/Blog/FullBlog.jsxsrc/views/Event/Event.jsxsrc/views/Event/PastEvent.jsxsrc/views/Event/components/Artwork.jsxsrc/views/Event/components/Disclosure.jsxsrc/views/Event/styles/Event.module.scsssrc/views/Home/Home.jsxsrc/views/Home/styles/Home.module.scsssrc/views/LiveEvents/Omega/Omega.jsxsrc/views/Social/Social.jsx
💤 Files with no reviewable changes (6)
- src/features/Modals/Profile/Admin/SectionModal.jsx
- src/layouts/Navbar/Navbar.tsx
- src/layouts/Navbar/styles/Navbar.module.scss
- src/features/Modals/Event/EventStats/EventStats.jsx
- src/sections/LiveEvents/Omega/Sponsors/Sponsors.jsx
- src/layouts/Blog/LeftSidebar/LeftSidebar.jsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/views/AttendancePage/AttendancePage.jsx
Comment on lines
+452
to
+474
| .fed-nav-link { | ||
| display: inline-flex; | ||
| align-items: center; | ||
| padding: 0.45rem 1.15rem; | ||
| border-radius: 9999px; | ||
| font-size: 0.9rem; | ||
| font-weight: 500; | ||
| color: #999999; | ||
| text-decoration: none; | ||
| transition: color 0.2s ease, background 0.2s ease; | ||
| white-space: nowrap; | ||
| } | ||
|
|
||
| .fed-nav-link:hover { | ||
| color: var(--fed-text); | ||
| background: rgba(255, 255, 255, 0.08); | ||
| } | ||
|
|
||
| .fed-nav-link--active { | ||
| background: rgba(255, 255, 255, 0.14); | ||
| color: var(--fed-text); | ||
| font-weight: 600; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the duplicate .fed-nav-link rule set.
Lines 278-301 already define .fed-nav-link, .fed-nav-link:hover, and .fed-nav-link--active. Lines 452-474 redefine the same three selectors with different values. The later block wins, so the earlier block is dead code and the two sets of values disagree (padding 1.25rem vs 1.15rem, color #a6a6a6 vs #999999, active background #2a2a2a vs rgba(255, 255, 255, 0.14), active font-weight 700 vs 600).
Keep one definition with the intended values.
♻️ Proposed cleanup
-.fed-nav-link {
- display: inline-flex;
- align-items: center;
- padding: 0.45rem 1.15rem;
- border-radius: 9999px;
- font-size: 0.9rem;
- font-weight: 500;
- color: `#999999`;
- text-decoration: none;
- transition: color 0.2s ease, background 0.2s ease;
- white-space: nowrap;
-}
-
-.fed-nav-link:hover {
- color: var(--fed-text);
- background: rgba(255, 255, 255, 0.08);
-}
-
-.fed-nav-link--active {
- background: rgba(255, 255, 255, 0.14);
- color: var(--fed-text);
- font-weight: 600;
-}
-📝 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
| .fed-nav-link { | |
| display: inline-flex; | |
| align-items: center; | |
| padding: 0.45rem 1.15rem; | |
| border-radius: 9999px; | |
| font-size: 0.9rem; | |
| font-weight: 500; | |
| color: #999999; | |
| text-decoration: none; | |
| transition: color 0.2s ease, background 0.2s ease; | |
| white-space: nowrap; | |
| } | |
| .fed-nav-link:hover { | |
| color: var(--fed-text); | |
| background: rgba(255, 255, 255, 0.08); | |
| } | |
| .fed-nav-link--active { | |
| background: rgba(255, 255, 255, 0.14); | |
| color: var(--fed-text); | |
| font-weight: 600; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/globals.css` around lines 452 - 474, Remove the later duplicate
.fed-nav-link, .fed-nav-link:hover, and .fed-nav-link--active rule set,
preserving the earlier definitions and their intended values as the single
source of truth.
|
|
||
| return ( | ||
| <div className={`${styles.carousel_outer} ${customStyles.carousel_outer}`}> | ||
| <div className={`${customStyles.carousel_outer}`}> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restore the default outer-container class.
customStyles defaults to {}. This line removes styles.carousel_outer for every caller that does not supply an override. Keep the module class and append the optional override.
Proposed fix
- <div className={`${customStyles.carousel_outer}`}>
+ <div className={`${styles.carousel_outer} ${customStyles.carousel_outer || ""}`}>📝 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={`${customStyles.carousel_outer}`}> | |
| <div className={`${styles.carousel_outer} ${customStyles.carousel_outer || ""}`}> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Carousel/Carousel.jsx` at line 51, Update the outer container
className in the Carousel component to always include the default
styles.carousel_outer module class, then append the optional
customStyles.carousel_outer override. Preserve behavior when customStyles is
empty or omitted.
Comment on lines
+93
to
+111
| useEffect(() => { | ||
| const outer = outerRef.current; | ||
| const inner = innerRef.current; | ||
| if (!outer || !inner) return; | ||
|
|
||
| const prefersReduced = window.matchMedia("(prefers-reduced-motion: reduce)").matches; | ||
| const isMobile = window.innerWidth < 720; | ||
|
|
||
| // ── Static fallback ─────────────────────────────────────────────── | ||
| if (prefersReduced || isMobile) { | ||
| inner.style.visibility = "visible"; | ||
| panelRefs.current.forEach((p) => { | ||
| if (!p) return; | ||
| p.style.opacity = "1"; | ||
| p.style.transform = "none"; | ||
| p.style.pointerEvents = "auto"; | ||
| }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Re-evaluate the breakpoint and motion preference after resize.
The effect reads prefers-reduced-motion and window.innerWidth once, and its dependency array is empty. Two paths break:
- Desktop to mobile:
innerkeepsposition: fixedand panels keep desktop scroll-driven opacity, so the section overlays the page at mobile width. - Mobile to desktop: the early
returnat line 110 already ran, so no scroll or resize listener exists and the panels stay in the static layout.
A tablet rotation crosses the 720px breakpoint, so this is reachable. Re-run the effect when the media queries change.
🐛 Proposed fix using a media-query state
export default function About() {
const outerRef = useRef(null);
const innerRef = useRef(null);
const panelRefs = useRef([]);
const rafId = useRef(null);
+ const [staticMode, setStaticMode] = useState(false);
+
+ useEffect(() => {
+ const reduce = window.matchMedia("(prefers-reduced-motion: reduce)");
+ const mobile = window.matchMedia("(max-width: 719px)");
+ const sync = () => setStaticMode(reduce.matches || mobile.matches);
+ sync();
+ reduce.addEventListener("change", sync);
+ mobile.addEventListener("change", sync);
+ return () => {
+ reduce.removeEventListener("change", sync);
+ mobile.removeEventListener("change", sync);
+ };
+ }, []);
useEffect(() => {
const outer = outerRef.current;
const inner = innerRef.current;
if (!outer || !inner) return;
- const prefersReduced = window.matchMedia("(prefers-reduced-motion: reduce)").matches;
- const isMobile = window.innerWidth < 720;
-
// ── Static fallback ───────────────────────────────────────────────
- if (prefersReduced || isMobile) {
+ if (staticMode) {
+ inner.style.cssText = "";
inner.style.visibility = "visible";Then change the dependency array at line 181 to [staticMode], and add useState to the React import.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/sections/Home/About/About.jsx` around lines 93 - 111, Track the
reduced-motion/mobile condition in a useState value driven by a matchMedia
change listener, and use that state as staticMode in the About component’s
effect. Update the effect dependency array to [staticMode] so crossing the 720px
breakpoint or changing the motion preference cleans up and reinitializes the
fixed scroll behavior and static fallback correctly.
Comment on lines
+72
to
+109
| const handleMouseMove = (e) => { | ||
| const containerRect = container.getBoundingClientRect(); | ||
| const containerX = e.clientX - containerRect.left; | ||
| const containerY = e.clientY - containerRect.top; | ||
| container.style.setProperty("--grid-mouse-x", `${containerX}px`); | ||
| container.style.setProperty("--grid-mouse-y", `${containerY}px`); | ||
| container.style.setProperty("--grid-spotlight-opacity", "1"); | ||
|
|
||
| const cards = track.children; | ||
| const maxDist = 380; | ||
|
|
||
| for (let i = 0; i < cards.length; i++) { | ||
| const card = cards[i]; | ||
| if (!card) continue; | ||
| const rect = card.getBoundingClientRect(); | ||
| const centerX = rect.left + rect.width / 2; | ||
| const centerY = rect.top + rect.height / 2; | ||
|
|
||
| const dist = Math.hypot(e.clientX - centerX, e.clientY - centerY); | ||
|
|
||
| const localX = e.clientX - rect.left; | ||
| const localY = e.clientY - rect.top; | ||
|
|
||
| card.style.setProperty("--mouse-x", `${localX}px`); | ||
| card.style.setProperty("--mouse-y", `${localY}px`); | ||
|
|
||
| if (dist < maxDist) { | ||
| const intensity = Math.pow((maxDist - dist) / maxDist, 1.2); | ||
| card.style.setProperty("--spotlight-opacity", intensity.toFixed(3)); | ||
| card.style.borderColor = `rgba(255, 138, 0, ${(0.15 + intensity * 0.55).toFixed(2)})`; | ||
| card.style.boxShadow = `0 0 ${Math.round(intensity * 24)}px rgba(255, 138, 0, ${(intensity * 0.35).toFixed(2)})`; | ||
| } else { | ||
| card.style.setProperty("--spotlight-opacity", "0"); | ||
| card.style.borderColor = ""; | ||
| card.style.boxShadow = ""; | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Throttle the pointer handler with requestAnimationFrame.
handleMouseMove runs on every mousemove event. Inside one pass it calls getBoundingClientRect() for each card and also writes inline borderColor and boxShadow. Line 148 renders the testimonial list twice, so the loop covers double the card count. The interleaved read and write forces layout on each iteration, which produces layout thrashing during pointer movement over a running marquee.
Store the event coordinates and perform the measurement and style writes once per frame.
⚡ Proposed throttle
+ let frame = null;
+ let lastEvent = null;
+
+ const paint = () => {
+ frame = null;
+ const e = lastEvent;
+ if (!e) return;
+ // existing measurement and style-write loop, using e.clientX / e.clientY
+ };
+
const handleMouseMove = (e) => {
- const containerRect = container.getBoundingClientRect();
- ...
+ lastEvent = { clientX: e.clientX, clientY: e.clientY };
+ if (!frame) frame = requestAnimationFrame(paint);
};Cancel the pending frame in the cleanup function:
return () => {
container.removeEventListener("mousemove", handleMouseMove);
container.removeEventListener("mouseleave", handleMouseLeave);
+ if (frame) cancelAnimationFrame(frame);
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/sections/Home/Feedback/Feedback.jsx` around lines 72 - 109, Throttle
handleMouseMove with requestAnimationFrame by storing the latest pointer
coordinates and scheduling at most one frame for the measurement and
style-update loop. Move the card getBoundingClientRect calls and inline style
writes into the scheduled callback, and cancel any pending animation frame in
the effect cleanup. Preserve the existing spotlight calculations and behavior
while ensuring only the latest event is processed per frame.
Comment on lines
+51
to
+100
| useEffect(() => { | ||
| const track = trackRef.current; | ||
| if (!track) return undefined; | ||
|
|
||
| const container = track.parentElement; | ||
| if (!container) return undefined; | ||
|
|
||
| let animationFrameId; | ||
|
|
||
| const updateScale = () => { | ||
| const containerRect = container.getBoundingClientRect(); | ||
| const containerCenter = containerRect.left + containerRect.width / 2; | ||
|
|
||
| const cards = track.children; | ||
| const maxDistance = window.innerWidth <= 640 ? 110 : 160; | ||
|
|
||
| const groupedSponserCards = groupSponserCards(); | ||
| for (let i = 0; i < cards.length; i++) { | ||
| const card = cards[i]; | ||
| if (!card) continue; | ||
| const cardRect = card.getBoundingClientRect(); | ||
| const cardCenter = cardRect.left + cardRect.width / 2; | ||
|
|
||
| const distance = Math.abs(cardCenter - containerCenter); | ||
|
|
||
| let scale = 1; | ||
| let boxShadow = ""; | ||
| let zIndex = ""; | ||
|
|
||
| if (distance < maxDistance) { | ||
| const factor = (maxDistance - distance) / maxDistance; | ||
| scale = 1 + factor * 0.25; | ||
| const glowOpacity = factor * 0.7; | ||
| boxShadow = `0 0 ${12 + factor * 22}px rgba(255, 138, 0, ${glowOpacity}), 0 0 ${6 + factor * 10}px rgba(255, 138, 0, ${factor * 0.4})`; | ||
| zIndex = "10"; | ||
| } | ||
|
|
||
| card.style.transform = `scale(${scale})`; | ||
| card.style.boxShadow = boxShadow; | ||
| card.style.zIndex = zIndex; | ||
| } | ||
|
|
||
| animationFrameId = requestAnimationFrame(updateScale); | ||
| }; | ||
|
|
||
| updateScale(); | ||
|
|
||
| return () => { | ||
| cancelAnimationFrame(animationFrameId); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Stop the animation frame loop when the marquee is not visible.
updateScale re-schedules itself on every frame with no exit condition. It measures getBoundingClientRect() for 2N cards and writes three inline styles per card, and the read and write are interleaved. The loop therefore runs at 60fps for the whole session, including while the sponsors section is off screen, and forces layout on each frame.
Gate the loop with an IntersectionObserver on the container, and skip it when the user requests reduced motion.
⚡ Proposed gating
let animationFrameId;
+ let active = false;
const updateScale = () => {
...
- animationFrameId = requestAnimationFrame(updateScale);
+ if (active) animationFrameId = requestAnimationFrame(updateScale);
};
- updateScale();
+ if (window.matchMedia("(prefers-reduced-motion: reduce)").matches) {
+ return undefined;
+ }
+
+ const observer = new IntersectionObserver(([entry]) => {
+ if (entry.isIntersecting && !active) {
+ active = true;
+ updateScale();
+ } else if (!entry.isIntersecting) {
+ active = false;
+ cancelAnimationFrame(animationFrameId);
+ }
+ });
+ observer.observe(container);
return () => {
+ active = false;
+ observer.disconnect();
cancelAnimationFrame(animationFrameId);
};📝 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
| useEffect(() => { | |
| const track = trackRef.current; | |
| if (!track) return undefined; | |
| const container = track.parentElement; | |
| if (!container) return undefined; | |
| let animationFrameId; | |
| const updateScale = () => { | |
| const containerRect = container.getBoundingClientRect(); | |
| const containerCenter = containerRect.left + containerRect.width / 2; | |
| const cards = track.children; | |
| const maxDistance = window.innerWidth <= 640 ? 110 : 160; | |
| const groupedSponserCards = groupSponserCards(); | |
| for (let i = 0; i < cards.length; i++) { | |
| const card = cards[i]; | |
| if (!card) continue; | |
| const cardRect = card.getBoundingClientRect(); | |
| const cardCenter = cardRect.left + cardRect.width / 2; | |
| const distance = Math.abs(cardCenter - containerCenter); | |
| let scale = 1; | |
| let boxShadow = ""; | |
| let zIndex = ""; | |
| if (distance < maxDistance) { | |
| const factor = (maxDistance - distance) / maxDistance; | |
| scale = 1 + factor * 0.25; | |
| const glowOpacity = factor * 0.7; | |
| boxShadow = `0 0 ${12 + factor * 22}px rgba(255, 138, 0, ${glowOpacity}), 0 0 ${6 + factor * 10}px rgba(255, 138, 0, ${factor * 0.4})`; | |
| zIndex = "10"; | |
| } | |
| card.style.transform = `scale(${scale})`; | |
| card.style.boxShadow = boxShadow; | |
| card.style.zIndex = zIndex; | |
| } | |
| animationFrameId = requestAnimationFrame(updateScale); | |
| }; | |
| updateScale(); | |
| return () => { | |
| cancelAnimationFrame(animationFrameId); | |
| }; | |
| }, []); | |
| useEffect(() => { | |
| const track = trackRef.current; | |
| if (!track) return undefined; | |
| const container = track.parentElement; | |
| if (!container) return undefined; | |
| let animationFrameId; | |
| let active = false; | |
| const updateScale = () => { | |
| const containerRect = container.getBoundingClientRect(); | |
| const containerCenter = containerRect.left + containerRect.width / 2; | |
| const cards = track.children; | |
| const maxDistance = window.innerWidth <= 640 ? 110 : 160; | |
| for (let i = 0; i < cards.length; i++) { | |
| const card = cards[i]; | |
| if (!card) continue; | |
| const cardRect = card.getBoundingClientRect(); | |
| const cardCenter = cardRect.left + cardRect.width / 2; | |
| const distance = Math.abs(cardCenter - containerCenter); | |
| let scale = 1; | |
| let boxShadow = ""; | |
| let zIndex = ""; | |
| if (distance < maxDistance) { | |
| const factor = (maxDistance - distance) / maxDistance; | |
| scale = 1 + factor * 0.25; | |
| const glowOpacity = factor * 0.7; | |
| boxShadow = `0 0 ${12 + factor * 22}px rgba(255, 138, 0, ${glowOpacity}), 0 0 ${6 + factor * 10}px rgba(255, 138, 0, ${factor * 0.4})`; | |
| zIndex = "10"; | |
| } | |
| card.style.transform = `scale(${scale})`; | |
| card.style.boxShadow = boxShadow; | |
| card.style.zIndex = zIndex; | |
| } | |
| if (active) animationFrameId = requestAnimationFrame(updateScale); | |
| }; | |
| if (window.matchMedia("(prefers-reduced-motion: reduce)").matches) { | |
| return undefined; | |
| } | |
| const observer = new IntersectionObserver(([entry]) => { | |
| if (entry.isIntersecting && !active) { | |
| active = true; | |
| updateScale(); | |
| } else if (!entry.isIntersecting) { | |
| active = false; | |
| cancelAnimationFrame(animationFrameId); | |
| } | |
| }); | |
| observer.observe(container); | |
| return () => { | |
| active = false; | |
| observer.disconnect(); | |
| cancelAnimationFrame(animationFrameId); | |
| }; | |
| }, []); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/sections/Home/Sponser/Sponser.jsx` around lines 51 - 100, Update the
useEffect containing updateScale to gate the requestAnimationFrame loop with an
IntersectionObserver observing container, starting the loop only while the
sponsors marquee is visible and canceling it when it leaves the viewport. Also
detect prefers-reduced-motion and skip the animation and per-card style updates
when reduced motion is requested, cleaning up the observer, media-query listener
if used, and any pending frame on unmount.
Event ranking. /Events and the attendance page already sorted by `eventPriority`, but the home page's featured three sorted on date alone, so the order set in the admin form had no effect in the one place it is most visible. It now ranks by priority first and falls back to date, matching /Events. Unranked events sort last rather than ahead of everything. Navbar. The scroll handler kept the previous offset in state and listed it as an effect dependency, so every scroll tick re-rendered the component and tore the listener down and re-attached it — several times per frame while scrolling. Moved to refs, throttled to one update per animation frame, and the listener is attached once. The show/hide threshold now only advances the reference point once it is crossed, which stops a slow drag from flickering between states. About section pin. Replaced the JavaScript pin with native `position: sticky`. It had been switching the element between `position: relative` and `fixed` mid-scroll and writing top/left/right every frame, which is what made it jump at both boundaries. The script now only computes progress. That change needed `overflow-x: hidden` to become `overflow-x: clip` on body/.page/main. `hidden` forces `overflow-y` to compute to `auto`, making each of them a scroll container, and a scroll container between a sticky element and the viewport stops it sticking — which is why the pin had been written in JavaScript in the first place. `clip` suppresses the same sideways overflow without that side effect. Verified no horizontal scrollbar appears and all ten routes still render. Team and Alumni headings. Both set `margin-top: 20px`, which predates the pill navbar and put the text behind it. They now use a shared `--fed-navbar-offset` token so the two values cannot drift from the navbar's own geometry. Measured: heading top moves 20px -> 120px against a navbar whose lower edge is at 82px.
Both problems came from the same place. `.page` used to carry `margin-top: 88px` in globals.scss to sit clear of the fixed navbar, but globals.css zeroes it with `margin-top: 0 !important` across body, .page and main. That is why /Team, /Alumni and /profile all rendered their first heading behind the navbar. Handled once in the layout instead of per page: everything gets a `page--nav-offset` class except Home and Omega, which open with a full-bleed hero that is meant to run up behind a transparent navbar. The two per-page margins added earlier were a symptom fix and are reverted, so there is a single source for the offset again. Measured against a navbar whose lower edge is at 82px — /Team heading 108px, /profile 123px, Home unchanged at 0 with no offset class. The navbar also had no idea whether anyone was signed in; it always rendered a Login pill. Restored what the previous navbar did: the avatar, linking to /profile, once a session is present, falling back to defaultImg when the account has no picture. The mobile menu gets the avatar with the member's name plus a Logout button, as before. The signed-out state is rendered while `isLoading` is still true, which is the case on the server and on the first client render, so hydration stays consistent and the control does not flash. Verified signed in: avatar shows the account's Google picture, no Login pill, and at 375px the mobile menu shows "Krishna Das" with Logout.
The offset added in the previous commit stacked on top of spacing the
Events page already had: `.page--nav-offset` contributed 88px and
`.shell` a further 110px, leaving a large empty band under the bar.
`.shell` no longer reserves navbar space at all — that is the layout's
job now — and keeps only breathing room, with `.group`'s own 32px margin
doing the visible spacing.
The offset itself was also short. It was set to 5.5rem from a reading of
82px taken before the avatar was added; the bar actually renders 80px
tall at `top: 1.25rem`, so its lower edge is at 100px and content was
sliding under it — /Events/pastEvents overlapped by 4px. Measured
properly and set to 8rem, with a comment recording where the number
comes from so it is not guessed again.
Gap between the navbar's lower edge and the first content, measured:
/Events 130px -> 60px
/Events/pastEvents -4px (overlapping) -> 36px
/Team 48px
/Alumni 48px
/Blog content starts at 128px; the space below it is the
page's own search bar, not a gap
/profile content starts at 128px; nothing renders behind the
navbar
Home and Omega still opt out, and their hero remains full-bleed.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
app/components/Navbar.tsx (1)
51-51: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPrevent false active-link matches.
Line 51 marks unrelated paths as active when they share a prefix. For example,
/event-detailsmatches an/eventslink. Keep exact and slash-delimited descendant matching. RemovecleanPath.startsWith(cleanHref).Proposed fix
- return cleanPath === cleanHref || cleanPath.startsWith(`${cleanHref}/`) || cleanPath.startsWith(cleanHref); + return cleanPath === cleanHref || cleanPath.startsWith(`${cleanHref}/`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/Navbar.tsx` at line 51, Update the active-link matching expression in the Navbar component to remove the raw cleanHref prefix check. Preserve exact matches and slash-delimited descendant matches using cleanPath === cleanHref and cleanPath.startsWith(`${cleanHref}/`) so paths like /event-details do not match /events.src/sections/Home/Events/EventsSection.jsx (1)
124-170: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRender an empty state when
upcomingEventsis empty.After an API failure or an empty response,
loadingbecomesfalseandupcomingEvents.map(...)renders no cards. Add a visible empty-state message in this branch so users know that no events are available.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sections/Home/Events/EventsSection.jsx` around lines 124 - 170, Update the loaded-events branch in the EventsSection render so it checks whether upcomingEvents is empty before mapping cards. Render a visible no-events message when empty, while preserving the existing eventsGrid and upcomingEvents.map flow when events are available.src/sections/Home/About/styles/About.module.scss (1)
181-249: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep the panel state synchronized when these styles re-enable desktop layout.
About.jsxreturns before it registers resize handling when the page first loads below 720px or with reduced motion enabled. If the viewport later enters desktop mode, these rules restore sticky and absolute panel layout, but every panel keeps the fallback inlineopacity: 1andtransform: nonevalues. The panels then overlap.Register a breakpoint and reduced-motion listener, or reinitialize the panel state when either condition changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sections/Home/About/styles/About.module.scss` around lines 181 - 249, The About panel logic must reinitialize when viewport width crosses the 720px breakpoint or reduced-motion preference changes, instead of returning permanently from the mobile/reduced-motion path. Update the initialization and cleanup around the component’s panel state and resize handling in About.jsx so desktop transitions restore each panel’s calculated opacity and transform values, while mobile and reduced-motion layouts remain visible and non-animated.src/views/Event/styles/Event.module.scss (2)
7-21: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftMigrate
PastEvent.jsxbefore removing its CSS-module contract.
PastEvent.jsxstill reads legacy keys includingstyle.main,style.eventwhole,style.pasteventCard,style.Outcard,style.cardone,style.error, and the circle classes at Lines 75-129. This stylesheet defines none of those keys, andPastEvent.jsxdoes not apply the new.page,.shell,.backLink, or.stateclasses. The past-events page will lose its layout and state styling. UpdatePastEvent.jsxto the new class contract, or retain compatibility selectors until that migration is complete.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/Event/styles/Event.module.scss` around lines 7 - 21, Update PastEvent.jsx to replace legacy CSS-module references (style.main, style.eventwhole, style.pasteventCard, style.Outcard, style.cardone, style.error, and circle classes) with the new Event.module.scss contract, applying .page, .shell, .backLink, and .state where appropriate. Preserve the existing past-events layout and loading/error/empty-state styling, or retain compatibility selectors if the component cannot yet be migrated fully.
226-226: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the countdown background token.
--accent-is not a defined token, so the countdown pill will lose its background color. Use the intended default accent token, such asvar(--accent).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/Event/styles/Event.module.scss` at line 226, Update the countdown pill’s background-color declaration to use the defined default accent token var(--accent) instead of the invalid --accent- token.app/globals.scss (1)
68-71: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve date-picker state colors.
These base
.react-calendar__navigation buttonand.react-calendar__tilerules override vendor-specificity, so state modifiers such as weekend, neighboring-month, outside-month, and disabled tiles lose their muted colors. Unavailable dates can then look like normal dates. Scope the base rules or add explicit state overrides after this block for both the react-calendar block and the.react-datepicker__daybase block around lines 102-109.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/globals.scss` around lines 68 - 71, Adjust the base color rules for .react-calendar__navigation button and .react-calendar__tile, plus the nearby .react-datepicker__day block, so they do not override vendor state modifiers. Scope the base selectors or add later explicit overrides preserving muted colors for weekend, neighboring/outside-month, and disabled or unavailable dates in both date-picker components.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/globals.scss`:
- Around line 262-266: Update the comments surrounding the overflow-x
declaration and the additional comment near it: insert an empty line before each
comment block to satisfy Stylelint, and remove the standalone empty “//”
comment. Preserve the existing explanatory comment text and overflow-x: clip
declaration.
In `@src/views/Event/PastEvent.jsx`:
- Around line 36-58: Add an explicit empty-state render in the PastEvent
component when the past-events list is empty after a successful fetch, including
responses containing only private events. Keep the existing event-card rendering
for non-empty results and ensure the empty state provides a clear message or
action instead of displaying only the “Past Events” heading.
In `@src/views/Event/styles/Event.module.scss`:
- Line 17: Insert an empty line immediately before the `// Top padding...`
comment in the stylesheet to satisfy the
`scss/double-slash-comment-empty-line-before` rule, without changing the comment
or surrounding styles.
---
Outside diff comments:
In `@app/components/Navbar.tsx`:
- Line 51: Update the active-link matching expression in the Navbar component to
remove the raw cleanHref prefix check. Preserve exact matches and
slash-delimited descendant matches using cleanPath === cleanHref and
cleanPath.startsWith(`${cleanHref}/`) so paths like /event-details do not match
/events.
In `@app/globals.scss`:
- Around line 68-71: Adjust the base color rules for .react-calendar__navigation
button and .react-calendar__tile, plus the nearby .react-datepicker__day block,
so they do not override vendor state modifiers. Scope the base selectors or add
later explicit overrides preserving muted colors for weekend,
neighboring/outside-month, and disabled or unavailable dates in both date-picker
components.
In `@src/sections/Home/About/styles/About.module.scss`:
- Around line 181-249: The About panel logic must reinitialize when viewport
width crosses the 720px breakpoint or reduced-motion preference changes, instead
of returning permanently from the mobile/reduced-motion path. Update the
initialization and cleanup around the component’s panel state and resize
handling in About.jsx so desktop transitions restore each panel’s calculated
opacity and transform values, while mobile and reduced-motion layouts remain
visible and non-animated.
In `@src/sections/Home/Events/EventsSection.jsx`:
- Around line 124-170: Update the loaded-events branch in the EventsSection
render so it checks whether upcomingEvents is empty before mapping cards. Render
a visible no-events message when empty, while preserving the existing eventsGrid
and upcomingEvents.map flow when events are available.
In `@src/views/Event/styles/Event.module.scss`:
- Around line 7-21: Update PastEvent.jsx to replace legacy CSS-module references
(style.main, style.eventwhole, style.pasteventCard, style.Outcard,
style.cardone, style.error, and circle classes) with the new Event.module.scss
contract, applying .page, .shell, .backLink, and .state where appropriate.
Preserve the existing past-events layout and loading/error/empty-state styling,
or retain compatibility selectors if the component cannot yet be migrated fully.
- Line 226: Update the countdown pill’s background-color declaration to use the
defined default accent token var(--accent) instead of the invalid --accent-
token.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 544979cd-98ab-4e97-a646-ca4ee8bf2822
📒 Files selected for processing (24)
app/(main)/layout.jsxapp/components/Navbar.tsxapp/globals.cssapp/globals.scsssrc/authentication/Login/GoogleLogin.jsxsrc/authentication/Login/Login.jsxsrc/authentication/SignUp/GoogleSignup.jsxsrc/data/Access.jsonsrc/data/FormData.jsonsrc/data/Team.jsonsrc/data/user.jsonsrc/features/Modals/Event/EventModal/EventModal.jsxsrc/sections/Home/About/About.jsxsrc/sections/Home/About/styles/About.module.scsssrc/sections/Home/Events/EventsSection.jsxsrc/sections/Profile/Admin/Form/MemberForm/AddMemberForm.jsxsrc/sections/Profile/Admin/View/ViewEvent/VIewEvent.jsxsrc/sections/Profile/Admin/View/ViewMember/ViewMember.jsxsrc/sections/Profile/General/EventsView/EventsView.jsxsrc/views/Alumni/Alumni.jsxsrc/views/Event/Event.jsxsrc/views/Event/EventForm.jsxsrc/views/Event/PastEvent.jsxsrc/views/Event/styles/Event.module.scss
💤 Files with no reviewable changes (11)
- src/data/Access.json
- src/data/user.json
- src/authentication/Login/GoogleLogin.jsx
- src/sections/Profile/Admin/View/ViewEvent/VIewEvent.jsx
- src/features/Modals/Event/EventModal/EventModal.jsx
- src/views/Event/EventForm.jsx
- src/authentication/Login/Login.jsx
- src/data/FormData.json
- src/sections/Profile/General/EventsView/EventsView.jsx
- src/data/Team.json
- src/authentication/SignUp/GoogleSignup.jsx
🚧 Files skipped from review as they are similar to previous changes (4)
- app/(main)/layout.jsx
- app/globals.css
- src/sections/Home/About/About.jsx
- src/views/Event/Event.jsx
Comment on lines
+262
to
+266
| // `clip` rather than `hidden`: `overflow-x: hidden` forces `overflow-y` to | ||
| // compute to `auto`, which makes <body> a scroll container and is the usual | ||
| // reason `position: sticky` silently stops working further down the tree. | ||
| // `clip` suppresses the same sideways overflow without that side effect. | ||
| overflow-x: clip; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the Stylelint errors in the comments.
Stylelint reports:
- Line 262: expected an empty line before the comment.
- Line 277: expected an empty line before the comment.
- Line 278: unexpected empty comment.
Insert blank lines before the comments and remove the standalone //.
Proposed formatting fix
color: $text-color;
+
// `clip` rather than `hidden`: `overflow-x: hidden` forces `overflow-y` to
...
background-color: $page-color;
+
// `overflow-x: clip`, not `overflow: hidden`.
- //
// The intent has always been to stop sideways overflow, but `hidden` makesAlso applies to: 277-278
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 262-262: Expected empty line before comment (scss/double-slash-comment-empty-line-before)
(scss/double-slash-comment-empty-line-before)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/globals.scss` around lines 262 - 266, Update the comments surrounding the
overflow-x declaration and the additional comment near it: insert an empty line
before each comment block to satisfy Stylelint, and remove the standalone empty
“//” comment. Preserve the existing explanatory comment text and overflow-x:
clip declaration.
Source: Linters/SAST tools
Comment on lines
+36
to
+58
| // No local fallback. Both failure branches used to load the bundled | ||
| // FormData.json sample, so a backend outage silently replaced the real | ||
| // archive with test records — "Test Payment 3", "QR TEST" and the like | ||
| // — under an error banner. Showing nothing is honest; showing | ||
| // fabricated events is not. | ||
| setPastEvents([]); | ||
| } | ||
| } catch (error) { | ||
| setError({ | ||
| message: "Sorry for the inconvenience, we are having issues fetching our Events", | ||
| }); | ||
| console.error("Error fetching events:", error); | ||
| // Fallback to local JSON data | ||
| const sortedPastEvents = events | ||
| .filter((event) => event.info.isEventPast) | ||
| .sort((a, b) => new Date(b.info.eventDate) - new Date(a.info.eventDate)); | ||
| setPastEvents(sortedPastEvents); | ||
| setPastEvents([]); | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| fetchPastEvents(); | ||
| }, [events]); | ||
| // Fetched once on mount. The dependency used to be the bundled sample | ||
| // array, which never changed, so this is the same behaviour without the | ||
| // reference to data that no longer exists. | ||
| }, []); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Render an explicit empty state after removing the fallback.
A successful 200 response with no past events, or only private past events, leaves the mapped list empty. The render then shows the Past Events heading with no cards, message, or action. Add an empty-state branch for this valid response.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/views/Event/PastEvent.jsx` around lines 36 - 58, Add an explicit
empty-state render in the PastEvent component when the past-events list is empty
after a successful fetch, including responses containing only private events.
Keep the existing event-card rendering for non-empty results and ensure the
empty state provides a clear message or action instead of displaying only the
“Past Events” heading.
| width: 100%; | ||
| max-width: 1200px; | ||
| margin-inline: auto; | ||
| // Top padding is breathing room only. Clearing the fixed navbar is the |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the Stylelint spacing error.
Stylelint reports scss/double-slash-comment-empty-line-before on Line 17. Insert an empty line before the // Top padding... comment.
Proposed fix
margin-inline: auto;
+
// Top padding is breathing room only.🧰 Tools
🪛 Stylelint (17.14.0)
[error] 17-17: Expected empty line before comment (scss/double-slash-comment-empty-line-before)
(scss/double-slash-comment-empty-line-before)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/views/Event/styles/Event.module.scss` at line 17, Insert an empty line
immediately before the `// Top padding...` comment in the stylesheet to satisfy
the `scss/double-slash-comment-empty-line-before` rule, without changing the
comment or surrounding styles.
Source: Linters/SAST tools
Paid events could only be paid by UPI/QR. They can now use an external
payment page instead: the admin picks a mode, and Link mode collects the
URL, the button label and a message shown under the button. QR mode is
unchanged, and an event saved before this setting reads as QR.
Participants also upload a payment screenshot now. The register route was
reading only `_id` and `sections` and discarding every file part, while
the client serialised the File to `{}` — so an uploaded screenshot went
nowhere. Files are now uploaded and their URLs written into the stored
answer, which fixes it for any file field, not just this one.
Admins had no way to see any of it. EventStats was written and exported
but never mounted by a route, and EventCard has been shipping an
analytics button pointing at /profile/events/Analytics/<id> that 404'd.
That route exists now, gated to ADMIN on the server, and carries a
payment proofs panel.
Also:
- The terms and conditions field was nested inside the UTR field's
`validations` array, where nothing renders it, so participants were
never actually shown the no-refund acknowledgement. It is a sibling
field now, and a checkbox rather than a lone radio nobody could clear.
- PreviewForm's `open &&` guard was written without braces, so "open && ("
and ")" rendered as literal text on every registration form.
- Event posters are square and the card's media area was 16:9, so `cover`
cropped 44% off a poster and took the title and logos with it. The box
is 4:3 now and the poster is drawn slightly taller than it, trading a
12% stretch for a 16% crop off the bottom.
- FormImages was capped at 196.37x350.67. Cloudinary takes integer pixels,
so the transform never applied and posters were stored at full size —
one live banner is a 4320x4320 PNG of 3.9 MB. Capped at 1600.
- The favicon was still the Vercel triangle create-next-app ships, and
metadata pinned it, overriding the correct app/icon.png.
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 (1)
src/components/EventCard/styles/EventCard.module.scss (1)
175-184: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPrevent footer overflow on narrow cards.
The footer does not wrap. Both CTA styles keep labels on one line, and
.toolsreserves fixed-width controls..ctaGhostalso retains its automatic minimum width. If the footer renders both CTAs and tools on a narrow card, the controls can exceed the card width. Add a narrow-layout rule that wraps the footer and allows both CTA labels to shrink or wrap.Proposed layout adjustment
.footer { position: relative; z-index: 1; display: flex; + flex-wrap: wrap; align-items: center; gap: 8px; } +.cta, +.ctaGhost { + min-width: 0; + white-space: normal; +}Also applies to: 186-206, 228-245, 252-270
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/EventCard/styles/EventCard.module.scss` around lines 175 - 184, Update the `.footer` responsive styling to wrap its contents on narrow cards, and override the one-line and automatic minimum-width constraints on both CTA styles (`.ctaPrimary` and `.ctaGhost`) so their labels can shrink or wrap. Ensure the `.tools` controls can participate in the wrapped layout without forcing the footer beyond the card width.
🧹 Nitpick comments (2)
app/api/form/payments/[id]/route.ts (1)
62-97: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider bounding the registration read.
The query loads every registration row for the event, including the full
valueblob, then flattens all sections in memory. For a large event this is a big payload and a slow response on the admin request thread. The route is admin-only, so the risk is limited, but pagination or atakelimit would keep the response predictable as events grow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/form/payments/`[id]/route.ts around lines 62 - 97, Bound the registration query in the payments route by adding pagination or an appropriate take limit to prisma.formRegistration.findMany, while preserving the existing field selection and flattening behavior. Use the query around registrations and the subsequent payments construction as the only scope of change, ensuring the admin response remains predictable for large events.app/api/form/addForm/route.ts (1)
59-68: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winNeither route validates the stored payment link. Both routes persist
paymentLinkexactly as submitted. The value is later rendered as an anchorhreffor participants. Only the client checks the protocol today, so a direct API call can store ajavascript:ordata:URL. Normalize the value in both routes so the persisted data is always safe.
app/api/form/addForm/route.ts#L59-L68: parsetext("paymentLink")withURL, accept onlyhttp:andhttps:, and storenullotherwise.app/api/form/editForm/[id]/route.ts#L77-L99: apply the same parse before assigninglink; keepbuttonTextandmessagehandling unchanged. Extract the check into a shared helper so both routes stay in step.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/form/addForm/route.ts` around lines 59 - 68, Extract a shared payment-link validation helper and use it in both app/api/form/addForm/route.ts lines 59-68 and app/api/form/editForm/[id]/route.ts lines 77-99. The helper should parse the submitted value with URL, return it only for http: or https: protocols, and otherwise return null; preserve the edit route’s existing buttonText and message handling unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/form/register/route.ts`:
- Around line 73-77: Update the form-entry collection and validation flow in the
register route to accept only non-empty File values whose names match declared
media field names, and enforce a maximum upload count before Cloudinary writes
occur. Preserve the existing per-file size validation while ensuring unmatched
fields and excess files are rejected or excluded consistently.
In `@src/components/EventCard/styles/EventCard.module.scss`:
- Line 30: Update the stylesheet comment formatting around the affected blocks:
add blank lines before the comments near the ratio, 58, 332, and 384 sections,
and replace the empty // markers near lines 40 and 43 with actual blank lines.
Preserve all styling and non-empty comment text.
In `@src/features/Modals/Event/EventStats/EventStats.jsx`:
- Around line 540-554: Update the payment proof viewer in EventStats so the
thumbnail uses a keyboard-accessible button while preserving its image
presentation and setZoomedProof(payment.screenshot) activation. Add Escape-key
handling in the existing effect or overlay logic to clear the zoomed proof, and
ensure the overlay remains dismissible through its current close control.
In `@src/sections/Profile/Admin/Form/NewForm/NewForm.jsx`:
- Around line 886-891: Initialize paymentSection from the stored payment section
when the mount effect loads an existing paid event from authCtx.eventData, so
the normal rebuild path in constructForPreview can rewire section navigation.
Remove the stale-section reappend branch guarded by data.eventType === "Paid"
and stale.length, since it bypasses that rewiring.
---
Outside diff comments:
In `@src/components/EventCard/styles/EventCard.module.scss`:
- Around line 175-184: Update the `.footer` responsive styling to wrap its
contents on narrow cards, and override the one-line and automatic minimum-width
constraints on both CTA styles (`.ctaPrimary` and `.ctaGhost`) so their labels
can shrink or wrap. Ensure the `.tools` controls can participate in the wrapped
layout without forcing the footer beyond the card width.
---
Nitpick comments:
In `@app/api/form/addForm/route.ts`:
- Around line 59-68: Extract a shared payment-link validation helper and use it
in both app/api/form/addForm/route.ts lines 59-68 and
app/api/form/editForm/[id]/route.ts lines 77-99. The helper should parse the
submitted value with URL, return it only for http: or https: protocols, and
otherwise return null; preserve the edit route’s existing buttonText and message
handling unchanged.
In `@app/api/form/payments/`[id]/route.ts:
- Around line 62-97: Bound the registration query in the payments route by
adding pagination or an appropriate take limit to
prisma.formRegistration.findMany, while preserving the existing field selection
and flattening behavior. Use the query around registrations and the subsequent
payments construction as the only scope of change, ensuring the admin response
remains predictable for large events.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4deea246-3afa-42e5-8da1-547838c801e2
⛔ Files ignored due to path filters (3)
app/apple-icon.pngis excluded by!**/*.pngapp/favicon.icois excluded by!**/*.icoapp/icon.pngis excluded by!**/*.png
📒 Files selected for processing (14)
app/(main)/profile/events/Analytics/[eventId]/page.jsxapp/api/form/addForm/route.tsapp/api/form/editForm/[id]/route.tsapp/api/form/payments/[id]/route.tsapp/api/form/register/route.tsapp/layout.tsxlib/config/images.tslib/types/event.tssrc/components/EventCard/EventCard.jsxsrc/components/EventCard/styles/EventCard.module.scsssrc/features/Modals/Event/EventStats/EventStats.jsxsrc/features/Modals/Profile/Admin/PreviewForm.jsxsrc/sections/Profile/Admin/Form/NewForm/NewForm.jsxsrc/views/Event/styles/Event.module.scss
🚧 Files skipped from review as they are similar to previous changes (2)
- src/views/Event/styles/Event.module.scss
- src/components/EventCard/EventCard.jsx
Comment on lines
+73
to
+77
| for (const [key, value] of form.entries()) { | ||
| if (value instanceof File && value.size > 0) { | ||
| uploadEntries.push({ name: key, file: value }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Limit the number of uploaded files, not only the size of each file.
The collection loop accepts every File part in the request. The validation loop then caps each file at 5 MB, but nothing caps the count. One request with 200 parts of 4.9 MB each passes every check and produces 200 Cloudinary writes. The per-user rate limit does not help, because this is a single request.
The route also uploads files whose names do not match any declared media field. Those uploads are stored, then discarded at Line 191 when no field matches. Filter the entries against the declared field names, and cap the count.
🛡️ Proposed cap and field matching
/** Cap on any single uploaded answer, e.g. a payment screenshot. */
const MAX_UPLOAD_BYTES = 5 * 1024 * 1024;
+/** Cap on how many media answers one registration can carry. */
+const MAX_UPLOAD_COUNT = 5;+ const mediaFieldNames = new Set(
+ (Array.isArray(sections) ? sections : []).flatMap((section) =>
+ ((section as { fields?: Array<{ name?: string; type?: string }> })
+ .fields ?? [])
+ .filter((f) => f?.type === "file" || f?.type === "image")
+ .map((f) => f?.name)
+ .filter((n): n is string => Boolean(n)),
+ ),
+ );
+
+ const pending = uploadEntries.filter(({ name }) =>
+ mediaFieldNames.has(name),
+ );
+ if (pending.length > MAX_UPLOAD_COUNT) {
+ return expressError(400, "Too many uploaded files");
+ }
+
const uploadedByField = new Map<string, string>();
- for (const { name, file } of uploadEntries) {
+ for (const { name, file } of pending) {Also applies to: 160-176
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/api/form/register/route.ts` around lines 73 - 77, Update the form-entry
collection and validation flow in the register route to accept only non-empty
File values whose names match declared media field names, and enforce a maximum
upload count before Cloudinary writes occur. Preserve the existing per-file size
validation while ensuring unmatched fields and excess files are rejected or
excluded consistently.
Comment on lines
+540
to
+554
| {payment.screenshot ? ( | ||
| <img | ||
| src={payment.screenshot} | ||
| alt={`Payment proof from ${payment.userEmail}`} | ||
| onClick={() => | ||
| setZoomedProof(payment.screenshot) | ||
| } | ||
| style={{ | ||
| width: "100%", | ||
| height: 120, | ||
| objectFit: "cover", | ||
| borderRadius: "4px", | ||
| cursor: "zoom-in", | ||
| }} | ||
| /> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
The payment proof viewer is not reachable or dismissible by keyboard.
The thumbnail at Line 541 is an img with onClick. It has no tabIndex, no role, and no key handler, so keyboard users cannot open a proof. The overlay at Line 611 closes only on click, and it does not handle Escape. An admin using a keyboard or a screen reader cannot complete this task.
Make the thumbnail a button, and close the overlay on Escape.
♿ Proposed fix
{payment.screenshot ? (
- <img
- src={payment.screenshot}
- alt={`Payment proof from ${payment.userEmail}`}
- onClick={() =>
- setZoomedProof(payment.screenshot)
- }
- style={{
- width: "100%",
- height: 120,
- objectFit: "cover",
- borderRadius: "4px",
- cursor: "zoom-in",
- }}
- />
+ <button
+ type="button"
+ onClick={() =>
+ setZoomedProof(payment.screenshot)
+ }
+ style={{
+ display: "block",
+ width: "100%",
+ padding: 0,
+ border: "none",
+ background: "none",
+ cursor: "zoom-in",
+ }}
+ >
+ <img
+ src={payment.screenshot}
+ alt={`Payment proof from ${payment.userEmail}`}
+ style={{
+ width: "100%",
+ height: 120,
+ objectFit: "cover",
+ borderRadius: "4px",
+ }}
+ />
+ </button>
) : (Add the Escape handler next to the other effects:
+ useEffect(() => {
+ if (!zoomedProof) return;
+ const onKeyDown = (event) => {
+ if (event.key === "Escape") setZoomedProof(null);
+ };
+ window.addEventListener("keydown", onKeyDown);
+ return () => window.removeEventListener("keydown", onKeyDown);
+ }, [zoomedProof]);Also applies to: 610-634
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 540-553: A list component should have a key to prevent re-rendering
Context: <img
src={payment.screenshot}
alt={Payment proof from ${payment.userEmail}}
onClick={() =>
setZoomedProof(payment.screenshot)
}
style={{
width: "100%",
height: 120,
objectFit: "cover",
borderRadius: "4px",
cursor: "zoom-in",
}}
/>
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/Modals/Event/EventStats/EventStats.jsx` around lines 540 - 554,
Update the payment proof viewer in EventStats so the thumbnail uses a
keyboard-accessible button while preserving its image presentation and
setZoomedProof(payment.screenshot) activation. Add Escape-key handling in the
existing effect or overlay logic to clear the zoomed proof, and ensure the
overlay remains dismissible through its current close control.
Both were fixed-position overlays rendered on top of the still-mounted event listing. A multi-step registration form with file uploads and a payment step is a destination, not a dialog, and the event detail is a shareable URL — neither wanted modal semantics. /Events/:id is now a detail page, and /Events/:id/Form a registration page with a back link. The listing is no longer rendered underneath either, so opening one no longer fetches and paints every event just to cover it. PreviewForm gains an `inline` prop. Only the three wrapper elements differ between the two modes — the form itself stays a single copy, so a change to a step cannot land in one mode and not the other. The reference implementation duplicated the whole tree instead. The scroll lock is now conditional on that prop. It exists so the page behind an overlay does not scroll while the overlay owns its own scroller; inline the form *is* the page, and locking the body left a form taller than the viewport unable to reach its own Submit button. The detail page keeps EventModal's behaviour unchanged — the countdown, the button state machine and the related-event lock are the same rules, so anything registerable before is registerable now. Only the shell and styling differ, and the styling uses the tokens in globals.scss rather than the old modal's palette. Two deliberate changes: it no longer waits three seconds before navigating to the form, which on a page reads as an unresponsive button, and the share URL is read in an effect rather than during render, where `window` does not exist. EventModal is left in the tree, still exported, but no longer routed.
The stats panel reused `.card` from EventModal.module.scss — a fixed 32rem wide with no height constraint. It carries far more than an event card does (counts, the year breakdown, the registrant list, and now a grid of payment proofs), so it simply grew past the bottom of the viewport with no way to reach what fell off. The inline `overflow-y: auto` on it could never engage: an element with no height bound never overflows. It now has its own stylesheet — the file existed but was empty — with a max-height of min(86vh, 900px) and its own scroller, and is widened to min(46rem, 94vw) so the payment grid is not squeezed into 32rem. The backdrop scrolls as a fallback below roughly 420px of viewport height, where the panel's own minimum would otherwise put its top out of reach, and `overscroll-behavior: contain` stops a flick at the end of the list from scrolling the page behind it. Verified with 2000px of injected content: the panel holds at 619px against a 720px viewport, scrolls internally, and leaves the page behind it unmoved. Also drops the skeleton block that rendered unconditionally next to the panel. It was not gated on `isLoading` — the real loading state is handled inside the panel — so it was permanently visible filler.
…nce export Three layout faults, all visible with a couple of registrants: - The overlay was `z-index: 10` while the navbar wrapper is 999, so the navbar floated over the modal and clipped its heading. - The registrant list was `height: 300px` rather than a maximum, and left `align-items` at its `stretch` default — so two entries reserved the full height and each card grew to fill all 300px of it. Cards now hug their content and the box shrinks to fit. - The close button used `.closeModal`, which is `top: -8%` and therefore positioned outside its own panel; it landed up beside the navbar. Those classes were shared from EventModal.module.scss but only ever used here, so they move into this component's own stylesheet rather than being overridden across module boundaries. The attendance export now carries `utr` and `paymentScreenshot`. Payment proof lives on the registration, not the attendance record, so it has to be joined in — without it the attendance sheet gave a volunteer at the desk no way to check a payment against what was actually uploaded. The extraction is now one shared helper used by both the export and the payments endpoint, instead of two copies that could drift. Also corrects both download filenames from .xlsx to .csv. Neither route has ever produced a workbook — they stream CSV, and Excel warns when the extension disagrees with the contents.
The crop/stretch treatment was applied to every event card. It only earns its keep on the spotlight, where the poster is large enough for the title, the logos and the subject to matter. On a 359px grid thumbnail it bought nothing and cost a 12% distortion on every image on the page. The grid goes back to what it was: a 16:9 box and a plain centred `cover`. Only `.featured` keeps the 4:3-ish box, the `fill` and the 1.19 stretch. The skeleton ratios follow the cards they stand in for, so nothing shifts on load. Measured: featured 573x429, 16% cropped, 12.3% stretched; grid 359x202 at `cover`, element exactly its box, no stretch.
The spotlight card was a flat 430px tall sitting under a 128px navbar offset, which on a short laptop ran it off the bottom of the screen — the one card the page exists to show could not be seen without scrolling. It is now sized as a share of the viewport at every width, so it occupies roughly the same portion of the screen everywhere: 53% on a 375x812 phone, 50% on a 768x1024 tablet, 46% on 1440x900, fitting above the fold in each. The navbar offset drops from 8rem to 6.5rem. It was set from an 80px reading of the bar; the bar renders 62px tall and ends at 82px, so 8rem was reserving 46px of empty band above every page on the site. Mobile fixes, all measured at 375px rather than assumed: - Every field rendered at 12px. Safari on iOS zooms the viewport in when a focused field's text is under 16px and never zooms back out, so the page magnified itself partway through signing up and stayed that way. Fields are 16px below the tablet breakpoint, with a global floor to catch react-select's own injected inputs. - `.inputTxtArea` and `.inputSelect` were a flat `width: 380px`, wider than the phone they were being viewed on. Fluid now. - The registration form's field row is a wrapping flexbox with a 3rem column gap; on a phone each field became a shrink-to-content item about 200px wide inside a 343px card. One field per row now, full width — the text input goes from 203px to 329px. - Buttons rendered 34px tall, under the comfortable touch minimum on the control every registration has to end with. 44px now. Checked for horizontal overflow on /, /Events, /Events/:id, the registration page, /Team, /Login and /SignUp: none of them scroll sideways. The marquee and glow elements report widths past the viewport but are clipped by `overflow-x: clip` on body and never scroll.
A session has two halves: the httpOnly `token` cookie proxy.ts gates routes on, and the localStorage copy AuthContext reads. They expired at different times -- 7 hours for the cookie, 2h40m for localStorage (and 3h, and 2h, depending on which of the five login call sites you came through). In the gap between the two, the app deadlocked. proxy.ts saw a live cookie and redirected /Login to /profile; the profile guard saw no localStorage and redirected back to /Login. Clicking Login did nothing at all, for the whole remainder of the cookie's life. The trigger was closing the tab over the localStorage expiry, so the logout timer never ran to clean up the cookie. The mismatched numbers came from the Vite frontend, where they were harmless -- that app gated nothing on the server, so /Login always rendered. Adding proxy.ts in the migration is what turned the mismatch into a trap. - SESSION_TTL_MS, exported and used at every login call site, matching the JWT. - clearServerSession(), so the client can drop a cookie it cannot see. Called when a stored session is found expired on mount. - Both route guards clear the cookie before redirecting to /Login, which breaks the loop for any residual disagreement. /profile needed this separately -- it has its own inline guard rather than using ProtectedRoute.
PR #15 was squash-merged upstream, so none of the commits on this branch appear there by SHA even though all of their content does. Merging normally therefore reported conflicts on every file both sides had touched, all of them spurious. Resolved by taking upstream's tree wholesale: upstream/main is a strict superset of this branch -- it carries the squashed migration work plus the navbar, About, Sidebar and payment-UI passes that landed after it. Verified before committing that the merge result is byte-identical to upstream/main.
…e weight Seven defects reported from production, and one found while verifying them. Event links (every past event 404'd) EventCard built its detail link as `modalpath + id`, and three of the four callers passed a path with no route: "/pastEvents/", "/Events/pastEvents/" and "/profile/Events/". `modalpath` predates the modal-to-page conversion, when it named a modal and was never navigated to. The card now builds `/Events/<id>` itself and the prop is gone, so a caller cannot reintroduce this. The same "/profile/Events" string was hardcoded in the profile events table's View button. Related-event lock (gated events were open to everyone) Both event views computed one page-wide `isRegisteredInRelatedEvents`: "is the visitor registered for *any* event that gates *any* other", then applied it to every card. Registering for one prerequisite unlocked every gated event on the site. It also only ever set the flag true, never back to false. Now evaluated per event in src/utils/prerequisite.js, matching the test the register route already applies server-side. Payment step vanished when a paid event was edited `constructForPreview` drops the stored "Payment Details" section and re-adds `paymentSection` in its place. Nothing hydrated `paymentSection` when loading an event for editing, so it stayed null and the step was dropped and never rebuilt -- saving an edit removed Pay Now, the UTR field and the screenshot upload from the live form. The stored section's id is reused so earlier sections' `onNext` pointers still resolve. Certificates were invisible to participants /api/certificate/sendCertificatesAndEvents was ported as an admin-only certificate *issuing* endpoint. In Express it is a lookup: given an email, return each issued certificate with its event. Its only caller is the participant profile, which got a 403, so the list stayed empty and View led nowhere. Restored to the Express contract, authorised as "your own certificates, or anyone's if you are an admin" -- the Express route had no auth at all. The client-side lookup also called two routes that were never ported; the joined event now comes back with the certificate instead. Analytics unreachable on mobile The admin bar was gated on an `isHovered` state. No touch device sets it, so admins on a phone could not reach Edit, Delete or Analytics. The reveal now lives in CSS behind `@media (hover: hover)`. Login card cramped on laptops `.authpage` centres with `align-items: center`, which sizes its child shrink-to-fit. Login nested two unclassed wrappers inside it, so the card's `width: 100%` resolved against a collapsed 257px instead of reaching its 480px max-width. Mobile performance The home page shipped 22.09 MB of images across 22 requests, ~521 MB once decoded -- including an 8.2 MB 6000x4000 JPEG drawn in a 208px box, fetched twice. No Cloudinary URL carried a transformation. `IMAGE_SIZES` caps uploads, so it does nothing for images already stored; this caps delivery instead, via f_auto/q_auto/c_limit for Cloudinary and /_next/image for the other hosts. Measured after: 0.66 MB and ~42 MB decoded, nothing above 1200px, no broken images. Also: `parentEventCount` was initialised to [] and compared against 0.
Measured at 375px while verifying that Analytics is reachable without hover: the Edit/Delete/Analytics buttons rendered 32px tall, under the 44px minimum used elsewhere in the mobile pass. They are now the only way an admin reaches Analytics on a phone, so they are worth sizing properly.
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
6 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
Bug Fixes