feat: navigation menu #175
Conversation
|
@BIT-zhaoyang is attempting to deploy a commit to the OrcDev Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughA new 8-bit styled navigation menu component system is introduced, including its UI implementation, documentation, and registry metadata. The changes add React components for the navigation menu, extend them with retro font styling, update navigation and registry configurations, and provide documentation and demo pages. A dependency on Radix UI's navigation menu is added. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NavigationMenuDemo
participant 8bitNavigationMenu
participant DocsPage
User->>DocsPage: Visit Navigation Menu docs
DocsPage->>NavigationMenuDemo: Render demo component
NavigationMenuDemo->>8bitNavigationMenu: Render 8-bit styled menu
User->>8bitNavigationMenu: Interact (open item, select link)
8bitNavigationMenu-->>User: Show menu content, navigate
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
app/docs/components/navigation-menu/page.tsxOops! Something went wrong! :( ESLint: 9.26.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react-hooks" was referenced from the config file in " » eslint-config-next/core-web-vitals » /node_modules/.pnpm/eslint-config-next@15.3.2_eslint@9.26.0_jiti@2.4.2__typescript@5.8.3/node_modules/eslint-config-next/index.js". If you still can't figure out the problem, please see https://eslint.org/docs/latest/use/troubleshooting. app/docs/components/navigation-menu/demo.tsxOops! Something went wrong! :( ESLint: 9.26.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react-hooks" was referenced from the config file in " » eslint-config-next/core-web-vitals » /node_modules/.pnpm/eslint-config-next@15.3.2_eslint@9.26.0_jiti@2.4.2__typescript@5.8.3/node_modules/eslint-config-next/index.js". If you still can't figure out the problem, please see https://eslint.org/docs/latest/use/troubleshooting. components/ui/8bit/navigation-menu.tsxOops! Something went wrong! :( ESLint: 9.26.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react-hooks" was referenced from the config file in " » eslint-config-next/core-web-vitals » /node_modules/.pnpm/eslint-config-next@15.3.2_eslint@9.26.0_jiti@2.4.2__typescript@5.8.3/node_modules/eslint-config-next/index.js". If you still can't figure out the problem, please see https://eslint.org/docs/latest/use/troubleshooting.
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (10)
config/nav-items.ts (1)
105-108: Navigation menu item added in correct positionThe new navigation menu entry is well-placed in the component list. Consider adding the
new: trueproperty to highlight it as a newly added component, similar to other recent component additions.{ title: "Navigation Menu", url: "/docs/components/navigation-menu", + new: true, }app/docs/components/navigation-menu/page.tsx (2)
15-17: Verify openGraph metadata for navigation menuThe component is using
labelMetaDatafor OpenGraph images. Consider creating and using navigation menu specific metadata images instead.openGraph: { - images: labelMetaData, + images: navigationMenuMetaData, },
31-33: Update component descriptionThe component description is generic and matches the metadata. Consider enhancing it to better describe the navigation menu's features and use cases.
<p className="text-muted-foreground"> - Displays an 8-bit navigation menu component. + An accessible navigation menu component with dropdown functionality, styled with 8-bit aesthetics. Useful for site navigation, multi-level menus, and application hierarchies. </p>app/docs/components/navigation-menu/demo.tsx (1)
66-67: Consider adding hover state to linkThe link in the navigation menu lacks explicit hover styling. Consider adding hover styles to enhance interactivity.
<Link - className="from-muted/50 to-muted flex h-full w-full flex-col justify-end rounded-md bg-linear-to-b p-6 no-underline outline-hidden select-none focus:shadow-md" + className="from-muted/50 to-muted flex h-full w-full flex-col justify-end rounded-md bg-linear-to-b p-6 no-underline outline-hidden select-none focus:shadow-md hover:bg-muted/80 transition-colors" href="/" >components/ui/8bit/navigation-menu.tsx (4)
38-42: Interface naming inconsistencyThe interface is named
BitNavigationMenuPropswhile the component is namedNavigationMenu. Consider renaming toNavigationMenuPropsfor consistency or ensuring this interface is used across all components.-export interface BitNavigationMenuProps +export interface NavigationMenuProps extends React.ComponentProps<typeof ShadcnNavigationMenu>, VariantProps<typeof navigationMenuVariants> { asChild?: boolean }
77-84: Explain the purpose of the "static" classThe NavigationMenuItem component adds a "static" class that isn't added to other components. Consider adding a comment explaining why this specific class is needed for this component.
function NavigationMenuItem({ className, font, ...props }: React.ComponentProps<typeof ShadcnNavigationMenuItem> & FontVariantProps) { return ( <ShadcnNavigationMenuItem className={cn( - "static", + "static", // Ensures proper positioning within the navigation menu font !== "normal" && pressStart.className, className )} {...props} /> ) }
102-117: Inconsistent props destructuring patternNavigationMenuContent and NavigationMenuViewport explicitly destructure
childrenwhile other components don't. For consistency, either:
- Explicitly destructure
childrenin all components, or- Remove explicit destructuring and let it be included in
...propsfunction NavigationMenuContent({ className, font, - children, ...props }: React.ComponentProps<typeof ShadcnNavigationMenuContent> & FontVariantProps) { return ( <ShadcnNavigationMenuContent className={cn(font !== "normal" && pressStart.className, className)} {...props} > - {children} + {props.children} </ShadcnNavigationMenuContent> ) }Or, for consistency across all components, explicitly destructure children everywhere they're used.
Also applies to: 119-131
44-55: Consider using a higher-order component pattern for DRY codeAll eight component implementations follow the same pattern of wrapping Shadcn components with font styling. Consider creating a higher-order component (HOC) to reduce code duplication.
// Example implementation of a higher-order component function with8BitFont<T extends { className?: string }>( Component: React.ComponentType<T>, additionalClasses: string = "" ) { return function Enhanced({ className, font, ...props }: T & FontVariantProps) { return ( <Component className={cn( additionalClasses, font !== "normal" && pressStart.className, className )} {...(props as any)} /> ); }; } // Then use it like: const NavigationMenu = with8BitFont(ShadcnNavigationMenu); const NavigationMenuItem = with8BitFont(ShadcnNavigationMenuItem, "static"); // etc.This approach would significantly reduce code duplication while maintaining the same functionality.
Also applies to: 57-69, 71-86, 88-100, 102-117, 119-131, 133-144, 146-158
components/ui/navigation-menu.tsx (2)
92-95: Complex CSS selector in NavigationMenuContentThe CSS class in NavigationMenuContent contains very complex selectors and a long string of utility classes that's difficult to parse. Consider breaking this into multiple variables for better readability and maintenance.
// Example refactoring const baseContentStyles = "data-[motion^=from-]:animate-in data-[motion^=to-]:animate-out data-[motion^=from-]:fade-in data-[motion^=to-]:fade-out data-[motion=from-end]:slide-in-from-right-52 data-[motion=from-start]:slide-in-from-left-52 data-[motion=to-end]:slide-out-to-right-52 data-[motion=to-start]:slide-out-to-left-52 top-0 left-0 w-full p-2 pr-2.5 md:absolute md:w-auto" const noViewportStyles = "group-data-[viewport=false]/navigation-menu:bg-popover group-data-[viewport=false]/navigation-menu:text-popover-foreground group-data-[viewport=false]/navigation-menu:data-[state=open]:animate-in group-data-[viewport=false]/navigation-menu:data-[state=closed]:animate-out group-data-[viewport=false]/navigation-menu:data-[state=closed]:zoom-out-95 group-data-[viewport=false]/navigation-menu:data-[state=open]:zoom-in-95 group-data-[viewport=false]/navigation-menu:data-[state=open]:fade-in-0 group-data-[viewport=false]/navigation-menu:data-[state=closed]:fade-out-0 group-data-[viewport=false]/navigation-menu:top-full group-data-[viewport=false]/navigation-menu:mt-1.5 group-data-[viewport=false]/navigation-menu:overflow-hidden group-data-[viewport=false]/navigation-menu:rounded-md group-data-[viewport=false]/navigation-menu:border group-data-[viewport=false]/navigation-menu:shadow group-data-[viewport=false]/navigation-menu:duration-200 **:data-[slot=navigation-menu-link]:focus:ring-0 **:data-[slot=navigation-menu-link]:focus:outline-none" // Then in component: className={cn(baseContentStyles, noViewportStyles, className)}
1-168: Add unit tests for the navigation menu componentsThe implementation is good, but consider adding unit tests to ensure the components render correctly and handle edge cases appropriately.
Would you like me to help create some unit tests for these components to ensure they function correctly across different scenarios?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/docs/components/navigation-menu/demo.tsx(1 hunks)app/docs/components/navigation-menu/page.tsx(1 hunks)components/ui/8bit/navigation-menu.tsx(1 hunks)components/ui/navigation-menu.tsx(1 hunks)config/nav-items.ts(1 hunks)package.json(1 hunks)public/r/8bit-navigation-menu.json(1 hunks)registry.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
app/docs/components/navigation-menu/demo.tsx (2)
components/ui/navigation-menu.tsx (8)
NavigationMenu(159-159)NavigationMenuList(160-160)NavigationMenuItem(161-161)NavigationMenuTrigger(163-163)NavigationMenuContent(162-162)NavigationMenuLink(164-164)navigationMenuTriggerStyle(167-167)NavigationMenuIndicator(165-165)components/ui/8bit/navigation-menu.tsx (8)
NavigationMenu(161-161)NavigationMenuList(166-166)NavigationMenuItem(164-164)NavigationMenuTrigger(167-167)NavigationMenuContent(162-162)NavigationMenuLink(165-165)navigationMenuTriggerStyle(17-17)NavigationMenuIndicator(163-163)
components/ui/navigation-menu.tsx (2)
components/ui/8bit/navigation-menu.tsx (2)
NavigationMenu(161-161)NavigationMenuViewport(168-168)lib/utils.ts (1)
cn(4-6)
🔇 Additional comments (8)
package.json (1)
22-22: Adding Radix UI Navigation Menu dependency looks goodThis addition is necessary for the new navigation menu component. The version with caret (
^1.2.12) allows for compatible updates.app/docs/components/navigation-menu/demo.tsx (1)
120-138: ListItem component implementation is solidGood job on creating a reusable ListItem component that handles the common pattern of rendering navigation menu items with titles and descriptions. This promotes consistent styling and behavior throughout the menu.
public/r/8bit-navigation-menu.json (1)
12-15: LGTM! Component path and configuration looks goodThe file reference is correctly configured, specifying both the path and target for the navigation menu component.
components/ui/8bit/navigation-menu.tsx (2)
2-3: LGTM! Font configuration for 8-bit stylingGood use of Next.js font optimization with the "Press Start 2P" Google font to achieve the 8-bit retro aesthetic.
24-34: LGTM! Well-structured variants with sensible defaultsThe navigation menu variants are well-defined using class-variance-authority, with "retro" set as the default font style for the 8-bit theme.
components/ui/navigation-menu.tsx (3)
8-30: LGTM! Good viewport handling in NavigationMenuThe root NavigationMenu component elegantly handles the conditional rendering of the viewport with a boolean prop, which provides flexibility for different use cases.
61-63: LGTM! Comprehensive trigger stylingWell-defined trigger styles with proper states for hover, focus, and active states, enhancing the user experience.
76-80: Good accessibility practice with aria-hiddenExcellent use of
aria-hidden="true"for the decorative ChevronDownIcon, ensuring screen readers don't announce it unnecessarily.
| const components: { title: string; href: string; description: string }[] = [ | ||
| { | ||
| title: "Alert Dialog", | ||
| href: "/docs/primitives/alert-dialog", | ||
| description: | ||
| "A modal dialog that interrupts the user with important content and expects a response.", | ||
| }, | ||
| { | ||
| title: "Hover Card", | ||
| href: "/docs/primitives/hover-card", | ||
| description: | ||
| "For sighted users to preview content available behind a link.", | ||
| }, | ||
| { | ||
| title: "Progress", | ||
| href: "/docs/primitives/progress", | ||
| description: | ||
| "Displays an indicator showing the completion progress of a task, typically displayed as a progress bar.", | ||
| }, | ||
| { | ||
| title: "Scroll-area", | ||
| href: "/docs/primitives/scroll-area", | ||
| description: "Visually or semantically separates content.", | ||
| }, | ||
| { | ||
| title: "Tabs", | ||
| href: "/docs/primitives/tabs", | ||
| description: | ||
| "A set of layered sections of content—known as tab panels—that are displayed one at a time.", | ||
| }, | ||
| { | ||
| title: "Tooltip", | ||
| href: "/docs/primitives/tooltip", | ||
| description: | ||
| "A popup that displays information related to an element when the element receives keyboard focus or the mouse hovers over it.", | ||
| }, | ||
| ] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Components array uses placeholder URLs
The components array contains URLs pointing to "/docs/primitives/..." paths, which appear to be placeholders. Consider updating these to point to actual documentation pages in your project.
const components: { title: string; href: string; description: string }[] = [
{
title: "Alert Dialog",
- href: "/docs/primitives/alert-dialog",
+ href: "/docs/components/alert-dialog",
description:
"A modal dialog that interrupts the user with important content and expects a response.",
},
// Update other URLs similarly...📝 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.
| const components: { title: string; href: string; description: string }[] = [ | |
| { | |
| title: "Alert Dialog", | |
| href: "/docs/primitives/alert-dialog", | |
| description: | |
| "A modal dialog that interrupts the user with important content and expects a response.", | |
| }, | |
| { | |
| title: "Hover Card", | |
| href: "/docs/primitives/hover-card", | |
| description: | |
| "For sighted users to preview content available behind a link.", | |
| }, | |
| { | |
| title: "Progress", | |
| href: "/docs/primitives/progress", | |
| description: | |
| "Displays an indicator showing the completion progress of a task, typically displayed as a progress bar.", | |
| }, | |
| { | |
| title: "Scroll-area", | |
| href: "/docs/primitives/scroll-area", | |
| description: "Visually or semantically separates content.", | |
| }, | |
| { | |
| title: "Tabs", | |
| href: "/docs/primitives/tabs", | |
| description: | |
| "A set of layered sections of content—known as tab panels—that are displayed one at a time.", | |
| }, | |
| { | |
| title: "Tooltip", | |
| href: "/docs/primitives/tooltip", | |
| description: | |
| "A popup that displays information related to an element when the element receives keyboard focus or the mouse hovers over it.", | |
| }, | |
| ] | |
| const components: { title: string; href: string; description: string }[] = [ | |
| { | |
| title: "Alert Dialog", | |
| href: "/docs/components/alert-dialog", | |
| description: | |
| "A modal dialog that interrupts the user with important content and expects a response.", | |
| }, | |
| { | |
| title: "Hover Card", | |
| href: "/docs/primitives/hover-card", | |
| description: | |
| "For sighted users to preview content available behind a link.", | |
| }, | |
| { | |
| title: "Progress", | |
| href: "/docs/primitives/progress", | |
| description: | |
| "Displays an indicator showing the completion progress of a task, typically displayed as a progress bar.", | |
| }, | |
| { | |
| title: "Scroll-area", | |
| href: "/docs/primitives/scroll-area", | |
| description: "Visually or semantically separates content.", | |
| }, | |
| { | |
| title: "Tabs", | |
| href: "/docs/primitives/tabs", | |
| description: | |
| "A set of layered sections of content—known as tab panels—that are displayed one at a time.", | |
| }, | |
| { | |
| title: "Tooltip", | |
| href: "/docs/primitives/tooltip", | |
| description: | |
| "A popup that displays information related to an element when the element receives keyboard focus or the mouse hovers over it.", | |
| }, | |
| ] |
|
hey @BIT-zhaoyang I wrote you on Discord you probably missed it. UI is not looking good on my side: 
|
|
Closed in favor of #203 |
2 participants
added navigation menu component
Summary by CodeRabbit
New Features
Chores