navigation menu [poc] #1246

GoldGroove06 · 2025-07-22T18:37:49Z

Screen.Recording.2025-07-22.230051.mp4

Summary by CodeRabbit

New Features
- Added a Navigation Menu suite (Root, Item, Trigger, Content, Link) with context-driven open/close state and keyboard focus management.
- Includes a Storybook example showcasing multi-level menu usage and behavior.
Style
- New SCSS for Navigation Menu: layout, dropdowns, triggers, links, animations and theme import for consistent styling.

coderabbitai · 2025-07-22T18:37:55Z

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

MCP integration is disabled by default for public repositories
Jira integration is disabled by default for public repositories
Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6fc3736 and 2470d0a.

📒 Files selected for processing (1)

styles/themes/default.scss (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

styles/themes/default.scss

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: build

✨ Finishing Touches

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch feat/navigation-menu

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.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

Visit our Status Page to check the current availability of CodeRabbit.
Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 9

🧹 Nitpick comments (8)

src/components/ui/NavigationMenu/fragments/NavigationMenuTrigger.tsx (2)
11-15: Consider adding accessibility attributes for better screen reader support.

The button should include ARIA attributes to indicate its role in the navigation menu and its expanded state. Based on the learning about verifying existing ARIA attributes, let me check if these are needed.
-            <button onClick={handleTrigger} className={clsx(`${rootClass}-trigger`, className)}>
+            <button 
+                onClick={handleTrigger} 
+                className={clsx(`${rootClass}-trigger`, className)}
+                aria-haspopup="true"
+                aria-expanded={itemOpen}
+            >
Note: You'll need to access itemOpen from NavigationMenuItemContext for the aria-expanded attribute:
-    const {handleTrigger} = React.useContext(NavigationMenuItemContext);
+    const {handleTrigger, itemOpen} = React.useContext(NavigationMenuItemContext);
7-17: Consider using React.forwardRef for better composability.

Adding forwardRef would allow parent components to access the button element directly if needed, which is a common pattern in UI libraries.
+export interface NavigationMenuTriggerProps {
+  children: React.ReactNode;
+  className?: string;
+}

-const NavigationMenuTrigger = ({children, className}:any) => {
+const NavigationMenuTrigger = React.forwardRef<HTMLButtonElement, NavigationMenuTriggerProps>(
+  ({children, className}, ref) => {
     const {handleTrigger} = React.useContext(NavigationMenuItemContext);
     const {rootClass} = React.useContext(NavigationMenuRootContext);
     return (
         <RovingFocusGroup.Item>
-            <button onClick={handleTrigger} className={clsx(`${rootClass}-trigger`, className)}>
+            <button ref={ref} onClick={handleTrigger} className={clsx(`${rootClass}-trigger`, className)}>
                 {children}
             </button>
         </RovingFocusGroup.Item>
     )
-}
+});

+NavigationMenuTrigger.displayName = 'NavigationMenuTrigger';
src/components/ui/NavigationMenu/fragments/NavigationMenuRoot.tsx (2)
18-18: Remove unused destructured props parameter.

The ...props parameter is destructured but never used in the component.
-const NavigationMenuRoot = ({children, value, defaultValue = '', onValueChange, customRootClass, className, ...props}:NavigationMenuRootProps) => {
+const NavigationMenuRoot = ({children, value, defaultValue = '', onValueChange, customRootClass, className}:NavigationMenuRootProps) => {
26-36: Consider removing the unnecessary outer div wrapper.

The outer div doesn't seem to serve any functional purpose and could be removed to simplify the component structure.
-    return (
-        <div >
-            <NavigationMenuRootContext.Provider value={{isOpen, setIsOpen, rootClass}} >
-            <RovingFocusGroup.Root>
-                <RovingFocusGroup.Group className={clsx(`${rootClass}-root`, className)}>
-                    {children}
-                </RovingFocusGroup.Group>
-            </RovingFocusGroup.Root>
-            </NavigationMenuRootContext.Provider>
-        </div>
-    )
+    return (
+        <NavigationMenuRootContext.Provider value={{isOpen, setIsOpen, rootClass}} >
+            <RovingFocusGroup.Root>
+                <RovingFocusGroup.Group className={clsx(`${rootClass}-root`, className)}>
+                    {children}
+                </RovingFocusGroup.Group>
+            </RovingFocusGroup.Root>
+        </NavigationMenuRootContext.Provider>
+    )
src/components/ui/NavigationMenu/stories/NavigationMenu.stories.tsx (1)
10-33: Consider adding more story variants for enhanced testing.

While the current story is good, consider adding additional stories to cover different states and configurations for better testing coverage.

Additional stories you might consider:
export const WithActiveStates = () => (
  <SandboxEditor>
    <NavigationMenu.Root defaultValue="item1">
      {/* existing menu structure */}
    </NavigationMenu.Root>
  </SandboxEditor>
);

export const Controlled = () => {
  const [value, setValue] = React.useState('');
  return (
    <SandboxEditor>
      <NavigationMenu.Root value={value} onValueChange={setValue}>
        {/* existing menu structure */}
      </NavigationMenu.Root>
    </SandboxEditor>
  );
};
styles/themes/components/navigation-menu.scss (3)
45-62: Provide a matching fade-out animation to avoid abrupt close

display: none is immediately applied on close, so the menu snaps out without the smoothness you added for opening. Consider a data-state="closed" selector (matching Radix primitives) that runs a reverse animation and only sets display:none after the animation ends (with JS or animation-fill-mode: forwards).

[performance] [UX]

45-62: Re-evaluate z-index: 100 for global stacking context collisions

Hard-coding z-index: 100 may still be lower than other pop-ups/tooltips already in the library and can create layering bugs. Either:

Use an existing design-system token (e.g. var(--rad-ui-z-layer-popover)) or

Centralise z-index values in a map and import here.

64-67: Prefix the keyframes to prevent name collisions

The project already uses a rad-ui- namespace for classes; doing the same for keyframes keeps the global timeline clean and avoids collisions with third-party CSS.
-@keyframes rad-ui-navigation-menu-fade-in {
+@keyframes rad-ui-navigation-menu--fade-in {
Remember to update the reference above.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5f79c92 and 6fc3736.

📒 Files selected for processing (11)

src/components/ui/NavigationMenu/NavigationMenu.tsx (1 hunks)
src/components/ui/NavigationMenu/contexts/NavigationMenuRootContext.tsx (1 hunks)
src/components/ui/NavigationMenu/contexts/NavigationMenyItemContext.tsx (1 hunks)
src/components/ui/NavigationMenu/fragments/NavigationMenuContent.tsx (1 hunks)
src/components/ui/NavigationMenu/fragments/NavigationMenuItem.tsx (1 hunks)
src/components/ui/NavigationMenu/fragments/NavigationMenuLink.tsx (1 hunks)
src/components/ui/NavigationMenu/fragments/NavigationMenuRoot.tsx (1 hunks)
src/components/ui/NavigationMenu/fragments/NavigationMenuTrigger.tsx (1 hunks)
src/components/ui/NavigationMenu/stories/NavigationMenu.stories.tsx (1 hunks)
styles/themes/components/navigation-menu.scss (1 hunks)
styles/themes/default.scss (1 hunks)

🧠 Learnings (7)

📓 Common learnings

Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.

src/components/ui/NavigationMenu/fragments/NavigationMenuRoot.tsx (3)

Learnt from: decipher-cs
PR: #417
File: src/components/ui/Dropdown/Dropdown.tsx:0-0
Timestamp: 2024-12-12T08:22:59.375Z
Learning: The Dropdown.Trigger component is customizable and needs to be used with Dropdown.Root.

Learnt from: kotAPI
PR: #1031
File: src/components/ui/Accordion/fragments/AccordionRoot.tsx:41-44
Timestamp: 2025-04-07T04:38:34.864Z
Learning: The Accordion component in rad-ui/ui supports both controlled and uncontrolled modes through props like value, defaultValue, and onValueChange. When implementing controlled components, remember to: 1) Initialize state from defaultValue, 2) Update internal state when value changes (controlled mode), 3) Call onValueChange callback, and 4) Prevent internal state updates when in controlled mode.

Learnt from: kotAPI
PR: #576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the TogglePrimitive component (src/core/primitives/Toggle/index.tsx), when the component becomes controlled, it's acceptable to not sync the internal isPressed state with the external pressed prop.

src/components/ui/NavigationMenu/contexts/NavigationMenyItemContext.tsx (1)

Learnt from: decipher-cs
PR: #417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the Dropdown.Trigger component in src/components/ui/Dropdown/Dropdown.stories.tsx.

src/components/ui/NavigationMenu/fragments/NavigationMenuTrigger.tsx (3)

Learnt from: decipher-cs
PR: #417
File: src/components/ui/Dropdown/Dropdown.tsx:0-0
Timestamp: 2024-12-12T08:22:59.375Z
Learning: The Dropdown.Trigger component is customizable and needs to be used with Dropdown.Root.

Learnt from: decipher-cs
PR: #417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the Dropdown.Trigger component in src/components/ui/Dropdown/Dropdown.stories.tsx.

Learnt from: GoldGroove06
PR: #1215
File: src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveTrigger.tsx:25-27
Timestamp: 2025-07-18T16:43:26.143Z
Learning: In the CheckboxGroupPrimitiveTrigger component (src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveTrigger.tsx), the component uses two separate useEffect hooks with different purposes: the first useEffect (lines 25-27) with empty dependency array [] is meant to set the initial state only once on mount by syncing with the group's checked values, while the second useEffect (lines 28-34) handles ongoing state updates by propagating local isChecked changes back to the group's checkedValues. This pattern prevents infinite loops while maintaining proper synchronization.

src/components/ui/NavigationMenu/stories/NavigationMenu.stories.tsx (2)

Learnt from: decipher-cs
PR: #417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the Dropdown.Trigger component in src/components/ui/Dropdown/Dropdown.stories.tsx.

Learnt from: kotAPI
PR: #484
File: src/components/ui/Kbd/stories/Kbd.stories.js:6-6
Timestamp: 2024-10-27T07:20:10.457Z
Learning: In the Storybook files, variations in the title prefixes like 'Components/', 'WIP/', 'Tools/', and 'Examples/' are intentional. These naming patterns are acceptable and should not be flagged as inconsistencies in future code reviews.

src/components/ui/NavigationMenu/fragments/NavigationMenuLink.tsx (1)

Learnt from: decipher-cs
PR: #417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the Dropdown.Trigger component in src/components/ui/Dropdown/Dropdown.stories.tsx.

src/components/ui/NavigationMenu/fragments/NavigationMenuItem.tsx (1)

Learnt from: decipher-cs
PR: #417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the Dropdown.Trigger component in src/components/ui/Dropdown/Dropdown.stories.tsx.

🧬 Code Graph Analysis (1)

src/components/ui/NavigationMenu/fragments/NavigationMenuRoot.tsx (1)

src/core/hooks/useControllableState/index.tsx (1)

useControllableState (15-78)

🪛 GitHub Check: lint

src/components/ui/NavigationMenu/fragments/NavigationMenuItem.tsx

[warning] 20-20:
Avoid non-native interactive elements. If using native HTML is not possible, add an appropriate role and support for tabbing, mouse, keyboard, and touch inputs to an interactive content element

🧰 Additional context used

🧠 Learnings (7)

📓 Common learnings

Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.