Conversation
…d bump pnpm version
…ons and responsive layout
WalkthroughUpdated package metadata and dev scripts; converted Header to a client, responsive component with mobile/desktop branching and new navigation UI; added a Radix-based NavigationMenu component suite; adjusted Glass default background and removed header CSS variable usage from layout; Hero min-height changed to full viewport. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Header
participant useMediaQuery
participant NavMenu
User->>Header: load page
Header->>useMediaQuery: evaluate "(max-width: 768px)"
useMediaQuery-->>Header: isMobile (true/false)
alt isMobile == true
Header->>Header: render MobileLayout (Logo, IconButtonsMobile, mobile menu)
else isMobile == false
Header->>NavMenu: render DesktopLayout (Logo, NavigationMenu, IconButtonsDesktop)
NavMenu->>NavMenu: handle triggers (open/close), render Content/Viewport
end
User->>Header: interact (click icons/nav)
Header->>NavMenu: invoke nav interactions (handlers)
🚥 Pre-merge checks | ❌ 4❌ Failed checks (2 warnings, 2 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/components/ui/navigation-menu.tsx (1)
59-66: Remove unnecessary empty string literal.Line 61 contains an empty string
{""}that serves no purpose and adds noise to the JSX.🧹 Proposed fix
> {children} - {""} <ChevronDownIcon className="relative top-[1px] ml-1 size-3 transition duration-300 group-data-[state=open]:rotate-180" aria-hidden="true" /> </NavigationMenuPrimitive.Trigger>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/navigation-menu.tsx` around lines 59 - 66, Remove the unnecessary empty string literal inside the NavigationMenuPrimitive.Trigger render block — specifically delete the `{""}` between `children` and the `ChevronDownIcon` in the `navigation-menu.tsx` component; ensure spacing is handled via CSS or the existing `ml-1` on `ChevronDownIcon` rather than an inline empty string so only `children` and the `ChevronDownIcon` remain inside `NavigationMenuPrimitive.Trigger`.src/components/header.tsx (1)
69-80: Placeholder handlers detected.These handlers only log to the console. Consider adding a TODO comment or tracking issue to implement the actual functionality (user profile modal, language selector, theme toggle).
Would you like me to open an issue to track implementing these icon button handlers?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/header.tsx` around lines 69 - 80, Replace the placeholder console.log statements in handleUserIconClick, handleGlobeIconClick, and handleMoonIconClick with actionable TODOs and optional hookup points: add TODO comments referencing a tracking issue/ID and call prop callbacks if provided (e.g., onOpenProfile, onOpenLanguageSelector, onToggleTheme) with a safe fallback to the existing console.log so behavior stays unchanged until implemented; update the component props/interface to include these callbacks if not present and ensure handlers call those prop functions (handleUserIconClick → onOpenProfile, handleGlobeIconClick → onOpenLanguageSelector, handleMoonIconClick → onToggleTheme) or log the TODO when callbacks are absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/glass.tsx`:
- Around line 7-10: The Glass component currently applies a default
semi-transparent background via the class "bg-white/40" which forces a
white-tinted background on all uses (see Glass component's JSX using cn(...,
props.className)); update Glass to remove the hardcoded "bg-white/40" default so
instances inherit their background or explicit overrides (e.g., Input and Select
use "bg-background-blur"); specifically, modify the Glass component's className
construction to only include border, blur and spacing defaults and let callers
supply any background class (or alternatively make the background conditional
based on a prop like "transparent?: boolean"), then ensure header usages (mobile
nav, desktop nav, and dropdown) either pass the appropriate background override
class (e.g., "bg-background-blur") or rely on the new default-less behavior.
In `@src/components/header.tsx`:
- Around line 90-102: The IconButtonsMobile component currently uses Link
href="#" for action handlers which causes unwanted navigation; replace each
<Link href="#"> usage in IconButtonsMobile with a <button type="button">
preserving the onClick handlers (handleUserIconClick, handleGlobeIconClick,
handleMoonIconClick), the aria-labels ("User profile", "Language", "Theme"), and
the existing className and icon children so these are semantic, non-navigating
action buttons and avoid scroll-to-top/browser history effects.
- Around line 104-122: IconButtonsDesktop has invalid Tailwind class names and
uses Link anchors for actions; change each "color-text-primary" to
"text-text-primary" and replace the <Link href="#"> elements inside
NavigationMenuLink asChild with <button type="button"> elements that keep the
same onClick handlers (handleUserIconClick, handleGlobeIconClick,
handleMoonIconClick) and aria-labels, remove any href attributes, and ensure the
button receives the same className={`${removeHoverClass} p-0`} and contains the
FiUser/FiGlobe/FiMoon icons; also remove any now-unused href props passed from
parents (e.g., places that were only supplying href="#" to these icon links).
---
Nitpick comments:
In `@src/components/header.tsx`:
- Around line 69-80: Replace the placeholder console.log statements in
handleUserIconClick, handleGlobeIconClick, and handleMoonIconClick with
actionable TODOs and optional hookup points: add TODO comments referencing a
tracking issue/ID and call prop callbacks if provided (e.g., onOpenProfile,
onOpenLanguageSelector, onToggleTheme) with a safe fallback to the existing
console.log so behavior stays unchanged until implemented; update the component
props/interface to include these callbacks if not present and ensure handlers
call those prop functions (handleUserIconClick → onOpenProfile,
handleGlobeIconClick → onOpenLanguageSelector, handleMoonIconClick →
onToggleTheme) or log the TODO when callbacks are absent.
In `@src/components/ui/navigation-menu.tsx`:
- Around line 59-66: Remove the unnecessary empty string literal inside the
NavigationMenuPrimitive.Trigger render block — specifically delete the `{""}`
between `children` and the `ChevronDownIcon` in the `navigation-menu.tsx`
component; ensure spacing is handled via CSS or the existing `ml-1` on
`ChevronDownIcon` rather than an inline empty string so only `children` and the
`ChevronDownIcon` remain inside `NavigationMenuPrimitive.Trigger`.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1d5df7cc-30bc-407c-8367-c4c56b6c27ae
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
package.jsonsrc/components/glass.tsxsrc/components/header.tsxsrc/components/ui/navigation-menu.tsx
Docstrings generation was requested by @itasimo. * #57 (comment) The following files were modified: * `src/components/glass.tsx` * `src/components/header.tsx` * `src/components/ui/navigation-menu.tsx`
| const components: { | ||
| title: string | ||
| href?: string | ||
| menu: { title: string; href: string; icon: React.ElementType }[] | null | ||
| }[] = [ |
There was a problem hiding this comment.
cambia nome pls
definisci i tipi in modo più specifico
| const components: { | |
| title: string | |
| href?: string | |
| menu: { title: string; href: string; icon: React.ElementType }[] | null | |
| }[] = [ | |
| export type HeaderSubmenuItem = { | |
| title: string | |
| href: string | |
| icon: React.ElementType | |
| } | |
| export type HeaderMenuItem = { | |
| title: string | |
| href: string | |
| } | { | |
| title: string | |
| menu: HeaderSubmenuItem[] | |
| } | |
| const components: { | |
| title: string | |
| href?: string | |
| menu: { title: string; href: string; icon: React.ElementType }[] | null | |
| }[] = [ |
| height={40} | ||
| /> | ||
| <h1 className="hidden font-bold text-2xl text-accent-foreground md:block">PoliNetwork</h1> | ||
| <Glass className="fixed top-[40px] z-20 m-auto mt-0 mb-0 box-border w-full max-w-[354px] rounded-[36px] px-[24px]! py-[20px]!"> |
|
|
||
| // --- Main Header Component --- | ||
| export function Header() { | ||
| const [mounted, setMounted] = React.useState(false) |
There was a problem hiding this comment.
nitpick: importa direttamente useState senza usare React come global import
| const [mounted, setMounted] = React.useState(false) | ||
| const isMobile = useMediaQuery("(max-width: 768px)") // TODO: Adjust breakpoint | ||
|
|
||
| React.useEffect(() => { | ||
| setMounted(true) | ||
| }, []) | ||
|
|
||
| // Wait for hydration to prevent flash. Default to mobile layout under the assumption that most traffic is mobile. | ||
| if (!mounted) { | ||
| return <MobileLayout /> | ||
| } | ||
|
|
||
| return isMobile ? <MobileLayout /> : <DesktopLayout /> |
There was a problem hiding this comment.
renderizza entrambi e dai md:hidden a MobileLayout e max-md:hidden a DesktopLayout
| const [mounted, setMounted] = React.useState(false) | |
| const isMobile = useMediaQuery("(max-width: 768px)") // TODO: Adjust breakpoint | |
| React.useEffect(() => { | |
| setMounted(true) | |
| }, []) | |
| // Wait for hydration to prevent flash. Default to mobile layout under the assumption that most traffic is mobile. | |
| if (!mounted) { | |
| return <MobileLayout /> | |
| } | |
| return isMobile ? <MobileLayout /> : <DesktopLayout /> | |
| return <> | |
| <MobileLayout /> | |
| <DesktopLayout /> | |
| </> |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/components/ui/navigation-menu.tsx (1)
59-65: Remove unnecessary empty string literal.The
{""}on line 61 serves no purpose and can be removed.🧹 Proposed fix
> {children} - {""} <FiChevronDown className="relative top-[1px] ml-1 size-3 transition duration-300 group-data-[state=open]:rotate-180" aria-hidden="true" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/navigation-menu.tsx` around lines 59 - 65, The JSX contains an unnecessary empty string literal between the children and the icon in the NavigationMenuTrigger component; remove the `{""}` so that the block simply renders `{children}` followed directly by the `<FiChevronDown ... />` element (refer to the JSX around `children` and the `FiChevronDown` icon in `navigation-menu.tsx`) and ensure spacing/whitespace is preserved as needed.src/components/header.tsx (2)
70-80: Console.log statements should be removed or replaced with actual functionality.These handlers appear to be debug placeholders. Consider removing them or implementing the actual functionality (e.g., opening modals, toggling theme).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/header.tsx` around lines 70 - 80, The three click handlers (handleUserIconClick, handleGlobeIconClick, handleMoonIconClick) currently only call console.log and should be replaced with real behavior: remove the console.log calls and either invoke the appropriate prop callbacks or implement the intended actions (e.g., have handleUserIconClick open the user/profile menu or call onOpenProfile(), have handleGlobeIconClick open a locale selector or call onOpenLocale(), and have handleMoonIconClick toggle theme via a theme context or call toggleTheme()). Locate these functions in src/components/header.tsx and wire them to the existing props, context methods, or state-updating logic used elsewhere in the component so the icons perform their real functions instead of logging.
39-67: All navigation links use placeholderhref="#".The navigation menu data contains only placeholder hrefs. Consider using actual routes or adding a TODO comment to track this work.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/header.tsx` around lines 39 - 67, The menu items in src/components/header.tsx currently have placeholder href="#" values; update each menu item's href property in the header component's menu arrays (the objects with title "Resources", "Community", "About" and their nested menu entries) to point to the actual routes/URLs you intend to use, or if routes are not ready add a clear TODO comment on each item (e.g., TODO: replace '#' with '/path') so the missing routes are tracked; ensure you update the href values for entries like "Materials", "Guides", "Computer Recs", "FAQs", "Rankings", "Tol Project", "Groups", "Projects", "Freshman", "Associations", "About us", "Join us", and "Contact us" accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/layout.tsx`:
- Line 69: Define the missing CSS variable --header-height so references in
Header (component Header or header.tsx) and the NotFound page (not-found.tsx)
resolve: add a global declaration for --header-height (for example on the body
or :root in layout.tsx or globals.css) with an appropriate value (e.g., height
of the header like 64px or a responsive CSS value) so that class names
max-h-[var(--header-height)] in Header and
min-h-[calc(100dvh-var(--header-height))] in not-found.tsx work correctly;
ensure the variable name exactly matches --header-height and is loaded before
those components render.
In `@src/components/header.tsx`:
- Around line 202-205: The Tailwind class used on the icon's className is
incorrect: replace any occurrence of "color-text-primary" with the correct
Tailwind utility "text-text-primary" (e.g., on the FiChevronDown JSX element and
the other identical class occurrence in this Header component) so the styling
matches the project's pattern; update the className strings in the Header
component where "color-text-primary" appears to "text-text-primary".
- Line 185: The class in NavigationMenu (header.tsx) still uses
max-h-[var(--header-height)] but the --header-height variable was removed from
layout.tsx, so update the class to use a valid height: either swap
var(--header-height) for an existing site variable (e.g.,
var(--site-header-height) if available) or provide a sensible fallback/hardcoded
value (e.g., max-h-[64px] or max-h-[var(--site-header-height,64px)]), ensuring
the change is applied in header.tsx's className string and removing any stale
reliance on --header-height.
---
Nitpick comments:
In `@src/components/header.tsx`:
- Around line 70-80: The three click handlers (handleUserIconClick,
handleGlobeIconClick, handleMoonIconClick) currently only call console.log and
should be replaced with real behavior: remove the console.log calls and either
invoke the appropriate prop callbacks or implement the intended actions (e.g.,
have handleUserIconClick open the user/profile menu or call onOpenProfile(),
have handleGlobeIconClick open a locale selector or call onOpenLocale(), and
have handleMoonIconClick toggle theme via a theme context or call
toggleTheme()). Locate these functions in src/components/header.tsx and wire
them to the existing props, context methods, or state-updating logic used
elsewhere in the component so the icons perform their real functions instead of
logging.
- Around line 39-67: The menu items in src/components/header.tsx currently have
placeholder href="#" values; update each menu item's href property in the header
component's menu arrays (the objects with title "Resources", "Community",
"About" and their nested menu entries) to point to the actual routes/URLs you
intend to use, or if routes are not ready add a clear TODO comment on each item
(e.g., TODO: replace '#' with '/path') so the missing routes are tracked; ensure
you update the href values for entries like "Materials", "Guides", "Computer
Recs", "FAQs", "Rankings", "Tol Project", "Groups", "Projects", "Freshman",
"Associations", "About us", "Join us", and "Contact us" accordingly.
In `@src/components/ui/navigation-menu.tsx`:
- Around line 59-65: The JSX contains an unnecessary empty string literal
between the children and the icon in the NavigationMenuTrigger component; remove
the `{""}` so that the block simply renders `{children}` followed directly by
the `<FiChevronDown ... />` element (refer to the JSX around `children` and the
`FiChevronDown` icon in `navigation-menu.tsx`) and ensure spacing/whitespace is
preserved as needed.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ed7ef850-8413-4a9a-952a-59527b85f4c5
📒 Files selected for processing (4)
src/app/layout.tsxsrc/components/header.tsxsrc/components/home/hero.tsxsrc/components/ui/navigation-menu.tsx
| } as React.CSSProperties | ||
| } | ||
| > | ||
| <body className="overflow-y-scroll"> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all remaining usages of --header-height CSS variable
rg -n '\-\-header-height' --type-add 'web:*.{tsx,jsx,ts,js,css,scss}' --type webRepository: PoliNetworkOrg/web
Length of output: 378
🏁 Script executed:
# Check layout.tsx around line 69 to see the body element
sed -n '60,75p' src/app/layout.tsxRepository: PoliNetworkOrg/web
Length of output: 665
🏁 Script executed:
# Check if --header-height is defined in CSS/globals files
rg '\-\-header-height\s*:' --type-add 'web:*.{tsx,jsx,ts,js,css,scss}' --type webRepository: PoliNetworkOrg/web
Length of output: 44
🏁 Script executed:
# Check not-found.tsx to verify the exact line with the usage
sed -n '1,15p' src/app/not-found.tsxRepository: PoliNetworkOrg/web
Length of output: 862
🏁 Script executed:
# Search in all CSS/SCSS files for --header-height definition
fd -e css -e scss | xargs rg '\-\-header-height' -nRepository: PoliNetworkOrg/web
Length of output: 46
🏁 Script executed:
# Also check for any style tags or CSS-in-JS that might define it
rg 'style\s*=' src/ -A 2 | head -50Repository: PoliNetworkOrg/web
Length of output: 44
🏁 Script executed:
# Look for globals.css or similar root style files
fd -e css -e scss | grep -i globalRepository: PoliNetworkOrg/web
Length of output: 84
🏁 Script executed:
# Read globals.css to check if --header-height is defined
cat -n src/styles/globals.cssRepository: PoliNetworkOrg/web
Length of output: 555
🏁 Script executed:
# Check the imported CSS files for --header-height definition
for file in src/styles/{typography,figma,sidebar,charts}.css; do
echo "=== $file ==="
cat "$file" | grep -n '\-\-header-height' || echo "Not found"
doneRepository: PoliNetworkOrg/web
Length of output: 225
--header-height CSS variable is undefined but used in multiple places, breaking layout.
The CSS variable --header-height is referenced in two files but never defined anywhere in the codebase:
src/components/header.tsx:185:max-h-[var(--header-height)]src/app/not-found.tsx:9:min-h-[calc(100dvh-var(--header-height))]
This undefined variable will cause layout failures in both the header component and the 404 page. Either define the variable (e.g., in layout.tsx body inline styles or globals.css) or remove its usage from these files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/layout.tsx` at line 69, Define the missing CSS variable
--header-height so references in Header (component Header or header.tsx) and the
NotFound page (not-found.tsx) resolve: add a global declaration for
--header-height (for example on the body or :root in layout.tsx or globals.css)
with an appropriate value (e.g., height of the header like 64px or a responsive
CSS value) so that class names max-h-[var(--header-height)] in Header and
min-h-[calc(100dvh-var(--header-height))] in not-found.tsx work correctly;
ensure the variable name exactly matches --header-height and is loaded before
those components render.
| <Glass className="fixed top-[40px] z-20 m-auto mt-0 mb-0 box-border w-full max-w-[1045px] rounded-full px-[70px]! py-[20px]!"> | ||
| <NavigationMenu | ||
| viewport={false} | ||
| className="top-0 isolate flex max-h-[var(--header-height)] max-w-full shrink-0 items-stretch bg-card [&>div]:w-full" |
There was a problem hiding this comment.
Stale reference to removed --header-height CSS variable.
The NavigationMenu component references var(--header-height) which was removed from layout.tsx. This will result in the variable being undefined.
🔧 Proposed fix
<NavigationMenu
viewport={false}
- className="top-0 isolate flex max-h-[var(--header-height)] max-w-full shrink-0 items-stretch bg-card [&>div]:w-full"
+ className="top-0 isolate flex max-w-full shrink-0 items-stretch bg-card [&>div]:w-full"
>📝 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.
| className="top-0 isolate flex max-h-[var(--header-height)] max-w-full shrink-0 items-stretch bg-card [&>div]:w-full" | |
| className="top-0 isolate flex max-w-full shrink-0 items-stretch bg-card [&>div]:w-full" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/header.tsx` at line 185, The class in NavigationMenu
(header.tsx) still uses max-h-[var(--header-height)] but the --header-height
variable was removed from layout.tsx, so update the class to use a valid height:
either swap var(--header-height) for an existing site variable (e.g.,
var(--site-header-height) if available) or provide a sensible fallback/hardcoded
value (e.g., max-h-[64px] or max-h-[var(--site-header-height,64px)]), ensuring
the change is applied in header.tsx's className string and removing any stale
reliance on --header-height.
| <FiChevronDown | ||
| size={24} | ||
| className="color-text-primary relative top-[1px] ml-1 transition duration-300 group-data-[state=open]:rotate-180" | ||
| /> |
There was a problem hiding this comment.
Invalid Tailwind class color-text-primary.
Lines 204 and 222 use color-text-primary which is not a valid Tailwind class. This should be text-text-primary to match the pattern used elsewhere in the codebase.
🔧 Proposed fix
<FiChevronDown
size={24}
- className="color-text-primary relative top-[1px] ml-1 transition duration-300 group-data-[state=open]:rotate-180"
+ className="text-text-primary relative top-[1px] ml-1 transition duration-300 group-data-[state=open]:rotate-180"
/>- <Icon size={24} className="color-text-primary" />
+ <Icon size={24} className="text-text-primary" />Also applies to: 222-222
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/header.tsx` around lines 202 - 205, The Tailwind class used on
the icon's className is incorrect: replace any occurrence of
"color-text-primary" with the correct Tailwind utility "text-text-primary"
(e.g., on the FiChevronDown JSX element and the other identical class occurrence
in this Header component) so the styling matches the project's pattern; update
the className strings in the Header component where "color-text-primary" appears
to "text-text-primary".
Closes #9