feat: DR-7392 Add navigation menu for website/blog#7533
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReplaces the blog layout to use a new WebNavigation component; adds a full navigation system, FontAwesome loader, star-count and scroll-threshold hooks, UI navigation components, font/token updates to Monaspace Neon Var, action/button variant adjustments, global CSS tweaks, and a workspace dependency in packages/ui/package.json. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Contributor
🍈 Lychee Link Check Report3664 links: ❌ Errors
Full Statistics Table
|
Add hooks for GH stargazer and scroll threshold Update button Update blog layout with navbar
Update blog layout for mobile
carlagn
force-pushed
the
feat/DR-7392-website-nav
branch
from
337f5f4 to
9d18100
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
packages/ui/src/components/navigation-menu.tsx (3)
120-122:⚠️ Potential issue | 🟡 MinorKeep
data-slotas a single slot identifier.Line 121 mixes a Tailwind class into
data-slot. Movetext-foreground-neutralintoclassName.Suggested fix
- data-slot="navigation-menu-trigger text-foreground-neutral" - className={cn(navigationMenuTriggerStyle(), "group", className)} + data-slot="navigation-menu-trigger" + className={cn( + navigationMenuTriggerStyle(), + "group text-foreground-neutral", + className, + )}#!/bin/bash # Find data-slot attributes that contain whitespace (likely mixed concerns). rg -nP 'data-slot="[^"]*\s+[^"]*"' packages/ui/src/components/navigation-menu.tsx # Expected: Line 121 match should disappear after fix.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/navigation-menu.tsx` around lines 120 - 122, The data-slot attribute on NavigationMenuPrimitive.Trigger currently contains two tokens ("navigation-menu-trigger text-foreground-neutral"); keep data-slot as a single semantic slot by removing the Tailwind class from it and instead add "text-foreground-neutral" to the className list. Update the JSX for NavigationMenuPrimitive.Trigger so data-slot="navigation-menu-trigger" and include "text-foreground-neutral" inside the cn(...) call alongside navigationMenuTriggerStyle(), "group", and className, referencing the NavigationMenuPrimitive.Trigger element, the data-slot attribute, the className prop, the cn helper, and navigationMenuTriggerStyle() to locate the change.
47-47:⚠️ Potential issue | 🟡 MinorFix invalid Tailwind responsive token at Line 47.
md-px-4!is not a valid responsive utility, so medium+ horizontal padding won’t apply.Suggested fix
- mobileOpen && "p-0 md:p-4! md-px-4!", + mobileOpen && "p-0 md:p-4! md:px-4!",#!/bin/bash # Verify the invalid token is still present in the file. rg -n "md-px-4!" packages/ui/src/components/navigation-menu.tsx # Expected: one hit at Line 47 (or no hits after fix).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/navigation-menu.tsx` at line 47, The Tailwind token "md-px-4!" in the class string is invalid so medium+ horizontal padding never applies; in navigation-menu.tsx (within the NavigationMenu component where the expression mobileOpen && "p-0 md:p-4! md-px-4!" appears) replace the invalid token "md-px-4!" with the correct responsive utility "md:px-4!" (or "md:px-4" if you don't want the important flag) so the medium breakpoint horizontal padding is applied.
239-265:⚠️ Potential issue | 🟠 MajorAdd accessible names for icon-only social links.
Lines 241/248/255/262 render icon-only links without accessible labels, so screen readers won’t get a reliable link name.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/navigation-menu.tsx` around lines 239 - 265, NavigationMenuLink instances that render icon-only social links (inside NavigationMenuItem) lack accessible names; update each social link (the Discord, Twitter/X, YouTube, LinkedIn NavigationMenuLink elements) to provide an accessible label by adding an aria-label or an offscreen/visually-hidden text node (e.g., "Discord", "Twitter", "YouTube", "LinkedIn") so screen readers can announce the link purpose while keeping the icon-only visual UI unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ui/src/components/navigation-menu.tsx`:
- Line 142: The Tailwind class string in the navigation menu component contains
malformed attribute-variant syntax and an incorrect easing utility; update any
occurrences of data-activation-direction=left and
data-activation-direction=right (and their starting/ending variants) to the
bracketed variant form data-[activation-direction=left] and
data-[activation-direction=right] (e.g., replace
data-ending-style:data-activation-direction=left:translate-x-[50%] with
data-ending-style:data-[activation-direction=left]:translate-x-[50%]) and
replace any easing-* utility (e.g., easing-[...]) with the correct ease-* form
(e.g., ease-[...]) inside the same class string used in the navigation menu
component (the long class on the container that includes
data-starting-style/data-ending-style and group-data-... variants).
---
Duplicate comments:
In `@packages/ui/src/components/navigation-menu.tsx`:
- Around line 120-122: The data-slot attribute on
NavigationMenuPrimitive.Trigger currently contains two tokens
("navigation-menu-trigger text-foreground-neutral"); keep data-slot as a single
semantic slot by removing the Tailwind class from it and instead add
"text-foreground-neutral" to the className list. Update the JSX for
NavigationMenuPrimitive.Trigger so data-slot="navigation-menu-trigger" and
include "text-foreground-neutral" inside the cn(...) call alongside
navigationMenuTriggerStyle(), "group", and className, referencing the
NavigationMenuPrimitive.Trigger element, the data-slot attribute, the className
prop, the cn helper, and navigationMenuTriggerStyle() to locate the change.
- Line 47: The Tailwind token "md-px-4!" in the class string is invalid so
medium+ horizontal padding never applies; in navigation-menu.tsx (within the
NavigationMenu component where the expression mobileOpen && "p-0 md:p-4!
md-px-4!" appears) replace the invalid token "md-px-4!" with the correct
responsive utility "md:px-4!" (or "md:px-4" if you don't want the important
flag) so the medium breakpoint horizontal padding is applied.
- Around line 239-265: NavigationMenuLink instances that render icon-only social
links (inside NavigationMenuItem) lack accessible names; update each social link
(the Discord, Twitter/X, YouTube, LinkedIn NavigationMenuLink elements) to
provide an accessible label by adding an aria-label or an
offscreen/visually-hidden text node (e.g., "Discord", "Twitter", "YouTube",
"LinkedIn") so screen readers can announce the link purpose while keeping the
icon-only visual UI unchanged.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
packages/ui/src/components/navigation-menu.tsx (4)
120-123:⚠️ Potential issue | 🟡 MinorKeep
data-slotas a slot identifier only (Line 121).
data-slotcurrently includes a Tailwind class token; that class should live inclassName.Suggested fix
- data-slot="navigation-menu-trigger text-foreground-neutral" - className={cn(navigationMenuTriggerStyle(), "group", className)} + data-slot="navigation-menu-trigger" + className={cn( + navigationMenuTriggerStyle(), + "group text-foreground-neutral", + className, + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/navigation-menu.tsx` around lines 120 - 123, The data-slot attribute on NavigationMenuPrimitive.Trigger currently contains a Tailwind class token; remove that token so data-slot is only the slot identifier ("navigation-menu-trigger") and move the Tailwind token into the className prop (which already combines navigationMenuTriggerStyle(), "group", and className). Update the JSX for NavigationMenuPrimitive.Trigger to set data-slot="navigation-menu-trigger" and ensure the className includes the Tailwind token that was removed.
142-142:⚠️ Potential issue | 🟡 MinorCorrect malformed Tailwind variant syntax in motion classes.
At Line 142,
data-activation-direction=...is missing bracketed attribute-variant syntax.
At Line 171,easing-[ease]should beease-[ease].Suggested fix
- data-ending-style:data-activation-direction=left:translate-x-[50%] data-ending-style:data-activation-direction=right:translate-x-[-50%] data-starting-style:data-activation-direction=left:translate-x-[-50%] data-starting-style:data-activation-direction=right:translate-x-[50%] + data-ending-style:data-[activation-direction=left]:translate-x-[50%] data-ending-style:data-[activation-direction=right]:translate-x-[-50%] data-starting-style:data-[activation-direction=left]:translate-x-[-50%] data-starting-style:data-[activation-direction=right]:translate-x-[50%]- data-[ending-style]:easing-[ease] + data-[ending-style]:ease-[ease]Also applies to: 171-171
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/navigation-menu.tsx` at line 142, In the navigation-menu class string in the NavigationMenu component, fix malformed Tailwind attribute-variant syntax by wrapping the attribute selector in brackets before the utility: change patterns like data-ending-style:data-activation-direction=left:translate-x-[50%] and data-starting-style:data-activation-direction=right:translate-x-[50%] to use bracketed selectors (e.g. data-ending-style:[data-activation-direction=left]:translate-x-[50%] and data-starting-style:[data-activation-direction=right]:translate-x-[50%]); also replace the incorrect easing utility easing-[ease] with the proper ease-[ease] utility where it appears.
241-243:⚠️ Potential issue | 🟠 MajorAdd accessible names for icon-only social links.
Line 241/248/255/262 links render icons only, so they currently have no usable accessible name.
Suggested fix
-<NavigationMenuLink className="p-0" href="https://pris.ly/discord"> - <i className="fa-brands fa-discord"></i> +<NavigationMenuLink className="p-0" href="https://pris.ly/discord" aria-label="Discord"> + <i className="fa-brands fa-discord" aria-hidden="true"></i> </NavigationMenuLink> -<NavigationMenuLink className="p-0" href="https://pris.ly/x"> - <i className="fa-brands fa-x-twitter"></i> +<NavigationMenuLink className="p-0" href="https://pris.ly/x" aria-label="X"> + <i className="fa-brands fa-x-twitter" aria-hidden="true"></i> </NavigationMenuLink> -<NavigationMenuLink className="p-0" href="https://pris.ly/youtube"> - <i className="fa-brands fa-youtube"></i> +<NavigationMenuLink className="p-0" href="https://pris.ly/youtube" aria-label="YouTube"> + <i className="fa-brands fa-youtube" aria-hidden="true"></i> </NavigationMenuLink> -<NavigationMenuLink className="p-0" href="https://pris.ly/linkedin"> - <i className="fa-brands fa-square-linkedin"></i> +<NavigationMenuLink className="p-0" href="https://pris.ly/linkedin" aria-label="LinkedIn"> + <i className="fa-brands fa-square-linkedin" aria-hidden="true"></i> </NavigationMenuLink>Also applies to: 248-250, 255-257, 262-264
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/navigation-menu.tsx` around lines 241 - 243, NavigationMenuLink instances that render icon-only social links (the NavigationMenuLink elements showing <i className="fa-...">) lack accessible names; add an accessible name to each by either adding an aria-label (e.g., aria-label="Discord", "GitHub", "Twitter", "YouTube" matching the icon) on the NavigationMenuLink or by including visually hidden text (a <span className="sr-only">...</span>) inside the link; update each NavigationMenuLink usage (the icon-only links in navigation-menu.tsx) to include the appropriate label so screen readers can identify the social destination.
47-47:⚠️ Potential issue | 🟡 MinorFix invalid Tailwind responsive token at Line 47.
md-px-4!is not a valid responsive utility, so the medium+ horizontal padding won’t apply.Suggested fix
- mobileOpen && "p-0 md:p-4! md-px-4!", + mobileOpen && "p-0 md:p-4! md:px-4!",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/navigation-menu.tsx` at line 47, The class string in navigation-menu.tsx that assembles mobileOpen styles contains an invalid Tailwind token ("md-px-4!"); update the conditional that uses mobileOpen (the expression producing "p-0 md:p-4! md-px-4!") to replace "md-px-4!" with the correct responsive utility "md:px-4" (and remove the trailing "!") so medium+ horizontal padding applies; if you need the rule to be important, use the proper Tailwind form like "md:!px-4" instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ui/src/components/navigation-menu.tsx`:
- Around line 311-314: The submenu items rendered inside NavigationMenuList
currently render MenuNavigationItem directly, breaking Radix's required
structure; wrap each MenuNavigationItem inside a Radix NavigationMenuItem so
NavigationMenuList's direct children are NavigationMenuItem elements (e.g., map
link.sub to return <NavigationMenuItem key={sublink.url}><MenuNavigationItem
link={sublink} /></NavigationMenuItem>), ensuring you keep the existing props
and keys on the inner MenuNavigationItem and preserve focus/keyboard behavior.
- Line 9: The file currently imports only useState from React but uses the type
React.ComponentPropsWithRef in multiple component signatures; add an explicit
import for ComponentPropsWithRef from "react" so TypeScript (especially under
stricter configs/React 19) can resolve those types reliably. Update the import
statement that currently reads `import { useState } from "react";` to also
import `ComponentPropsWithRef`, ensuring all component signatures that reference
ComponentPropsWithRef (the four uses in this file) remain type-safe.
---
Duplicate comments:
In `@packages/ui/src/components/navigation-menu.tsx`:
- Around line 120-123: The data-slot attribute on
NavigationMenuPrimitive.Trigger currently contains a Tailwind class token;
remove that token so data-slot is only the slot identifier
("navigation-menu-trigger") and move the Tailwind token into the className prop
(which already combines navigationMenuTriggerStyle(), "group", and className).
Update the JSX for NavigationMenuPrimitive.Trigger to set
data-slot="navigation-menu-trigger" and ensure the className includes the
Tailwind token that was removed.
- Line 142: In the navigation-menu class string in the NavigationMenu component,
fix malformed Tailwind attribute-variant syntax by wrapping the attribute
selector in brackets before the utility: change patterns like
data-ending-style:data-activation-direction=left:translate-x-[50%] and
data-starting-style:data-activation-direction=right:translate-x-[50%] to use
bracketed selectors (e.g.
data-ending-style:[data-activation-direction=left]:translate-x-[50%] and
data-starting-style:[data-activation-direction=right]:translate-x-[50%]); also
replace the incorrect easing utility easing-[ease] with the proper ease-[ease]
utility where it appears.
- Around line 241-243: NavigationMenuLink instances that render icon-only social
links (the NavigationMenuLink elements showing <i className="fa-...">) lack
accessible names; add an accessible name to each by either adding an aria-label
(e.g., aria-label="Discord", "GitHub", "Twitter", "YouTube" matching the icon)
on the NavigationMenuLink or by including visually hidden text (a <span
className="sr-only">...</span>) inside the link; update each NavigationMenuLink
usage (the icon-only links in navigation-menu.tsx) to include the appropriate
label so screen readers can identify the social destination.
- Line 47: The class string in navigation-menu.tsx that assembles mobileOpen
styles contains an invalid Tailwind token ("md-px-4!"); update the conditional
that uses mobileOpen (the expression producing "p-0 md:p-4! md-px-4!") to
replace "md-px-4!" with the correct responsive utility "md:px-4" (and remove the
trailing "!") so medium+ horizontal padding applies; if you need the rule to be
important, use the proper Tailwind form like "md:!px-4" instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Style