Club-site clean up#449
Draft
myr124 wants to merge 8 commits into
Draft
Conversation
…classes to switch between the different types of buttons)
📝 WalkthroughWalkthroughThis PR refactors the Club app's navigation and page structure. It introduces a simplified ChangesNavigation and Page Infrastructure
Styling System Update
Legacy Page and Component Removal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes This PR involves substantial, wide-reaching deletions (50+ files) balanced by focused new components. While most individual removals are straightforward file deletions, the sheer volume, variety of asset types (SVGs, animation logic, form components, page layouts), and the coordinated introduction of a new navigation/shell system across multiple pages demand careful verification that no references to deleted exports remain and that new pages integrate correctly with the routing and styling updates. Suggested labels
🚥 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
apps/club/src/app/globals.css (5)
62-62: 💤 Low valueStylelint warning: CSS keyword should be lowercase.
CSS keywords should use lowercase per spec.
geometricPrecisionshould begeometricprecision.♻️ Fix Stylelint warning
- 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 62, The text-rendering CSS declaration uses a capitalized keyword; update the value in the text-rendering rule in globals.css (the text-rendering property) to the lowercase spec form "geometricprecision" so the keyword conforms to Stylelint/CSS spec.
3-3: 💤 Low valueStylelint warning: Font import syntax.
Stylelint expects the import without the
url()wrapper for@importstatements.♻️ Fix Stylelint warning
-@import url("https://fonts.googleapis.com/css2?family=Inter:wght@700&display=swap"); +@import "https://fonts.googleapis.com/css2?family=Inter:wght@700&display=swap";🤖 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 3, The `@import` line currently uses the url() wrapper which triggers a Stylelint warning; update the import statement in globals.css by removing the url() wrapper and use a plain string import (i.e. change `@import` url("https://fonts.googleapis.com/css2?family=Inter:wght@700&display=swap"); to the equivalent `@import` "https://fonts.googleapis.com/css2?family=Inter:wght@700&display=swap";) so Stylelint accepts the font import.
61-61: 💤 Low valueStylelint warnings: Remove quotes from font family names.
Stylelint's
font-family-name-quotesrule expects generic font family names and specific families without special characters to be unquoted.♻️ Fix Stylelint warnings
- font-family: "Prompt", var(--font-geist-sans), Arial, Helvetica, sans-serif; + font-family: Prompt, var(--font-geist-sans), Arial, Helvetica, sans-serif;Apply the same pattern to remove quotes from
"Pragati Narrow","Prompt", and"Inter"throughout the file (lines 144, 150, 157, 166).Also applies to: 144-144, 150-150, 157-157, 166-166
🤖 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 61, The CSS uses quoted font family names that trigger Stylelint's font-family-name-quotes rule; remove the unnecessary quotes from the font-family declarations (e.g., replace "Prompt", "Pragati Narrow", and "Inter" with unquoted Prompt, Pragati Narrow, Inter) wherever they appear in globals.css (search for the font-family: declarations including these names such as the existing line with font-family: "Prompt", var(--font-geist-sans), Arial, Helvetica, sans-serif and the other occurrences referenced) so that generic and simple family names are unquoted while keeping quotes only for names that require them.
182-192: ⚡ Quick winReduce duplication in button hover states.
The button variants repeat their background and color values in both default and hover states. This makes maintenance harder and increases CSS size.
♻️ Consolidate button styles
.club-button-dark { background: `#170d1c`; color: `#fff`; box-shadow: 4px 4px 0 var(--club-gold-soft); } .club-button-dark:hover { - background: `#170d1c`; - color: `#fff`; box-shadow: 6px 6px 0 var(--club-gold-soft); } .club-button-pink { background: linear-gradient(90deg, `#ffe1bd` 0%, `#fe88a4` 100%); color: `#000`; box-shadow: 4px 4px 0 `#f6a3bb`; } .club-button-pink:hover { - background: linear-gradient(90deg, `#ffe1bd` 0%, `#fe88a4` 100%); - color: `#000`; box-shadow: 6px 6px 0 `#f6a3bb`; }Also applies to: 194-203
🤖 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` around lines 182 - 192, The CSS duplicates background and color between the default and hover rules for club buttons (e.g., .club-button-dark); consolidate shared properties (background, color) into the base selector (.club-button-dark) and keep only the hover-specific changes (like box-shadow) in .club-button-dark:hover; apply the same consolidation pattern to the other button variant rules referenced (the other .club-button-* selectors) so hover rules only override what's different.
44-54: ⚡ Quick winConsider extracting hardcoded color values to CSS variables.
The button and navbar styles use hardcoded hex colors (
#170d1c,#ffe1bd,#fe88a4,#f6a3bb,#F65C2933,#00000038) that don't reference the--club-*variables defined in:root. This creates fragmentation and makes it harder to maintain a consistent color system.Per your PR note about using global CSS variables to avoid fragmented CSS, consider adding these colors to the
:rootblock and referencing them throughout. For example::root { /* existing variables... */ --club-button-bg-dark: `#170d1c`; --club-button-gradient-start: `#ffe1bd`; --club-button-gradient-end: `#fe88a4`; --club-button-shadow-pink: `#f6a3bb`; --club-navbar-bg: `#F65C2933`; --club-navbar-shadow: `#00000038`; }Then reference them in your classes and components.
🤖 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` around lines 44 - 54, Add new CSS variables to the existing :root block (e.g., --club-button-bg-dark, --club-button-gradient-start, --club-button-gradient-end, --club-button-shadow-pink, --club-navbar-bg, --club-navbar-shadow) and replace all hardcoded hex literals (`#170d1c`, `#ffe1bd`, `#fe88a4`, `#f6a3bb`, `#F65C2933`, `#00000038`) in your button and navbar styles with var(--club-...) references; search for those hex values in the CSS (button styles and navbar selectors) and update them to use the new variables so the color palette is centralized in :root.apps/club/src/app/_components/navbar.tsx (1)
52-52: ⚡ Quick winConsider extracting external URLs to configuration.
The Blade and Discord URLs are hardcoded. If these URLs change or vary by environment, you'll need to update them in multiple places.
♻️ Extract URLs to constants
At the top of the file:
+const EXTERNAL_LINKS = { + blade: "https://blade.knighthacks.org", + discord: "https://discord.gg/knighthacks", +} as const; + const navItems = [Then reference them:
- <a href="https://blade.knighthacks.org">Sign Up With Blade</a> + <a href={EXTERNAL_LINKS.blade}>Sign Up With Blade</a>- <a href="https://discord.gg/knighthacks">Join Discord</a> + <a href={EXTERNAL_LINKS.discord}>Join Discord</a>Also applies to: 59-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 `@apps/club/src/app/_components/navbar.tsx` at line 52, Extract hardcoded external URLs in navbar.tsx into named constants (e.g., BLADE_URL and DISCORD_URL) declared near the top of the module and replace the literal hrefs in the anchor elements (the "Sign Up With Blade" link and the Discord link) with those constants; ensure the constants are exported or centrally sourced if you have a config/env module so they can be changed per environment and referenced by other 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 `@apps/club/src/app/_components/navbar.tsx`:
- Around line 32-44: The nav links are only shown for large screens; add a
mobile menu by introducing a toggle state (e.g., useState called isMobileOpen)
in the navbar component and render a hamburger button (visible with lg:hidden)
that toggles isMobileOpen; when open, render a mobile menu panel (hidden on lg)
that maps navItems and uses the same Link rendering logic and pathname active
check to show the current item, ensure the button has accessible attributes
(aria-expanded, aria-controls) and the mobile panel uses appropriate classes
(e.g., fixed/absolute dropdown or slide-in with lg:hidden) so Teams, Events,
Sponsors are reachable on small screens.
- Around line 52-53: The external anchor inside the Navbar component (the <a>
wrapped by Button that links to https://blade.knighthacks.org and the other
external CTA at the same area) lacks rel="noopener" which is a security risk;
update those anchor elements in the Navbar (the anchors rendered inside Button
in navbar.tsx) to include rel="noopener" (or rel="noopener noreferrer") and, if
you want the link to open in a new tab, also add target="_blank" alongside rel
to follow the common pattern.
- Around line 46-61: The CTA buttons are hidden on small screens because the
wrapper div uses "hidden ... md:flex"; update navbar.tsx to surface at least one
CTA on mobile by either moving the Button elements into the mobile menu
component or adding a mobile-specific block (e.g., a div with "flex md:hidden"
or adding a duplicate Button with className "club-button club-button-pink px-7
md:hidden") so that the "Sign Up With Blade" and/or "Join Discord" anchors
remain accessible on mobile; target the Button elements with classNames
"club-button club-button-dark" and "club-button club-button-pink" when making
the change.
- Line 26: The Image element in navbar.tsx currently uses an empty alt attribute
which hides meaning from assistive tech; update the <Image src="/kh-icon.svg"
... /> instance (inside the Navbar component) to provide a descriptive alt
string such as "Knight Hacks logo" or "Knight Hacks logo, home" so screen
readers can convey the image purpose while keeping the existing aria-label on
the surrounding link.
---
Nitpick comments:
In `@apps/club/src/app/_components/navbar.tsx`:
- Line 52: Extract hardcoded external URLs in navbar.tsx into named constants
(e.g., BLADE_URL and DISCORD_URL) declared near the top of the module and
replace the literal hrefs in the anchor elements (the "Sign Up With Blade" link
and the Discord link) with those constants; ensure the constants are exported or
centrally sourced if you have a config/env module so they can be changed per
environment and referenced by other components.
In `@apps/club/src/app/globals.css`:
- Line 62: The text-rendering CSS declaration uses a capitalized keyword; update
the value in the text-rendering rule in globals.css (the text-rendering
property) to the lowercase spec form "geometricprecision" so the keyword
conforms to Stylelint/CSS spec.
- Line 3: The `@import` line currently uses the url() wrapper which triggers a
Stylelint warning; update the import statement in globals.css by removing the
url() wrapper and use a plain string import (i.e. change `@import`
url("https://fonts.googleapis.com/css2?family=Inter:wght@700&display=swap"); to
the equivalent `@import`
"https://fonts.googleapis.com/css2?family=Inter:wght@700&display=swap";) so
Stylelint accepts the font import.
- Line 61: The CSS uses quoted font family names that trigger Stylelint's
font-family-name-quotes rule; remove the unnecessary quotes from the font-family
declarations (e.g., replace "Prompt", "Pragati Narrow", and "Inter" with
unquoted Prompt, Pragati Narrow, Inter) wherever they appear in globals.css
(search for the font-family: declarations including these names such as the
existing line with font-family: "Prompt", var(--font-geist-sans), Arial,
Helvetica, sans-serif and the other occurrences referenced) so that generic and
simple family names are unquoted while keeping quotes only for names that
require them.
- Around line 182-192: The CSS duplicates background and color between the
default and hover rules for club buttons (e.g., .club-button-dark); consolidate
shared properties (background, color) into the base selector (.club-button-dark)
and keep only the hover-specific changes (like box-shadow) in
.club-button-dark:hover; apply the same consolidation pattern to the other
button variant rules referenced (the other .club-button-* selectors) so hover
rules only override what's different.
- Around line 44-54: Add new CSS variables to the existing :root block (e.g.,
--club-button-bg-dark, --club-button-gradient-start, --club-button-gradient-end,
--club-button-shadow-pink, --club-navbar-bg, --club-navbar-shadow) and replace
all hardcoded hex literals (`#170d1c`, `#ffe1bd`, `#fe88a4`, `#f6a3bb`, `#F65C2933`,
`#00000038`) in your button and navbar styles with var(--club-...) references;
search for those hex values in the CSS (button styles and navbar selectors) and
update them to use the new variables so the color palette is centralized in
:root.
🪄 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: 0d73d934-9c58-4ff4-b076-db3e13261473
⛔ Files ignored due to path filters (2)
apps/club/public/kh-icon.svgis excluded by!**/*.svgapps/club/src/home.svgis excluded by!**/*.svg
📒 Files selected for processing (79)
apps/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/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/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/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/_components/section-page-shell.tsxapps/club/src/app/contact/page.tsxapps/club/src/app/events/page.tsxapps/club/src/app/globals.cssapps/club/src/app/layout.tsxapps/club/src/app/links/page.tsxapps/club/src/app/officers/page.tsxapps/club/src/app/page.tsxapps/club/src/app/sponsors/page.tsxapps/club/src/app/teams/page.tsxapps/club/src/app/trpc/server.ts
💤 Files with no reviewable changes (68)
- apps/club/src/app/_components/contact/assets/abstract-left.tsx
- apps/club/src/app/_components/links/assets/abstract-shape-right-2.tsx
- apps/club/src/app/_components/links/assets/mail.tsx
- apps/club/src/app/_components/landing/hero-assets/typing-text.tsx
- apps/club/src/app/_components/links/assets/officer-card-assets/linkedin-icon.tsx
- apps/club/src/app/_components/landing/impact.tsx
- apps/club/src/app/_components/links/assets/youtube.tsx
- apps/club/src/app/_components/links/assets/blank-calendar.tsx
- apps/club/src/app/_components/links/assets/menu-horizontal.tsx
- apps/club/src/app/_components/links/assets/abstract-shape-left-2.tsx
- apps/club/src/app/_components/landing/hero-assets/hero-icon.tsx
- apps/club/src/app/links/page.tsx
- apps/club/src/app/_components/links/assets/tk-neon.tsx
- apps/club/src/app/_components/links/assets/knighthacks-text.tsx
- apps/club/src/app/_components/officers/assets/officer-card-assets/major.tsx
- apps/club/src/app/_components/landing/discover-assets/discover-button.tsx
- apps/club/src/app/_components/officers/assets/officer-card-assets/linkedin-icon.tsx
- apps/club/src/app/_components/links/assets/abstract-shape-left-1.tsx
- apps/club/src/app/_components/links/assets/instagram.tsx
- apps/club/src/app/officers/page.tsx
- apps/club/src/app/_components/links/assets/twitter.tsx
- apps/club/src/app/_components/officers/officers.tsx
- apps/club/src/app/_components/landing/assets/coolbutton.tsx
- apps/club/src/app/_components/contact/right-side.tsx
- apps/club/src/app/contact/page.tsx
- apps/club/src/app/_components/navigation/navbar.tsx
- apps/club/src/app/_components/links/assets/kh-logo.tsx
- apps/club/src/app/_components/landing/hero.tsx
- apps/club/src/app/_components/landing/assets/group.tsx
- apps/club/src/app/_components/officers/assets/officer-card.tsx
- apps/club/src/app/_components/navigation/mobile/desktop.tsx
- apps/club/src/app/_components/links/assets/facebook.tsx
- apps/club/src/app/_components/links/assets/chat-bubble.tsx
- apps/club/src/app/_components/landing/discover.tsx
- apps/club/src/app/_components/officers/header.tsx
- apps/club/src/app/_components/contact/header.tsx
- apps/club/src/app/_components/links/links-header.tsx
- apps/club/src/app/trpc/server.ts
- apps/club/src/app/_components/landing/discover-assets/counter.tsx
- apps/club/src/app/_components/navigation/mobile/nav-sheet.tsx
- apps/club/src/app/_components/links/button.tsx
- apps/club/src/app/_components/navigation/navlink.tsx
- apps/club/src/app/_components/links/assets/officer-card-assets/round-major.tsx
- apps/club/src/app/_components/links/assets/linkedin.tsx
- apps/club/src/app/_components/landing/assets/terminal.tsx
- apps/club/src/app/layout.tsx
- apps/club/src/app/_components/landing/sponsors.tsx
- apps/club/src/app/_components/landing/assets/coolbutton2.tsx
- apps/club/src/app/_components/landing/about.tsx
- apps/club/src/app/_components/navigation/mobile/mobile-navbar.tsx
- apps/club/src/app/_components/contact/assets/shield.tsx
- apps/club/src/app/_components/links/assets/terminal-icon.tsx
- apps/club/src/app/_components/officers/assets/officer-header-svg.tsx
- apps/club/src/app/_components/contact/left-side.tsx
- apps/club/src/app/_components/contact/assets/abstract-right.tsx
- apps/club/src/app/_components/landing/calendar.tsx
- apps/club/src/app/_components/landing/sponsors-assets/sponsor-card.tsx
- apps/club/src/app/_components/landing/assets/sword.tsx
- apps/club/src/app/_components/landing/impact-assets/wave-reveal.tsx
- apps/club/src/app/_components/links/assets/laptop-charging.tsx
- apps/club/src/app/_components/links/assets/tk-neon-sign.tsx
- apps/club/src/app/_components/officers/assets/officer-card-assets/round-major.tsx
- apps/club/src/app/_components/landing/assets/neon-tk.tsx
- apps/club/src/app/_components/links/assets/abstract-shape-right-1.tsx
- apps/club/src/app/_components/links/assets/officer-card-assets/major.tsx
- apps/club/src/app/_components/landing/impact-assets/expandable.tsx
- apps/club/src/app/_components/contact/contact-form.tsx
- apps/club/src/app/_components/landing/sponsors-assets/sponsor-marquee.tsx
| className="flex shrink-0 items-center gap-3" | ||
| aria-label="Knight Hacks home" | ||
| > | ||
| <Image src="/kh-icon.svg" alt="" width={40} height={40} priority /> |
There was a problem hiding this comment.
Add descriptive alt text for the logo image.
The logo image has an empty alt="" attribute. While the link has aria-label="Knight Hacks home", screen reader users navigating by images won't get context about what the image represents.
♿ Proposed fix
- <Image src="/kh-icon.svg" alt="" width={40} height={40} priority />
+ <Image src="/kh-icon.svg" alt="Knight Hacks lightning bolt logo" width={40} height={40} priority />📝 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
| <Image src="/kh-icon.svg" alt="" width={40} height={40} priority /> | |
| <Image src="/kh-icon.svg" alt="Knight Hacks lightning bolt logo" width={40} height={40} priority /> |
🤖 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/navbar.tsx` at line 26, The Image element in
navbar.tsx currently uses an empty alt attribute which hides meaning from
assistive tech; update the <Image src="/kh-icon.svg" ... /> instance (inside the
Navbar component) to provide a descriptive alt string such as "Knight Hacks
logo" or "Knight Hacks logo, home" so screen readers can convey the image
purpose while keeping the existing aria-label on the surrounding link.
Comment on lines
+32
to
+44
| <div className="hidden items-center gap-12 lg:flex"> | ||
| {navItems.map((item) => ( | ||
| <Link | ||
| key={item.href} | ||
| href={item.href} | ||
| className={`font-inter text-[13px] font-bold uppercase leading-[19.5px] tracking-[0.57px] transition hover:text-[var(--club-gold)] ${ | ||
| pathname === item.href ? "text-[var(--club-gold)]" : "text-white" | ||
| }`} | ||
| > | ||
| {item.label} | ||
| </Link> | ||
| ))} | ||
| </div> |
There was a problem hiding this comment.
Missing mobile navigation menu.
The navigation links are hidden on small screens (lg:flex on Line 32), but there's no hamburger menu or mobile navigation alternative. Users on mobile devices won't be able to access Teams, Events, or Sponsors pages.
Consider adding a mobile menu component with a hamburger toggle button.
🤖 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/navbar.tsx` around lines 32 - 44, The nav links
are only shown for large screens; add a mobile menu by introducing a toggle
state (e.g., useState called isMobileOpen) in the navbar component and render a
hamburger button (visible with lg:hidden) that toggles isMobileOpen; when open,
render a mobile menu panel (hidden on lg) that maps navItems and uses the same
Link rendering logic and pathname active check to show the current item, ensure
the button has accessible attributes (aria-expanded, aria-controls) and the
mobile panel uses appropriate classes (e.g., fixed/absolute dropdown or slide-in
with lg:hidden) so Teams, Events, Sponsors are reachable on small screens.
Comment on lines
+46
to
+61
| <div className="hidden shrink-0 items-center gap-3 md:flex"> | ||
| <Button | ||
| asChild | ||
| size="lg" | ||
| className="club-button club-button-dark" | ||
| > | ||
| <a href="https://blade.knighthacks.org">Sign Up With Blade</a> | ||
| </Button> | ||
| <Button | ||
| asChild | ||
| size="lg" | ||
| className="club-button club-button-pink px-7" | ||
| > | ||
| <a href="https://discord.gg/knighthacks">Join Discord</a> | ||
| </Button> | ||
| </div> |
There was a problem hiding this comment.
CTA buttons hidden on mobile.
The "Sign Up With Blade" and "Join Discord" buttons are hidden below the md breakpoint (Line 46: md:flex), making these important CTAs inaccessible on small screens.
Consider displaying at least one CTA button on mobile, or include both in the mobile menu.
🤖 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/navbar.tsx` around lines 46 - 61, The CTA
buttons are hidden on small screens because the wrapper div uses "hidden ...
md:flex"; update navbar.tsx to surface at least one CTA on mobile by either
moving the Button elements into the mobile menu component or adding a
mobile-specific block (e.g., a div with "flex md:hidden" or adding a duplicate
Button with className "club-button club-button-pink px-7 md:hidden") so that the
"Sign Up With Blade" and/or "Join Discord" anchors remain accessible on mobile;
target the Button elements with classNames "club-button club-button-dark" and
"club-button club-button-pink" when making the change.
Comment on lines
+52
to
+53
| <a href="https://blade.knighthacks.org">Sign Up With Blade</a> | ||
| </Button> |
There was a problem hiding this comment.
Add rel="noopener" to external links for security.
External links should include rel="noopener" to prevent the new page from accessing window.opener and protect against potential security vulnerabilities.
🔒 Proposed fix
- <a href="https://blade.knighthacks.org">Sign Up With Blade</a>
+ <a href="https://blade.knighthacks.org" target="_blank" rel="noopener noreferrer">Sign Up With Blade</a>- <a href="https://discord.gg/knighthacks">Join Discord</a>
+ <a href="https://discord.gg/knighthacks" target="_blank" rel="noopener noreferrer">Join Discord</a>Note: Adding target="_blank" is optional but common for external CTAs; rel="noopener noreferrer" is the security fix.
Also applies to: 59-60
🤖 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/navbar.tsx` around lines 52 - 53, The external
anchor inside the Navbar component (the <a> wrapped by Button that links to
https://blade.knighthacks.org and the other external CTA at the same area) lacks
rel="noopener" which is a security risk; update those anchor elements in the
Navbar (the anchors rendered inside Button in navbar.tsx) to include
rel="noopener" (or rel="noopener noreferrer") and, if you want the link to open
in a new tab, also add target="_blank" alongside rel to follow the common
pattern.
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
1 participant
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):
Checklist
pnpm db:generateand committed the generated files inpackages/db/drizzle/Summary by CodeRabbit
New Features
Bug Fixes / Removals
Style