Club/new site#450
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughComplete rebuild of the ChangesClub Site Redesign
Sequence Diagram(s)sequenceDiagram
participant Browser
participant RootLayout
participant ClubMotionRuntime
participant Navbar
participant Page
participant HomeEvents
participant loadClubEvents
participant BladeAPI
Browser->>RootLayout: mount app
RootLayout->>ClubMotionRuntime: mount useEffect
ClubMotionRuntime->>Browser: install IntersectionObserver, scroll listener
RootLayout->>Navbar: render with bladeUrl prop
Navbar->>Browser: attach Framer Motion scroll listener
RootLayout->>Page: render children (HomePage)
Page->>HomeEvents: render with bladeUrl, eventLimit=6
HomeEvents->>loadClubEvents: call(bladeUrl, AbortSignal)
loadClubEvents->>BladeAPI: httpBatchLink POST /api/trpc
BladeAPI-->>loadClubEvents: raw event records
loadClubEvents-->>HomeEvents: PublicClubEvent[]
HomeEvents-->>Browser: render event rows + skeleton/empty handling
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 6 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
i do think the navbar should not contain the sign up buttons if it goes into hamburger mode
|
farisAwheel
previously requested changes
May 21, 2026
farisAwheel
left a comment
farisAwheel
left a comment
Contributor
There was a problem hiding this comment.
LGTM except the navbar behaves weirdly on mid-sized screens, see prev comment
Comment on lines
+45
to
+54
| const fieldTriggerClassName = | ||
| "h-12 overflow-hidden rounded-none border-x-0 border-b-2 border-t-0 border-white/75 bg-transparent px-0 text-left text-lg font-medium text-white shadow-none transition-colors hover:border-white hover:bg-transparent hover:text-white focus:ring-0 focus-visible:ring-0 focus-visible:ring-offset-0 data-[placeholder]:text-white/35 [&>span]:block [&>span]:max-w-[calc(100%-2rem)] [&>span]:truncate [&>svg]:text-white/55 sm:h-14 sm:text-xl md:text-2xl"; | ||
| const fieldLabelClassName = | ||
| "text-base font-semibold leading-none text-white/85 md:text-lg"; | ||
| const optionalTextClassName = | ||
| "ml-2 text-sm font-medium italic text-white/45 md:text-base"; | ||
| const requiredMarkClassName = | ||
| "text-xl font-black text-[#ff4fd8] drop-shadow-[0_0_10px_rgba(255,79,216,0.85)] md:text-2xl"; | ||
| const checkboxClassName = | ||
| "mt-0.5 flex h-5 w-5 items-center justify-center border-white/45 bg-white/5 text-white shadow-none data-[state=checked]:border-white data-[state=checked]:bg-white data-[state=checked]:text-[#21103d] focus-visible:ring-white/40 [&>span>svg]:h-5 [&>span>svg]:w-5"; |
Contributor
There was a problem hiding this comment.
Why are you using class names like this rather than components?
Contributor
There was a problem hiding this comment.
im confused on this too but honestly its ok probably just codex shenanigans lol
Comment on lines
+16
to
+19
| }: { | ||
| profile?: { firstName?: string | null; lastName?: string | null } | null; | ||
| profileKind?: PassProfileKind; | ||
| }) { |
Contributor
There was a problem hiding this comment.
Is there a better way to represent this?
alexanderpaolini
left a comment
alexanderpaolini
left a comment
Contributor
There was a problem hiding this comment.
I only looked over @forge/blade and @forge/api for the most part. Most of everything I'm asking is just curiosity
Comment on lines
+70
to
+71
| if (profile && (!profile.firstName || !profile.lastName)) { | ||
| toast.error("Missing profile information"); |
Contributor
There was a problem hiding this comment.
I'm a bit confused why this is even possible. Since this component passes through profile, can we not just require that profile is defined? Might be something to look into.
| > | ||
| {selectedJudge | ||
| ? `${selectedJudge.name} — ${selectedJudge.challengeTitle}${ | ||
| ? `${selectedJudge.name}, ${selectedJudge.challengeTitle}${ |
Contributor
There was a problem hiding this comment.
A side effect of the control-f on m-dashes? I think this was unintentional
| </a> | ||
| ) : ( | ||
| "—" | ||
| "Not rated" |
Contributor
There was a problem hiding this comment.
Haven't seen what this look like, but surely the m-dash is better than Not rated. Low importance.
Contributor
|
general tidbit: please mark any coomments as resolved when addressed in the changes. im unsure as to whether or not alex's comments have been addressed atm |
DanielJEfres
previously requested changes
Jun 16, 2026
DanielJEfres
left a comment
DanielJEfres
left a comment
Contributor
There was a problem hiding this comment.
pls read over my comments and resolve them when fixed / answer if im wrong about something. once all are resolved it's basically approved from me !
apart from what i pointed out everything looked amazing, this is a great job well done i just wanted to make sure it was as perfect as we could get it. mostly looked for accessibility or performance issues, but found some other stuff worth pointing out on the way aswell
great job chat
| @@ -4,13 +4,32 @@ const config = { | |||
| reactStrictMode: true, | |||
| images: { | |||
| unoptimized: true, | |||
Contributor
There was a problem hiding this comment.
critical: why are images unoptimized? we should just remove this line and rely on the remote patterns and cdn r2
Contributor
There was a problem hiding this comment.
codex say bad. jason not know enough to no agree
There was a problem hiding this comment.
codex also say bad. michael don't know web
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Michael-RDev
left a comment
Michael-RDev
left a comment
There was a problem hiding this comment.
lowkey most of it looks fine,no new issues that someone else didn't address.
| @@ -4,13 +4,32 @@ const config = { | |||
| reactStrictMode: true, | |||
| images: { | |||
| unoptimized: true, | |||
There was a problem hiding this comment.
codex also say bad. michael don't know web
…classes to switch between the different types of buttons)
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (11)
apps/club/src/app/sponsors/sponsors-config.ts (1)
150-366: 💤 Low valueConsider normalizing sponsor ID and logoUrl key references.
The
PAST_SPONSORSarray mixes direct key access (e.g.,ONLINE_SPONSOR_LOGOS.github) with bracket notation using space-separated keys (e.g.,ONLINE_SPONSOR_LOGOS["lockheed martin"]). While this works at runtime becausenormalizeSponsorKeyhandles both, it creates maintainability friction. Consider either:
- Using kebab-case IDs consistently and updating
ONLINE_SPONSOR_LOGOSkeys to match, or- Extracting a helper like
getLogo(sponsorId)that normalizes access.🤖 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 `@apps/club/src/app/sponsors/sponsors-config.ts` around lines 150 - 366, The PAST_SPONSORS array uses inconsistent patterns to access ONLINE_SPONSOR_LOGOS, mixing direct key access like ONLINE_SPONSOR_LOGOS.github with bracket notation like ONLINE_SPONSOR_LOGOS["lockheed martin"]. Create a helper function (e.g., getLogo) that takes a sponsor ID and returns the normalized logo URL from ONLINE_SPONSOR_LOGOS by handling the key normalization internally. Then replace all logoUrl assignments in the PAST_SPONSORS array to use this helper function instead of direct ONLINE_SPONSOR_LOGOS access, ensuring consistent and maintainable code.apps/club/src/app/hackathons/hackathon-history.tsx (1)
47-104: ⚡ Quick winConsider documenting the source of precise timeline coordinates.
The
baseTimelineRowsarray contains coordinates with sub-pixel precision (e.g.,92.71826171875px). While this precision is valid, adding a comment explaining these values come from Figma or design specs would help future maintainers understand why they shouldn't be rounded.📝 Suggested documentation
+// Timeline row positions derived from Figma design specs (do not round) const baseTimelineRows = [ { side: "left",🤖 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 `@apps/club/src/app/hackathons/hackathon-history.tsx` around lines 47 - 104, The baseTimelineRows array contains coordinates with sub-pixel precision that lack documentation about their origin. Add a comment block above the baseTimelineRows constant explaining that these precise coordinate values come from Figma or design specifications, and clarify that this precision should be maintained and not rounded to simpler values to ensure accurate layout positioning.apps/club/src/app/events/page.tsx (1)
12-18: ⚡ Quick winUnify Blade URL source to avoid schema/link drift.
eventsPageJsonLd.offers.urlusesBLADE_URL, while the page UI and client fetches useenv.BLADE_URL. Keeping one source prevents mismatches between structured data and user-facing links.Proposed fix
import { - BLADE_URL, createPageMetadata, createWebPageJsonLd, DISCORD_URL, INSTAGRAM_URL, SITE_URL, } from "../seo"; @@ offers: { "`@type`": "Offer", - url: BLADE_URL, + url: env.BLADE_URL, price: "0", priceCurrency: "USD", availability: "https://schema.org/InStock", },Also applies to: 72-75, 148-149, 168-169
🤖 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 `@apps/club/src/app/events/page.tsx` around lines 12 - 18, The file imports BLADE_URL from the seo module, but uses env.BLADE_URL in other places (lines 72-75, 148-149, 168-169), creating inconsistency between structured data and UI links. Replace all instances of env.BLADE_URL with the imported BLADE_URL from the seo module to ensure a single, unified source for the Blade URL throughout the entire file, preventing schema and link drift.apps/club/src/app/events/events-client.tsx (2)
253-275: ⚡ Quick winAdd explicit date semantics to calendar day buttons.
The button text is numeric-only, which is ambiguous for assistive tech. Add
aria-label(date + event count) andaria-pressedfor selected state.Proposed fix
{cells.map((cell) => { const hasVisibleEvent = cell.inMonth && cell.eventCount > 0; + const dayLabel = `${cell.dateKey}${cell.eventCount > 0 ? `, ${cell.eventCount} event${cell.eventCount === 1 ? "" : "s"}` : ", no events"}`; return ( <button key={cell.key} type="button" disabled={!cell.inMonth} + aria-label={dayLabel} + aria-pressed={cell.inMonth ? cell.isSelected : undefined} className={cn(🤖 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 `@apps/club/src/app/events/events-client.tsx` around lines 253 - 275, The calendar day button in the events-client.tsx file needs improved accessibility attributes. Add an aria-label attribute to the button element that includes the date information from cell.dateKey and indicates whether there is a visible event (using hasVisibleEvent), and add an aria-pressed attribute set to the cell.isSelected value to properly communicate the selected state to assistive technologies. This will make the semantic meaning of each calendar day button clear to screen readers.
408-418: ⚡ Quick winExpose active pagination state with
aria-current.Screen readers currently don’t get an explicit current-page marker. Add
aria-current="page"on the active page button.Proposed fix
<button key={page} type="button" + aria-current={page === currentPage ? "page" : undefined} className={cn( "underline-offset-4 transition-colors hover:text-white", page === currentPage && "text-white underline", )}🤖 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 `@apps/club/src/app/events/events-client.tsx` around lines 408 - 418, The pagination button component needs accessibility improvements for screen readers. When a page button is the active page (when page === currentPage), add the aria-current attribute with value "page" to explicitly mark the current page button. This should be added conditionally to the button element alongside the existing className prop, so screen readers can announce which page is currently selected.apps/club/src/app/_lib/club-events.ts (1)
48-52: ⚡ Quick winReplace the broad cast with a type guard.
Line 50 uses a broad
ascast in external-data normalization. A narrow guard keeps the same runtime checks while preserving stronger TypeScript safety.Proposed refactor
interface BladeEventRecord { id?: unknown; name?: unknown; description?: unknown; startDateTime?: unknown; endDateTime?: unknown; location?: unknown; tag?: unknown; } +function isBladeEventRecord(value: unknown): value is BladeEventRecord { + return typeof value === "object" && value !== null; +} + function normalizeBladeEvents( value: unknown, limit = DEFAULT_EVENT_LIMIT, ): PublicClubEvent[] { if (!Array.isArray(value)) return []; const now = new Date(); return value .map((event): PublicClubEvent | null => { - if (!event || typeof event !== "object") return null; - - const bladeEvent = event as BladeEventRecord; + if (!isBladeEventRecord(event)) return null; + const bladeEvent = event;As per coding guidelines,
**/*.{ts,tsx,js,jsx}and**/*.{ts,tsx}disallow broad casts/TypeScript escape hatches.🤖 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 `@apps/club/src/app/_lib/club-events.ts` around lines 48 - 52, Replace the broad `as BladeEventRecord` cast on the line assigning the bladeEvent variable with a proper type guard function. Create a type guard function that validates the event object has the required properties of BladeEventRecord (such as startDateTime and endDateTime fields) using property checks, then use that type guard instead of the cast. This maintains the same runtime safety checks while preserving stronger TypeScript type safety and complying with the coding guidelines that restrict broad casts in TypeScript files.Source: Coding guidelines
apps/club/src/app/globals.css (1)
94-94: 💤 Low valueConsider lowercase
text-renderingvalue for stylelint compliance.Stylelint expects
geometricprecision(lowercase), but the CSS spec accepts both. If CI enforces this rule, update the casing.- text-rendering: geometricPrecision; + text-rendering: geometricprecision;🤖 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 `@apps/club/src/app/globals.css` at line 94, The text-rendering property value in the globals.css file uses camelCase casing (geometricPrecision) but stylelint requires lowercase casing (geometricprecision) for CI compliance. Locate the text-rendering property and change its value from geometricPrecision to geometricprecision to match stylelint's expected formatting rules.Source: Linters/SAST tools
apps/club/src/app/_components/footer.tsx (2)
73-73: ⚡ Quick winAdd
role="list"or use a semantic element for the social links container.The
aria-labelon a generic<div>is less meaningful to assistive technologies without a corresponding role. Consider addingrole="list"or using a<nav>or<ul>element.- <div className={className} aria-label="Social links"> + <nav className={className} aria-label="Social links"> {SOCIAL_LINKS.map((social) => { ... })} - </div> + </nav>🤖 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 `@apps/club/src/app/_components/footer.tsx` at line 73, The social links container div element uses aria-label without a proper semantic role, which reduces accessibility for assistive technologies. Either add role="list" to the existing div with className and aria-label="Social links", or replace the div element with a more semantic element such as nav or ul depending on the structure of the social links content within it.
176-179: ⚡ Quick winConsider using dynamic year for copyright.
The hardcoded year "2026" will require annual updates. A dynamic approach prevents staleness:
- <span className="md:hidden">© 2026 Knight Hacks.</span> - <span className="hidden md:inline"> - © Copyright 2026, All Rights Reserved by Knight Hacks - </span> + <span className="md:hidden">© {new Date().getFullYear()} Knight Hacks.</span> + <span className="hidden md:inline"> + © Copyright {new Date().getFullYear()}, All Rights Reserved by Knight Hacks + </span>Note: Since this is a Server Component,
new Date()evaluates at build/request time, which is typically acceptable for copyright years.🤖 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 `@apps/club/src/app/_components/footer.tsx` around lines 176 - 179, Replace the hardcoded year "2026" in both the mobile view span (className="md:hidden") and the desktop view span (className="hidden md:inline") in the footer.tsx footer component with a dynamic calculation using new Date().getFullYear() to ensure the copyright year automatically updates without requiring annual maintenance.apps/club/src/app/_components/json-ld.tsx (1)
1-10: 💤 Low value
dangerouslySetInnerHTMLis safe here for server-controlled JSON-LD.The static analysis warning about XSS is a false positive in this context. The pattern used—escaping
<as\u003c—is the standard safe approach for JSON-LD injection, preventing</script>breakout attacks.This works safely because:
- The
dataoriginates from server-controlledsiteJsonLdinseo.ts, not user input- JSON-LD scripts are non-executable data blocks for search engines
For extra defense-in-depth, you could also escape
>:Optional: Enhanced escaping
- __html: JSON.stringify(data).replace(/</g, "\\u003c"), + __html: JSON.stringify(data).replace(/</g, "\\u003c").replace(/>/g, "\\u003e"),🤖 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 `@apps/club/src/app/_components/json-ld.tsx` around lines 1 - 10, The JsonLd component currently escapes only the `<` character to prevent script tag breakout attacks, which is safe for server-controlled JSON-LD data. For enhanced defense-in-depth security, modify the dangerouslySetInnerHTML escaping logic in the JsonLd function to also escape the `>` character as `\u003e` in addition to the existing `<` to `\u003c` replacement, providing an extra layer of protection against potential edge cases.Source: Linters/SAST tools
apps/club/src/app/global-error.tsx (1)
3-21: ⚡ Quick winAdd error recovery capability to the global error boundary.
Next.js provides
errorandresetprops to global error boundaries. The current implementation doesn't accept these props, which means users have no way to recover from errors without manually refreshing the page.Adding a reset button improves UX by letting users retry without a full page reload.
🔄 Suggested enhancement
-export default function GlobalError() { +export default function GlobalError({ + reset, +}: { + error: Error & { digest?: string }; + reset: () => void; +}) { return ( <html lang="en"> <body> <main className="flex min-h-screen items-center justify-center bg-[`#140316`] px-6 text-center text-white"> <div className="max-w-xl"> <p className="club-eyebrow text-sm font-black">Knight Hacks</p> <h1 className="mt-5 text-4xl font-black uppercase md:text-6xl"> Something went wrong </h1> <p className="mt-5 text-base font-semibold leading-7 text-[`#d1d5dc`]"> Please refresh the page or try again in a moment. </p> + <button + onClick={() => reset()} + className="mt-6 rounded-lg bg-[var(--club-gold)] px-6 py-3 font-bold text-black transition hover:opacity-90" + > + Try again + </button> </div> </main> </body> </html> ); }🤖 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 `@apps/club/src/app/global-error.tsx` around lines 3 - 21, The GlobalError component is not accepting the error and reset props that Next.js provides to global error boundaries, which prevents users from recovering from errors without manual page refresh. Update the GlobalError function signature to accept error and reset as parameters, then add a clickable button element within the component that calls the reset function when clicked to allow users to retry without requiring a full page reload.
🤖 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 `@apps/club/src/app/_components/home-events.tsx`:
- Around line 22-26: The LoadingRows function is using the hardcoded
HOME_EVENT_LIMIT constant when rendering loading placeholders instead of
respecting a dynamic eventLimit parameter. Modify the LoadingRows function
signature to accept an eventLimit parameter, then replace HOME_EVENT_LIMIT in
the Array.from call on line 25 with the eventLimit parameter. Additionally,
update any calls to LoadingRows to pass the appropriate eventLimit value as an
argument to ensure loading placeholders match the number of events actually
displayed.
In `@apps/club/src/app/_lib/club-events.ts`:
- Around line 54-63: The validation block is incorrectly filtering events using
startDate <= now, which removes active in-progress events. Replace the startDate
comparison on line 57 with endDate <= now to properly expire only past events.
Additionally, add a validation check to reject invalid date ranges where
startDate >= endDate, ensuring events have a valid time window before the
current date check.
In `@apps/club/src/app/_lib/site-config.ts`:
- Around line 30-37: The CLUB_NAV_ITEMS constant currently contains 6 navigation
items, but the design spec calls for a 4-link primary navigation in the header.
Split this constant into two separate navigation lists: keep Home, Events,
Teams, and Sponsors in the primary CLUB_NAV_ITEMS (which the Navbar component
consumes), and create a new secondary navigation constant for About and
Hackathons that can be used in footer or secondary navigation areas. This
ensures the primary header navigation aligns with the 4-link desktop/mobile
specification.
In `@apps/club/src/app/layout.tsx`:
- Line 104: The root wrapper div with className "club-home-bg flex min-h-screen
flex-col overflow-hidden" is preventing horizontal scrolling on child elements
like the hackathon history page because the parent's overflow-hidden takes
precedence over child overflow-x-auto rules. Either remove the overflow-hidden
class from this root wrapper entirely and apply overflow-hidden only to specific
child components (like the hero component) that need it, or replace
overflow-hidden with overflow-y-hidden overflow-x-visible on the root wrapper to
prevent vertical clipping while allowing horizontal scroll. Choose the approach
that best maintains your layout design while enabling the hackathon history page
to scroll horizontally as needed.
In `@apps/club/src/app/seo.ts`:
- Around line 83-85: The alternates.canonical property is using the relative
path variable instead of the computed url variable. Since the function already
computes const url = absoluteUrl(path) and uses it for openGraph.url, replace
the canonical value from path to url to ensure consistency in how absolute URLs
are used throughout the metadata object.
In `@apps/club/src/app/sponsors/sponsors-client.tsx`:
- Around line 222-227: The anchor tag with href={sponsorUrl} in the
sponsors-client.tsx file is missing security attributes required for external
links. Add target="_blank" and rel="noopener noreferrer" attributes to the <a>
element that wraps the "Become a Sponsor" text and ArrowRight icon to prevent
security vulnerabilities when opening external URLs, especially user-controlled
ones from PUBLIC_LINKS.blade.
---
Nitpick comments:
In `@apps/club/src/app/_components/footer.tsx`:
- Line 73: The social links container div element uses aria-label without a
proper semantic role, which reduces accessibility for assistive technologies.
Either add role="list" to the existing div with className and aria-label="Social
links", or replace the div element with a more semantic element such as nav or
ul depending on the structure of the social links content within it.
- Around line 176-179: Replace the hardcoded year "2026" in both the mobile view
span (className="md:hidden") and the desktop view span (className="hidden
md:inline") in the footer.tsx footer component with a dynamic calculation using
new Date().getFullYear() to ensure the copyright year automatically updates
without requiring annual maintenance.
In `@apps/club/src/app/_components/json-ld.tsx`:
- Around line 1-10: The JsonLd component currently escapes only the `<`
character to prevent script tag breakout attacks, which is safe for
server-controlled JSON-LD data. For enhanced defense-in-depth security, modify
the dangerouslySetInnerHTML escaping logic in the JsonLd function to also escape
the `>` character as `\u003e` in addition to the existing `<` to `\u003c`
replacement, providing an extra layer of protection against potential edge
cases.
In `@apps/club/src/app/_lib/club-events.ts`:
- Around line 48-52: Replace the broad `as BladeEventRecord` cast on the line
assigning the bladeEvent variable with a proper type guard function. Create a
type guard function that validates the event object has the required properties
of BladeEventRecord (such as startDateTime and endDateTime fields) using
property checks, then use that type guard instead of the cast. This maintains
the same runtime safety checks while preserving stronger TypeScript type safety
and complying with the coding guidelines that restrict broad casts in TypeScript
files.
In `@apps/club/src/app/events/events-client.tsx`:
- Around line 253-275: The calendar day button in the events-client.tsx file
needs improved accessibility attributes. Add an aria-label attribute to the
button element that includes the date information from cell.dateKey and
indicates whether there is a visible event (using hasVisibleEvent), and add an
aria-pressed attribute set to the cell.isSelected value to properly communicate
the selected state to assistive technologies. This will make the semantic
meaning of each calendar day button clear to screen readers.
- Around line 408-418: The pagination button component needs accessibility
improvements for screen readers. When a page button is the active page (when
page === currentPage), add the aria-current attribute with value "page" to
explicitly mark the current page button. This should be added conditionally to
the button element alongside the existing className prop, so screen readers can
announce which page is currently selected.
In `@apps/club/src/app/events/page.tsx`:
- Around line 12-18: The file imports BLADE_URL from the seo module, but uses
env.BLADE_URL in other places (lines 72-75, 148-149, 168-169), creating
inconsistency between structured data and UI links. Replace all instances of
env.BLADE_URL with the imported BLADE_URL from the seo module to ensure a
single, unified source for the Blade URL throughout the entire file, preventing
schema and link drift.
In `@apps/club/src/app/global-error.tsx`:
- Around line 3-21: The GlobalError component is not accepting the error and
reset props that Next.js provides to global error boundaries, which prevents
users from recovering from errors without manual page refresh. Update the
GlobalError function signature to accept error and reset as parameters, then add
a clickable button element within the component that calls the reset function
when clicked to allow users to retry without requiring a full page reload.
In `@apps/club/src/app/globals.css`:
- Line 94: The text-rendering property value in the globals.css file uses
camelCase casing (geometricPrecision) but stylelint requires lowercase casing
(geometricprecision) for CI compliance. Locate the text-rendering property and
change its value from geometricPrecision to geometricprecision to match
stylelint's expected formatting rules.
In `@apps/club/src/app/hackathons/hackathon-history.tsx`:
- Around line 47-104: The baseTimelineRows array contains coordinates with
sub-pixel precision that lack documentation about their origin. Add a comment
block above the baseTimelineRows constant explaining that these precise
coordinate values come from Figma or design specifications, and clarify that
this precision should be maintained and not rounded to simpler values to ensure
accurate layout positioning.
In `@apps/club/src/app/sponsors/sponsors-config.ts`:
- Around line 150-366: The PAST_SPONSORS array uses inconsistent patterns to
access ONLINE_SPONSOR_LOGOS, mixing direct key access like
ONLINE_SPONSOR_LOGOS.github with bracket notation like
ONLINE_SPONSOR_LOGOS["lockheed martin"]. Create a helper function (e.g.,
getLogo) that takes a sponsor ID and returns the normalized logo URL from
ONLINE_SPONSOR_LOGOS by handling the key normalization internally. Then replace
all logoUrl assignments in the PAST_SPONSORS array to use this helper function
instead of direct ONLINE_SPONSOR_LOGOS access, ensuring consistent and
maintainable code.
🪄 Autofix (Beta)
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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: bf61c18d-e4a3-4836-9b46-69ebd834d40e
⛔ Files ignored due to path filters (33)
apps/bloomknights/public/event-banner.pngis excluded by!**/*.pngapps/club/public/community.jpgis excluded by!**/*.jpgapps/club/public/community.pngis excluded by!**/*.pngapps/club/public/hackathon.JPGis excluded by!**/*.jpgapps/club/public/hackathons/figma-card-real-0.pngis excluded by!**/*.pngapps/club/public/hackathons/figma-card-real-1.pngis excluded by!**/*.pngapps/club/public/hackathons/figma-card-real-2.pngis excluded by!**/*.pngapps/club/public/hackathons/figma-card-real-3.pngis excluded by!**/*.pngapps/club/public/hackathons/figma-card-real-4.pngis excluded by!**/*.pngapps/club/public/hackathons/figma-card-real-5.pngis excluded by!**/*.pngapps/club/public/hackathons/figma-card-real-6.pngis excluded by!**/*.pngapps/club/public/hackathons/figma-footer-logo.pngis excluded by!**/*.pngapps/club/public/hackathons/figma-timeline-extended.pngis excluded by!**/*.pngapps/club/public/hackathons/icon-email.pngis excluded by!**/*.pngapps/club/public/hackathons/icon-instagram.pngis excluded by!**/*.pngapps/club/public/hackathons/icon-linkedin.pngis excluded by!**/*.pngapps/club/public/hackathons/icon-twitter.pngis excluded by!**/*.pngapps/club/public/hackathons/logo-130.svgis excluded by!**/*.svgapps/club/public/hero/club-hero.mp4is excluded by!**/*.mp4apps/club/public/hero/club-hero.webmis excluded by!**/*.webmapps/club/public/kh-icon.svgis excluded by!**/*.svgapps/club/public/khtext.svgis excluded by!**/*.svgapps/club/public/knight-hacks-sets-you-apart.pngis excluded by!**/*.pngapps/club/public/knighthacks.svgis excluded by!**/*.svgapps/club/public/logos/microsoft.svgis excluded by!**/*.svgapps/club/public/members.JPGis excluded by!**/*.jpgapps/club/public/officers/bobda.pngis excluded by!**/*.pngapps/club/public/officers/daniel.pngis excluded by!**/*.pngapps/club/public/officers/ricky.pngis excluded by!**/*.pngapps/club/public/projects1.JPGis excluded by!**/*.jpgapps/club/public/projects2.jpgis excluded by!**/*.jpgapps/club/public/sigilKH.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (116)
apps/club/next.config.jsapps/club/package.jsonapps/club/public/community-card.webpapps/club/public/hackathons/figma-background.webpapps/club/public/hackathons/kh-history-hero.webpapps/club/public/hero/club-hero-poster.webpapps/club/public/projects-card.webpapps/club/public/workshop-card.webpapps/club/src/app/_components/club-content-page.tsxapps/club/src/app/_components/club-motion-runtime.tsxapps/club/src/app/_components/contact/assets/abstract-left.tsxapps/club/src/app/_components/contact/assets/abstract-right.tsxapps/club/src/app/_components/contact/assets/shield.tsxapps/club/src/app/_components/contact/assets/sword-left.tsxapps/club/src/app/_components/contact/assets/sword-right.tsxapps/club/src/app/_components/contact/contact-form.tsxapps/club/src/app/_components/contact/header.tsxapps/club/src/app/_components/contact/left-side.tsxapps/club/src/app/_components/contact/right-side.tsxapps/club/src/app/_components/footer.tsxapps/club/src/app/_components/home-community-carousel.tsxapps/club/src/app/_components/home-events.tsxapps/club/src/app/_components/json-ld.tsxapps/club/src/app/_components/landing/about.tsxapps/club/src/app/_components/landing/assets/coolbutton.tsxapps/club/src/app/_components/landing/assets/coolbutton2.tsxapps/club/src/app/_components/landing/assets/group.tsxapps/club/src/app/_components/landing/assets/neon-tk.tsxapps/club/src/app/_components/landing/assets/sword.tsxapps/club/src/app/_components/landing/assets/terminal.tsxapps/club/src/app/_components/landing/calendar.tsxapps/club/src/app/_components/landing/discover-assets/counter.tsxapps/club/src/app/_components/landing/discover-assets/discover-button.tsxapps/club/src/app/_components/landing/discover.tsxapps/club/src/app/_components/landing/hero-assets/hero-icon.tsxapps/club/src/app/_components/landing/hero-assets/typing-text.tsxapps/club/src/app/_components/landing/hero.tsxapps/club/src/app/_components/landing/impact-assets/expandable.tsxapps/club/src/app/_components/landing/impact-assets/wave-reveal.tsxapps/club/src/app/_components/landing/impact.tsxapps/club/src/app/_components/landing/sponsors-assets/sponsor-card.tsxapps/club/src/app/_components/landing/sponsors-assets/sponsor-marquee.tsxapps/club/src/app/_components/landing/sponsors.tsxapps/club/src/app/_components/legacy-redirect-page.tsxapps/club/src/app/_components/links/assets/abstract-shape-left-1.tsxapps/club/src/app/_components/links/assets/abstract-shape-left-2.tsxapps/club/src/app/_components/links/assets/abstract-shape-right-1.tsxapps/club/src/app/_components/links/assets/abstract-shape-right-2.tsxapps/club/src/app/_components/links/assets/binary-icon.tsxapps/club/src/app/_components/links/assets/blank-calendar.tsxapps/club/src/app/_components/links/assets/chat-bubble.tsxapps/club/src/app/_components/links/assets/facebook.tsxapps/club/src/app/_components/links/assets/instagram.tsxapps/club/src/app/_components/links/assets/kh-logo.tsxapps/club/src/app/_components/links/assets/knighthacks-text.tsxapps/club/src/app/_components/links/assets/laptop-charging.tsxapps/club/src/app/_components/links/assets/linkedin.tsxapps/club/src/app/_components/links/assets/mail.tsxapps/club/src/app/_components/links/assets/menu-horizontal.tsxapps/club/src/app/_components/links/assets/officer-card-assets/linkedin-icon.tsxapps/club/src/app/_components/links/assets/officer-card-assets/major.tsxapps/club/src/app/_components/links/assets/officer-card-assets/round-major.tsxapps/club/src/app/_components/links/assets/outline.tsxapps/club/src/app/_components/links/assets/terminal-icon.tsxapps/club/src/app/_components/links/assets/tk-neon-sign.tsxapps/club/src/app/_components/links/assets/tk-neon.tsxapps/club/src/app/_components/links/assets/twitter.tsxapps/club/src/app/_components/links/assets/youtube.tsxapps/club/src/app/_components/links/button.tsxapps/club/src/app/_components/links/links-header.tsxapps/club/src/app/_components/navbar.tsxapps/club/src/app/_components/navigation/mobile/desktop.tsxapps/club/src/app/_components/navigation/mobile/mobile-navbar.tsxapps/club/src/app/_components/navigation/mobile/nav-sheet.tsxapps/club/src/app/_components/navigation/navbar.tsxapps/club/src/app/_components/navigation/navlink.tsxapps/club/src/app/_components/not-found-redirect.tsxapps/club/src/app/_components/officers/assets/officer-card-assets/linkedin-icon.tsxapps/club/src/app/_components/officers/assets/officer-card-assets/major.tsxapps/club/src/app/_components/officers/assets/officer-card-assets/round-major.tsxapps/club/src/app/_components/officers/assets/officer-card.tsxapps/club/src/app/_components/officers/assets/officer-header-svg.tsxapps/club/src/app/_components/officers/header.tsxapps/club/src/app/_components/officers/officers.tsxapps/club/src/app/_lib/assets.tsapps/club/src/app/_lib/blade-trpc.tsapps/club/src/app/_lib/club-events.tsapps/club/src/app/_lib/site-config.tsapps/club/src/app/about/page.tsxapps/club/src/app/contact/page.tsxapps/club/src/app/events/events-client.tsxapps/club/src/app/events/page.tsxapps/club/src/app/global-error.tsxapps/club/src/app/globals.cssapps/club/src/app/hackathons/hackathon-history.tsxapps/club/src/app/hackathons/page.tsxapps/club/src/app/layout.tsxapps/club/src/app/links/page.tsxapps/club/src/app/not-found.tsxapps/club/src/app/officers/page.tsxapps/club/src/app/page.tsxapps/club/src/app/robots.tsapps/club/src/app/seo.tsapps/club/src/app/sitemap.tsapps/club/src/app/sponsors/page.tsxapps/club/src/app/sponsors/sponsors-client.tsxapps/club/src/app/sponsors/sponsors-config.tsapps/club/src/app/teams/page.tsxapps/club/src/app/teams/team-roster.tsapps/club/src/app/teams/teams-client.tsxapps/club/src/app/teams/teams-config.tsapps/club/src/app/trpc/server.tsapps/club/tailwind.config.tspackages/api/src/routers/event.tspackages/api/src/routers/guild.tspackages/consts/src/team.ts
💤 Files with no reviewable changes (63)
- apps/club/src/app/_components/contact/assets/shield.tsx
- apps/club/src/app/_components/contact/assets/abstract-left.tsx
- apps/club/src/app/_components/landing/sponsors-assets/sponsor-marquee.tsx
- apps/club/src/app/_components/links/assets/linkedin.tsx
- apps/club/src/app/_components/links/assets/instagram.tsx
- apps/club/src/app/_components/contact/right-side.tsx
- apps/club/src/app/_components/contact/header.tsx
- apps/club/src/app/_components/officers/officers.tsx
- apps/club/src/app/_components/links/button.tsx
- apps/club/src/app/_components/landing/sponsors.tsx
- apps/club/src/app/_components/links/assets/laptop-charging.tsx
- apps/club/src/app/_components/contact/assets/abstract-right.tsx
- apps/club/src/app/_components/landing/about.tsx
- apps/club/src/app/_components/landing/hero-assets/typing-text.tsx
- apps/club/src/app/_components/landing/assets/terminal.tsx
- apps/club/src/app/_components/navigation/navbar.tsx
- apps/club/src/app/_components/navigation/navlink.tsx
- apps/club/src/app/_components/links/assets/menu-horizontal.tsx
- apps/club/src/app/_components/links/assets/officer-card-assets/linkedin-icon.tsx
- apps/club/src/app/_components/links/assets/abstract-shape-left-1.tsx
- apps/club/src/app/_components/landing/impact.tsx
- apps/club/src/app/_components/landing/assets/neon-tk.tsx
- apps/club/src/app/_components/landing/assets/coolbutton.tsx
- apps/club/src/app/_components/officers/assets/officer-card-assets/round-major.tsx
- apps/club/src/app/_components/landing/hero-assets/hero-icon.tsx
- apps/club/src/app/_components/landing/assets/sword.tsx
- apps/club/src/app/_components/contact/contact-form.tsx
- apps/club/src/app/_components/links/assets/abstract-shape-right-1.tsx
- apps/club/src/app/_components/officers/assets/officer-card.tsx
- apps/club/src/app/_components/officers/assets/officer-card-assets/linkedin-icon.tsx
- apps/club/src/app/_components/links/assets/tk-neon-sign.tsx
- apps/club/src/app/_components/links/assets/mail.tsx
- apps/club/src/app/_components/landing/discover-assets/counter.tsx
- apps/club/src/app/_components/landing/calendar.tsx
- apps/club/src/app/_components/navigation/mobile/desktop.tsx
- apps/club/src/app/_components/links/assets/officer-card-assets/major.tsx
- apps/club/src/app/_components/links/assets/officer-card-assets/round-major.tsx
- apps/club/src/app/_components/landing/discover.tsx
- apps/club/src/app/_components/links/assets/abstract-shape-left-2.tsx
- apps/club/src/app/_components/navigation/mobile/mobile-navbar.tsx
- apps/club/src/app/_components/landing/discover-assets/discover-button.tsx
- apps/club/src/app/_components/officers/assets/officer-header-svg.tsx
- apps/club/src/app/_components/links/assets/chat-bubble.tsx
- apps/club/src/app/_components/links/assets/kh-logo.tsx
- apps/club/src/app/_components/officers/assets/officer-card-assets/major.tsx
- apps/club/src/app/_components/landing/assets/group.tsx
- apps/club/src/app/_components/links/assets/abstract-shape-right-2.tsx
- apps/club/src/app/_components/links/assets/facebook.tsx
- apps/club/src/app/_components/landing/impact-assets/expandable.tsx
- apps/club/src/app/_components/landing/impact-assets/wave-reveal.tsx
- apps/club/src/app/_components/links/assets/knighthacks-text.tsx
- apps/club/src/app/_components/links/links-header.tsx
- apps/club/src/app/_components/landing/sponsors-assets/sponsor-card.tsx
- apps/club/src/app/_components/landing/assets/coolbutton2.tsx
- apps/club/src/app/_components/links/assets/twitter.tsx
- apps/club/src/app/_components/officers/header.tsx
- apps/club/src/app/_components/landing/hero.tsx
- apps/club/src/app/_components/links/assets/youtube.tsx
- apps/club/src/app/_components/navigation/mobile/nav-sheet.tsx
- apps/club/src/app/_components/links/assets/blank-calendar.tsx
- apps/club/src/app/_components/links/assets/terminal-icon.tsx
- apps/club/src/app/_components/contact/left-side.tsx
- apps/club/src/app/_components/links/assets/tk-neon.tsx
Comment on lines
+30
to
+37
| export const CLUB_NAV_ITEMS = [ | ||
| { label: "Home", href: "/" }, | ||
| { label: "About", href: "/about" }, | ||
| { label: "Events", href: "/events" }, | ||
| { label: "Hackathons", href: "/hackathons" }, | ||
| { label: "Teams", href: "/teams" }, | ||
| { label: "Sponsors", href: "/sponsors" }, | ||
| ] as const; |
There was a problem hiding this comment.
Keep the primary nav aligned with the header design.
Navbar consumes CLUB_NAV_ITEMS directly, so About and Hackathons will render in the primary header wherever this constant is used. If the 4-link desktop/mobile spec from the PR is still the target, move those routes to a secondary/footer-only list instead.
🛠 Suggested split
export const CLUB_NAV_ITEMS = [
{ label: "Home", href: "/" },
- { label: "About", href: "/about" },
{ label: "Events", href: "/events" },
- { label: "Hackathons", href: "/hackathons" },
{ label: "Teams", href: "/teams" },
{ label: "Sponsors", href: "/sponsors" },
] as const;🤖 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 `@apps/club/src/app/_lib/site-config.ts` around lines 30 - 37, The
CLUB_NAV_ITEMS constant currently contains 6 navigation items, but the design
spec calls for a 4-link primary navigation in the header. Split this constant
into two separate navigation lists: keep Home, Events, Teams, and Sponsors in
the primary CLUB_NAV_ITEMS (which the Navbar component consumes), and create a
new secondary navigation constant for About and Hackathons that can be used in
footer or secondary navigation areas. This ensures the primary header navigation
aligns with the 4-link desktop/mobile specification.
Contributor
Follow-up review notesThis is much cleaner than the previous version — the unrelated DB/email/QR/resume/security changes are gone, and CI is green. I was also able to pass local checks after rebuilding pnpm --filter=@forge/api build
pnpm --filter=@forge/club typecheck
pnpm --filter=@forge/club lint
BLADE_URL=https://blade.knighthacks.org pnpm --filter=@forge/club build
pnpm --filter=@forge/api typecheck
pnpm --filter=@forge/consts typecheck
git diff --check origin/main...HEADRemaining concerns I noticed before merge:
Overall: this is a massive improvement and looks close, but there are still a few cleanup/ownership items before I would call it fully merge-ready. Dylan and I will address these ourselves tomorrow, so this is mostly a tracking note rather than a request for immediate changes tonight. |
Contributor
|
hello dylan yaya here are the responses / fixes
SummaryThis PR is the cleaned-up Club site refresh. It replaces the old static/officer/link/contact-heavy Club pages with the new public Club site experience, including the new home page, about page, events page, teams page, sponsors page, hackathon history timeline, shared nav/footer, SEO metadata, sitemap/robots, and static-export-safe legacy route handling. It is not purely frontend-only anymore because the Club site now needs public Blade/Guild-backed data to avoid hardcoding everything. This adds a small public Club data API surface:
The team roster endpoint only returns members with Cleanup / scope historyThis branch was cleaned up from the earlier oversized version. The unrelated DB/email/QR/resume/security changes were removed so the remaining scope is focused on the Club site and the API/const surfaces needed by the Club site. The remaining cross-package changes are intentional:
Review fixes addressed
ChecksThe branch has passing local/CI-equivalent checks for the Club/API work, including format/lint/typecheck/build after rebuilding the API types when needed. |
DVidal1205
dismissed stale reviews from farisAwheel, DanielJEfres, and coderabbitai[bot]
resolved
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
10 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.
Why
To clean up club site files so that we can start implementing Figma from design team.
What
Closes: #ISSUE_NUMBER
Doesn't close any issues but helps devs start work on their respective pages.
Notes:
Test Plan
Current Home Page:
Other page with sample text placeholder (we should remove this comp once we put actual content):
Mobile Nav
Hamburger Menu (we can add css animations imo)
Checklist
pnpm db:generateand committed the generated files inpackages/db/drizzle/Summary by CodeRabbit