-
-
Notifications
You must be signed in to change notification settings - Fork 53
Feat/dropdown menu #1274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/dropdown menu #1274
Conversation
WalkthroughThis update introduces a new DropdownMenu UI component suite, including context, fragments, and Storybook stories. It also adds a MenuPrimitive system with context, fragments, and stories, and expands the Floater primitive's exports. Accompanying SCSS styles for dropdown menus are added and imported into the default theme. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DropdownMenu.Root
participant DropdownMenu.Trigger
participant DropdownMenu.Portal
participant DropdownMenu.Content
participant DropdownMenu.Item
participant DropdownMenu.Sub
participant DropdownMenu.SubTrigger
User->>DropdownMenu.Trigger: Clicks trigger
DropdownMenu.Trigger->>DropdownMenu.Root: Toggle open state
DropdownMenu.Root->>DropdownMenu.Portal: Render portal if open
DropdownMenu.Portal->>DropdownMenu.Content: Render menu content
DropdownMenu.Content->>DropdownMenu.Item: Render menu items
DropdownMenu.Item-->>User: User selects item
DropdownMenu.Content->>DropdownMenu.SubTrigger: Render sub-menu trigger
DropdownMenu.SubTrigger->>DropdownMenu.Sub: Open sub-menu
DropdownMenu.Sub->>DropdownMenu.Content: Render sub-menu content
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (15)
src/core/primitives/Menu/contexts/MenuPrimitiveRootContext.tsx (1)
17-19: Consider more specific typing for prop getters.The prop getter functions use
anytypes for both parameters and return values. While this provides flexibility, consider using more specific types if the floating-ui library provides them, or at leastRecord<string, unknown>for better type safety.- getReferenceProps: (userProps?: any) => any; - getFloatingProps: (userProps?: any) => any; - getItemProps: (userProps?: any) => any; + getReferenceProps: (userProps?: Record<string, unknown>) => Record<string, unknown>; + getFloatingProps: (userProps?: Record<string, unknown>) => Record<string, unknown>; + getItemProps: (userProps?: Record<string, unknown>) => Record<string, unknown>;styles/themes/components/dropdown-menu.scss (2)
51-51: Fix indentation formatting.Line 51 is missing proper indentation.
-width:max-content; + width: max-content;
73-73: Remove redundant width declarations.Both
.rad-ui-dropdown-menu-sub-triggerand.rad-ui-dropdown-menu-itemhavewidth: max-contentfollowed bywidth: 100%, where the latter overrides the former.- width:max-content; cursor: pointer; border-radius: 2px; display: flex; justify-content: space-between; align-items: center; width: 100%;Also applies to: 117-117
src/components/ui/DropdownMenu/fragments/DropdownMenuSub.tsx (2)
11-16: Consider using console.warn and improve error message.The context validation follows good practices, but consider using
console.warninstead ofconsole.logfor better visibility of this error condition.- console.log('DropdownMenuSub should be used in the DropdownMenuRoot'); + console.warn('DropdownMenuSub should be used within DropdownMenuRoot');
6-9: Consider spreading remaining props to MenuPrimitive.Sub.The component accepts
MenuPrimitiveProps.Subbut only useschildrenandclassName. Consider spreading the remaining props to ensure full primitive functionality is available.-const DropdownMenuSub = ({ children, className }:DropdownMenuSubProps) => { +const DropdownMenuSub = ({ children, className, ...props }:DropdownMenuSubProps) => { const context = React.useContext(DropdownMenuContext); if (!context) { - console.log('DropdownMenuSub should be used in the DropdownMenuRoot'); + console.warn('DropdownMenuSub should be used within DropdownMenuRoot'); return null; } const { rootClass } = context; return ( - <MenuPrimitive.Sub className={clsx(`${rootClass}-sub`, className)}> + <MenuPrimitive.Sub className={clsx(`${rootClass}-sub`, className)} {...props}> {children} </MenuPrimitive.Sub> );Also applies to: 11-22
src/components/ui/DropdownMenu/fragments/DropdownMenuContent.tsx (1)
11-23: LGTM! Consistent with established patterns.The component correctly follows the context-based styling pattern used throughout the codebase (similar to
DialogContent,AlertDialogContent, etc.). The implementation properly validates context availability and usesclsxfor className composition.Consider using
console.warninstead ofconsole.logfor consistency with warning patterns:- console.log('DropdownMenuContent should be used in the DropdownMenuRoot'); + console.warn('DropdownMenuContent should be used in the DropdownMenuRoot');src/components/ui/DropdownMenu/fragments/DropdownMenuItem.tsx (1)
12-24: LGTM! Follows established item component patterns.The component correctly implements the context-based styling pattern and properly forwards the
labelprop to the underlyingMenuPrimitive.Item. The structure is consistent with similar item components likeSelectItemshown in the relevant code snippets.For consistency with warning patterns, consider using
console.warn:- console.log('DropdownMenuItem should be used in the DropdownMenuRoot'); + console.warn('DropdownMenuItem should be used in the DropdownMenuRoot');src/core/primitives/Menu/stories/MenuPrimitive.stories.tsx (1)
20-44: Improve story content for better demonstration.The story has inconsistent and repetitive item labels that could confuse users trying to understand the component's functionality.
Consider applying these improvements for better clarity:
- <MenuPrimitive.Item className="px-4 py-2 hover:bg-gray-100 cursor-pointer rounded" label="item 1">item 1</MenuPrimitive.Item> - <MenuPrimitive.Item className="px-4 py-2 hover:bg-gray-100 cursor-pointer rounded" label="item 2">item 2</MenuPrimitive.Item> - <MenuPrimitive.Item className="px-4 py-2 hover:bg-gray-100 cursor-pointer rounded" label="item 3">item 3</MenuPrimitive.Item> + <MenuPrimitive.Item className="px-4 py-2 hover:bg-gray-100 cursor-pointer rounded" label="New Document">New Document</MenuPrimitive.Item> + <MenuPrimitive.Item className="px-4 py-2 hover:bg-gray-100 cursor-pointer rounded" label="Open File">Open File</MenuPrimitive.Item> + <MenuPrimitive.Item className="px-4 py-2 hover:bg-gray-100 cursor-pointer rounded" label="Save">Save</MenuPrimitive.Item>And fix the mismatch on line 28:
- <MenuPrimitive.Item className="px-4 py-2 hover:bg-gray-100 cursor-pointer rounded" label="item 3">item 1</MenuPrimitive.Item> + <MenuPrimitive.Item className="px-4 py-2 hover:bg-gray-100 cursor-pointer rounded" label="Export">Export</MenuPrimitive.Item>src/components/ui/DropdownMenu/fragments/DropdownMenuTrigger.tsx (2)
11-11: Fix formatting: Missing space after colon in function parameter.There's a missing space after the colon in the function parameter destructuring.
-const DropdownMenuTrigger = ({ children, className }:DropdownMenuTriggerProps) => { +const DropdownMenuTrigger = ({ children, className }: DropdownMenuTriggerProps) => {
13-16: Consider using console.warn instead of console.log for better error visibility.The warning message should use
console.warninstead ofconsole.logto make it more visible in development environments.- console.log('DropdownMenuTrigger should be used in the DropdownMenuRoot'); + console.warn('DropdownMenuTrigger should be used in the DropdownMenuRoot');src/components/ui/DropdownMenu/fragments/DropdownMenuRoot.tsx (1)
15-15: Fix formatting: Missing space after colon in function parameter.There's a missing space after the colon in the function parameter destructuring.
-const DropdownMenuRoot = ({ children, customRootClass, className }:DropdownMenuRootProps) => { +const DropdownMenuRoot = ({ children, customRootClass, className }: DropdownMenuRootProps) => {src/components/ui/DropdownMenu/stories/DropdownMenu.stories.tsx (1)
19-48: Consider removing empty className props or providing meaningful examples.The story includes many empty
className=""props throughout. Consider either removing these entirely or providing meaningful example class names to better demonstrate styling capabilities.- <DropdownMenu.Trigger className=""><Hamburger /></DropdownMenu.Trigger> + <DropdownMenu.Trigger><Hamburger /></DropdownMenu.Trigger>Or provide example classes:
- <DropdownMenu.Trigger className=""><Hamburger /></DropdownMenu.Trigger> + <DropdownMenu.Trigger className="btn btn-outline"><Hamburger /></DropdownMenu.Trigger>src/core/primitives/Menu/MenuPrimitive.tsx (1)
9-9: Incomplete warning messageThe warning message only mentions
RootandItemcomponents, but there are 6 available sub-components.- console.warn('Direct usage of MenuPrimitive is not supported. Please use MenuPrimitive.Root, MenuPrimitive.Item instead.'); + console.warn('Direct usage of MenuPrimitive is not supported. Please use MenuPrimitive.Root, MenuPrimitive.Item, MenuPrimitive.Trigger, MenuPrimitive.Content, MenuPrimitive.Sub, or MenuPrimitive.Portal instead.');src/core/primitives/Menu/fragments/MenuPrimitiveRoot.tsx (2)
37-41: Consider additional floating UI middlewareThe current setup only uses
flipmiddleware. Consider addingoffsetandshiftmiddleware for better positioning control, especially to prevent the menu from touching viewport edges.middleware: [ + Floater.offset(4), Floater.flip({ mainAxis: true - }) + }), + Floater.shift({ padding: 8 }) ],
94-106: Remove unused dependencies from useEffectThe
nodeIdandparentIdare not used within the effect but are included in the dependencies array, causing unnecessary effect re-runs.- }, [tree, nodeId, parentId]); + }, [tree, setIsOpen]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
src/components/ui/DropdownMenu/DropdownMenu.tsx(1 hunks)src/components/ui/DropdownMenu/contexts/DropdownMenuContext.tsx(1 hunks)src/components/ui/DropdownMenu/fragments/DropdownMenuContent.tsx(1 hunks)src/components/ui/DropdownMenu/fragments/DropdownMenuItem.tsx(1 hunks)src/components/ui/DropdownMenu/fragments/DropdownMenuPortal.tsx(1 hunks)src/components/ui/DropdownMenu/fragments/DropdownMenuRoot.tsx(1 hunks)src/components/ui/DropdownMenu/fragments/DropdownMenuSub.tsx(1 hunks)src/components/ui/DropdownMenu/fragments/DropdownMenuSubTrigger.tsx(1 hunks)src/components/ui/DropdownMenu/fragments/DropdownMenuTrigger.tsx(1 hunks)src/components/ui/DropdownMenu/stories/DropdownMenu.stories.tsx(1 hunks)src/core/primitives/Floater/index.tsx(1 hunks)src/core/primitives/Menu/MenuPrimitive.tsx(1 hunks)src/core/primitives/Menu/contexts/MenuPrimitiveRootContext.tsx(1 hunks)src/core/primitives/Menu/fragments/MenuPrimitiveContent.tsx(1 hunks)src/core/primitives/Menu/fragments/MenuPrimitiveItem.tsx(1 hunks)src/core/primitives/Menu/fragments/MenuPrimitivePortal.tsx(1 hunks)src/core/primitives/Menu/fragments/MenuPrimitiveRoot.tsx(1 hunks)src/core/primitives/Menu/fragments/MenuPrimitiveSub.tsx(1 hunks)src/core/primitives/Menu/fragments/MenuPrimitiveTrigger.tsx(1 hunks)src/core/primitives/Menu/stories/MenuPrimitive.stories.tsx(1 hunks)styles/themes/components/dropdown-menu.scss(1 hunks)styles/themes/default.scss(1 hunks)
🧰 Additional context used
🧠 Learnings (22)
📓 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`.
Learnt from: decipher-cs
PR: rad-ui/ui#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`.
styles/themes/default.scss (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: 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.
src/components/ui/DropdownMenu/contexts/DropdownMenuContext.tsx (2)
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.
src/components/ui/DropdownMenu/fragments/DropdownMenuContent.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: 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.
src/components/ui/DropdownMenu/fragments/DropdownMenuSub.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: 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.
src/core/primitives/Menu/fragments/MenuPrimitiveSub.tsx (1)
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/core/primitives/Menu/fragments/MenuPrimitivePortal.tsx (1)
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/core/primitives/Menu/stories/MenuPrimitive.stories.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/core/primitives/Menu/fragments/MenuPrimitiveTrigger.tsx (4)
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.
Learnt from: GoldGroove06
PR: #1215
File: src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveTrigger.tsx:25-27
Timestamp: 2025-07-18T16:43:26.175Z
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.
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.
src/components/ui/DropdownMenu/fragments/DropdownMenuPortal.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: 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.
src/components/ui/DropdownMenu/fragments/DropdownMenuItem.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: 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.
src/components/ui/DropdownMenu/fragments/DropdownMenuTrigger.tsx (4)
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.175Z
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.
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/core/primitives/Menu/fragments/MenuPrimitiveContent.tsx (1)
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/DropdownMenu/DropdownMenu.tsx (2)
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.
src/components/ui/DropdownMenu/fragments/DropdownMenuSubTrigger.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.175Z
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/core/primitives/Menu/contexts/MenuPrimitiveRootContext.tsx (2)
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.
Learnt from: GoldGroove06
PR: #1215
File: src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveTrigger.tsx:25-27
Timestamp: 2025-07-18T16:43:26.175Z
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/core/primitives/Menu/fragments/MenuPrimitiveRoot.tsx (3)
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.
Learnt from: GoldGroove06
PR: #1215
File: src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveTrigger.tsx:25-27
Timestamp: 2025-07-18T16:43:26.175Z
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.
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.
src/components/ui/DropdownMenu/fragments/DropdownMenuRoot.tsx (2)
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.
src/core/primitives/Menu/fragments/MenuPrimitiveItem.tsx (2)
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.
Learnt from: GoldGroove06
PR: #1215
File: src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveTrigger.tsx:25-27
Timestamp: 2025-07-18T16:43:26.175Z
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/DropdownMenu/stories/DropdownMenu.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: 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.
src/core/primitives/Menu/MenuPrimitive.tsx (2)
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.
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.
styles/themes/components/dropdown-menu.scss (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: 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.
🧬 Code Graph Analysis (9)
src/components/ui/DropdownMenu/contexts/DropdownMenuContext.tsx (4)
src/components/ui/Select/contexts/SelectRootContext.tsx (1)
SelectRootContextType(3-5)src/components/ui/Callout/contexts/CalloutContext.tsx (1)
CalloutContextType(3-5)src/components/ui/CheckboxGroup/context/CheckboxGroupRootContext.tsx (1)
CheckboxGroupRootContextProps(5-7)src/components/ui/CheckboxCards/context/CheckboxCardsRootContext.tsx (1)
CheckboxCardsRootContextProps(5-7)
src/components/ui/DropdownMenu/fragments/DropdownMenuContent.tsx (2)
src/core/primitives/Menu/MenuPrimitive.tsx (1)
Content(24-24)src/components/ui/Select/fragments/SelectContent.tsx (1)
SelectContent(6-20)
src/components/ui/DropdownMenu/fragments/DropdownMenuSub.tsx (2)
src/core/primitives/Menu/MenuPrimitive.tsx (1)
Sub(25-25)src/components/ui/Select/fragments/SelectRoot.tsx (1)
SelectRoot(8-26)
src/core/primitives/Menu/fragments/MenuPrimitivePortal.tsx (1)
src/core/primitives/Select/fragments/SelectPrimitivePortal.tsx (1)
SelectPrimitivePortal(6-24)
src/components/ui/DropdownMenu/fragments/DropdownMenuItem.tsx (2)
src/core/primitives/Menu/MenuPrimitive.tsx (1)
Item(22-22)src/components/ui/Select/fragments/SelectItem.tsx (1)
SelectItem(6-22)
src/core/primitives/Menu/fragments/MenuPrimitiveContent.tsx (1)
src/core/primitives/Select/fragments/SelectPrimitiveContent.tsx (1)
SelectPrimitiveContent(12-59)
src/components/ui/DropdownMenu/fragments/DropdownMenuSubTrigger.tsx (2)
src/core/primitives/Menu/MenuPrimitive.tsx (1)
Trigger(23-23)src/components/ui/Select/fragments/SelectTrigger.tsx (1)
SelectTrigger(7-24)
src/components/ui/DropdownMenu/fragments/DropdownMenuRoot.tsx (2)
src/core/primitives/Menu/MenuPrimitive.tsx (1)
Root(21-21)src/components/ui/Select/fragments/SelectRoot.tsx (1)
SelectRoot(8-26)
src/core/primitives/Menu/MenuPrimitive.tsx (5)
src/core/primitives/Menu/fragments/MenuPrimitiveRoot.tsx (1)
MenuPrimitiveRootProps(6-12)src/core/primitives/Menu/fragments/MenuPrimitiveItem.tsx (1)
MenuPrimitiveItemProps(5-9)src/core/primitives/Menu/fragments/MenuPrimitiveTrigger.tsx (1)
MenuPrimitiveTriggerProps(5-8)src/core/primitives/Menu/fragments/MenuPrimitiveContent.tsx (1)
MenuPrimitiveContentProps(6-9)src/core/primitives/Menu/fragments/MenuPrimitiveSub.tsx (1)
MenuPrimitiveSubProps(4-10)
🪛 Biome (2.1.2)
src/core/primitives/Menu/fragments/MenuPrimitivePortal.tsx
[error] 10-10: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 13-13: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
src/core/primitives/Menu/fragments/MenuPrimitiveTrigger.tsx
[error] 14-14: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 24-24: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
src/core/primitives/Menu/fragments/MenuPrimitiveItem.tsx
[error] 15-15: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 18-18: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🪛 GitHub Check: lint
src/components/ui/DropdownMenu/fragments/DropdownMenuPortal.tsx
[warning] 15-15:
'rootClass' is assigned a value but never used
⏰ 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
🔇 Additional comments (21)
src/components/ui/DropdownMenu/contexts/DropdownMenuContext.tsx (2)
5-7: LGTM! Context interface follows established patterns.The
DropdownMenuContextPropsinterface is consistent with other component contexts in the codebase (Select, CheckboxGroup, etc.), maintaining the standardrootClassproperty pattern for theming.
9-11: LGTM! Context implementation follows React best practices.The context creation and default export are correctly implemented, following the established pattern used throughout the codebase.
src/core/primitives/Menu/contexts/MenuPrimitiveRootContext.tsx (1)
5-29: Comprehensive interface for floating UI menu primitives.The
MenuPrimitiveRootPrimitiveContextPropsinterface effectively encapsulates all necessary state and functionality for floating UI menus, including state management, DOM references, positioning, and interaction handlers. The typing appears appropriate for floating-ui integration.styles/themes/components/dropdown-menu.scss (2)
30-33: LGTM! Excellent accessibility implementation.The focus styles properly replace browser outlines with custom box-shadow, maintaining accessibility while providing consistent visual feedback across browsers.
150-159: LGTM! Proper selected state implementation.The
aria-selected="true"styling provides clear visual feedback for selected items with appropriate hover states, following accessibility best practices.styles/themes/default.scss (1)
40-40: LGTM! Proper theme integration.The dropdown-menu styles are correctly imported following the established pattern and maintaining alphabetical order with other component imports.
src/components/ui/DropdownMenu/fragments/DropdownMenuSub.tsx (1)
19-21: LGTM! Proper className composition and primitive usage.The component correctly composes the className using the context's rootClass and renders the underlying MenuPrimitive.Sub with the children.
src/core/primitives/Menu/fragments/MenuPrimitiveSub.tsx (1)
4-21: LGTM! Well-structured primitive component.The
MenuPrimitiveSubcomponent follows good React patterns with proper TypeScript typing, appropriate default values, and clean prop forwarding to the underlyingMenuComponentRoot. The implementation is concise and serves its purpose as a wrapper for nested menu functionality.src/components/ui/DropdownMenu/DropdownMenu.tsx (1)
9-22: LGTM! Well-structured compound component pattern.The implementation correctly follows the compound component pattern with proper developer guidance. The warning message helps prevent misuse, and the static property assignments provide a clean API for accessing subcomponents. This approach is consistent with established patterns in the codebase.
src/core/primitives/Floater/index.tsx (1)
1-32: LGTM! Expanded floating UI exports.The expanded exports from
@floating-ui/reactare properly imported and re-exported to support the new menu primitive functionality. The additions are consistent with the existing pattern and provide the necessary utilities for floating menus.src/components/ui/DropdownMenu/fragments/DropdownMenuTrigger.tsx (2)
6-9: Props interface follows proper extension pattern.The props interface correctly extends
MenuPrimitiveProps.Triggerwhile adding component-specific props. This follows the established pattern seen in other UI components.
18-22: Context integration and className composition look correct.The component properly consumes the context to get
rootClassand usesclsxto compose the final className with the-triggersuffix. This follows the established pattern from other UI components in the codebase.src/components/ui/DropdownMenu/fragments/DropdownMenuRoot.tsx (2)
7-11: Props interface correctly extends MenuPrimitive props.The props interface properly extends
MenuPrimitiveProps.Rootand includes the standardcustomRootClassandclassNameprops. This is consistent with other root components in the codebase.
16-23: Root component implementation follows established pattern.The implementation correctly follows the established pattern seen in other root components:
- Uses
customClassSwitcherto generate the root class- Provides context with the root class
- Renders the primitive root with composed className
- Structure matches
SelectRoot,DialogRoot, and other root componentssrc/components/ui/DropdownMenu/stories/DropdownMenu.stories.tsx (2)
13-15: Inline SVG components are well-implemented.The
ChevronRightandHamburgericon components are properly implemented as functional components with appropriate SVG attributes. The viewBox and paths look correct for their intended icons.
16-51: Story demonstrates comprehensive DropdownMenu usage.The story effectively demonstrates the full API surface of the DropdownMenu component, including:
- Root with customRootClass and className props
- Trigger with icon
- Portal for proper rendering
- Nested Content and Items
- Submenu structure with Sub and SubTrigger
- Multiple levels of nesting
This provides a good example for developers using the component.
src/core/primitives/Menu/fragments/MenuPrimitiveItem.tsx (2)
21-33: Button implementation has good accessibility features.The button element properly implements:
- Ref forwarding for list item management
- Dynamic tabIndex based on active state (0 for active, -1 for inactive)
- Event handling with tree event emission
- Proper prop spreading from getItemProps
5-9: Props interface is well-defined.The props interface includes appropriate properties for a menu item: children for content, optional className for styling, and optional label for accessibility.
src/core/primitives/Menu/fragments/MenuPrimitiveContent.tsx (3)
11-14: Context handling and conditional rendering are correct.The component properly checks for both context existence and
isOpenstate before rendering, which prevents unnecessary DOM nodes when the menu is closed. This pattern is consistent with other floating content components.
18-34: Floating UI integration follows best practices.The component correctly integrates with the floating UI system:
- Uses
FloatingListfor proper list item management- Configures
FocusManagerwith appropriate settings for nested vs non-nested menus- Applies floating styles and props correctly
- Structure is consistent with other floating content components in the codebase
22-23: Focus management configuration is well thought out.The
initialFocusandreturnFocusprops are properly configured based on theisNestedstate:
- Nested menus use
initialFocus={-1}andreturnFocus={false}- Root menus use
initialFocus={0}andreturnFocus={true}This ensures proper focus behavior in both nested and standalone menu scenarios.
| console.log('DropdownMenuPortal should be used in the DropdownMenuRoot'); | ||
| return null; | ||
| } | ||
| const { rootClass } = context; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove unused variable.
The rootClass variable is extracted from context but never used in the component.
Apply this diff to remove the unused variable:
const { rootClass } = context;
- const { rootClass } = context;Or if the rootClass will be used in the future, consider adding a comment explaining its intended purpose.
📝 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 { rootClass } = context; |
🧰 Tools
🪛 GitHub Check: lint
[warning] 15-15:
'rootClass' is assigned a value but never used
🤖 Prompt for AI Agents
In src/components/ui/DropdownMenu/fragments/DropdownMenuPortal.tsx at line 15,
the variable rootClass is destructured from context but never used in the
component. Remove the declaration of rootClass to clean up unused code. If
rootClass is intended for future use, add a comment explaining its purpose
instead of removing it.
| const DropdownMenuSubTrigger = ({ children, className }:DropdownMenuSubTriggerProps) => { | ||
| const context = React.useContext(DropdownMenuContext); | ||
| if (!context) { | ||
| console.log('DropdownMenuSubTrigger should be used in the DropdownMenuRoot'); | ||
| return null; | ||
| } | ||
| const { rootClass } = context; | ||
| return ( | ||
| <MenuPrimitive.Trigger className={clsx(`${rootClass}-sub-trigger`, className)}> | ||
| {children} | ||
| </MenuPrimitive.Trigger> | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add prop forwarding for complete API support.
The component extends MenuPrimitiveProps.Trigger but doesn't forward additional props to the underlying MenuPrimitive.Trigger, which may limit its functionality.
Apply this diff to add proper prop forwarding:
-const DropdownMenuSubTrigger = ({ children, className }:DropdownMenuSubTriggerProps) => {
+const DropdownMenuSubTrigger = ({ children, className, ...props }:DropdownMenuSubTriggerProps) => {
const context = React.useContext(DropdownMenuContext);
if (!context) {
console.log('DropdownMenuSubTrigger should be used in the DropdownMenuRoot');
return null;
}
const { rootClass } = context;
return (
- <MenuPrimitive.Trigger className={clsx(`${rootClass}-sub-trigger`, className)}>
+ <MenuPrimitive.Trigger className={clsx(`${rootClass}-sub-trigger`, className)} {...props}>
{children}
</MenuPrimitive.Trigger>
);This ensures all trigger-related props are properly forwarded to the underlying primitive.
📝 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 DropdownMenuSubTrigger = ({ children, className }:DropdownMenuSubTriggerProps) => { | |
| const context = React.useContext(DropdownMenuContext); | |
| if (!context) { | |
| console.log('DropdownMenuSubTrigger should be used in the DropdownMenuRoot'); | |
| return null; | |
| } | |
| const { rootClass } = context; | |
| return ( | |
| <MenuPrimitive.Trigger className={clsx(`${rootClass}-sub-trigger`, className)}> | |
| {children} | |
| </MenuPrimitive.Trigger> | |
| ); | |
| const DropdownMenuSubTrigger = ({ children, className, ...props }: DropdownMenuSubTriggerProps) => { | |
| const context = React.useContext(DropdownMenuContext); | |
| if (!context) { | |
| console.log('DropdownMenuSubTrigger should be used in the DropdownMenuRoot'); | |
| return null; | |
| } | |
| const { rootClass } = context; | |
| return ( | |
| <MenuPrimitive.Trigger | |
| className={clsx(`${rootClass}-sub-trigger`, className)} | |
| {...props} | |
| > | |
| {children} | |
| </MenuPrimitive.Trigger> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In src/components/ui/DropdownMenu/fragments/DropdownMenuSubTrigger.tsx around
lines 11 to 22, the DropdownMenuSubTrigger component does not forward additional
props to the underlying MenuPrimitive.Trigger, limiting its API support. To fix
this, update the component to accept and forward all extra props to
MenuPrimitive.Trigger by spreading the remaining props onto it. This will ensure
full compatibility with the MenuPrimitive.Trigger API.
| const context = React.useContext(MenuPrimitiveRootContext); | ||
| if (!context) return null; | ||
| const { activeIndex, getItemProps } = context; | ||
| const { ref, index } = Floater.useListItem({ | ||
| label | ||
| }); | ||
| const tree = Floater.useFloatingTree(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Fix hook ordering violation - hooks called after early return.
The component violates the Rules of Hooks by calling Floater.useListItem and Floater.useFloatingTree after a conditional early return. This can cause React to lose track of hook order between renders.
Move the hooks before the context check:
const MenuPrimitiveItem = ({ children, className, label }: MenuPrimitiveItemProps) => {
const context = React.useContext(MenuPrimitiveRootContext);
- if (!context) return null;
- const { activeIndex, getItemProps } = context;
const { ref, index } = Floater.useListItem({
label
});
const tree = Floater.useFloatingTree();
+
+ if (!context) return null;
+ const { activeIndex, getItemProps } = context;
const isActive = activeIndex === index;📝 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 context = React.useContext(MenuPrimitiveRootContext); | |
| if (!context) return null; | |
| const { activeIndex, getItemProps } = context; | |
| const { ref, index } = Floater.useListItem({ | |
| label | |
| }); | |
| const tree = Floater.useFloatingTree(); | |
| const MenuPrimitiveItem = ({ children, className, label }: MenuPrimitiveItemProps) => { | |
| const context = React.useContext(MenuPrimitiveRootContext); | |
| const { ref, index } = Floater.useListItem({ | |
| label | |
| }); | |
| const tree = Floater.useFloatingTree(); | |
| if (!context) return null; | |
| const { activeIndex, getItemProps } = context; | |
| const isActive = activeIndex === index; | |
| // ...rest of render logic | |
| }; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 15-15: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 18-18: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🤖 Prompt for AI Agents
In src/core/primitives/Menu/fragments/MenuPrimitiveItem.tsx around lines 12 to
18, the hooks Floater.useListItem and Floater.useFloatingTree are called after
an early return caused by a missing context, violating the Rules of Hooks. To
fix this, move all hook calls, including React.useContext, Floater.useListItem,
and Floater.useFloatingTree, to the top of the component before any conditional
returns to ensure consistent hook order across renders.
| function MenuPrimitivePortal({ children }: { children: React.ReactNode }) { | ||
| const context = useContext(MenuPrimitiveRootContext); | ||
| if (!context) return null; | ||
| const { isOpen } = context; | ||
| const [rootElementFound, setRootElementFound] = useState(false); | ||
| const rootElement = document.querySelector('#rad-ui-theme-container') || document.body as HTMLElement | null; | ||
|
|
||
| useEffect(() => { | ||
| if (rootElement) { | ||
| setRootElementFound(true); | ||
| } | ||
| }, [rootElement]); | ||
|
|
||
| if (!isOpen || !rootElementFound) return null; | ||
| return ( | ||
| <Floater.Portal root={rootElement}> | ||
| {children} | ||
| </Floater.Portal> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix React hooks rule violation.
The component violates React's rules of hooks by calling useState and useEffect after a conditional return statement. All hooks must be called unconditionally at the top level.
Apply this diff to fix the hooks rule violation:
function MenuPrimitivePortal({ children }: { children: React.ReactNode }) {
- const context = useContext(MenuPrimitiveRootContext);
- if (!context) return null;
- const { isOpen } = context;
const [rootElementFound, setRootElementFound] = useState(false);
const rootElement = document.querySelector('#rad-ui-theme-container') || document.body as HTMLElement | null;
useEffect(() => {
if (rootElement) {
setRootElementFound(true);
}
}, [rootElement]);
+ const context = useContext(MenuPrimitiveRootContext);
+ if (!context) return null;
+ const { isOpen } = context;
+
if (!isOpen || !rootElementFound) return null;
return (
<Floater.Portal root={rootElement}>
{children}
</Floater.Portal>
);
}📝 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.
| function MenuPrimitivePortal({ children }: { children: React.ReactNode }) { | |
| const context = useContext(MenuPrimitiveRootContext); | |
| if (!context) return null; | |
| const { isOpen } = context; | |
| const [rootElementFound, setRootElementFound] = useState(false); | |
| const rootElement = document.querySelector('#rad-ui-theme-container') || document.body as HTMLElement | null; | |
| useEffect(() => { | |
| if (rootElement) { | |
| setRootElementFound(true); | |
| } | |
| }, [rootElement]); | |
| if (!isOpen || !rootElementFound) return null; | |
| return ( | |
| <Floater.Portal root={rootElement}> | |
| {children} | |
| </Floater.Portal> | |
| ); | |
| } | |
| function MenuPrimitivePortal({ children }: { children: React.ReactNode }) { | |
| const [rootElementFound, setRootElementFound] = useState(false); | |
| const rootElement = | |
| (document.querySelector('#rad-ui-theme-container') || | |
| document.body) as HTMLElement | null; | |
| useEffect(() => { | |
| if (rootElement) { | |
| setRootElementFound(true); | |
| } | |
| }, [rootElement]); | |
| const context = useContext(MenuPrimitiveRootContext); | |
| if (!context) return null; | |
| const { isOpen } = context; | |
| if (!isOpen || !rootElementFound) return null; | |
| return ( | |
| <Floater.Portal root={rootElement}> | |
| {children} | |
| </Floater.Portal> | |
| ); | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 10-10: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 13-13: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🤖 Prompt for AI Agents
In src/core/primitives/Menu/fragments/MenuPrimitivePortal.tsx lines 6 to 25, the
useState and useEffect hooks are called after a conditional return, violating
React's rules of hooks. To fix this, move all hook calls (useContext, useState,
useEffect) to the top of the component before any conditional returns. Then
perform conditional returns only after all hooks have been called
unconditionally.
| const MenuPrimitiveTrigger = ({ children, className }: MenuPrimitiveTriggerProps) => { | ||
| const context = useContext(MenuPrimitiveRootContext); | ||
| if (!context) return null; | ||
| const { isOpen, setIsOpen, activeIndex, refs, floatingStyles, getReferenceProps, isNested } = context; | ||
| const { ref, index } = Floater.useListItem(); | ||
|
|
||
| return ( | ||
| <button | ||
| className={className} | ||
| onClick={() => setIsOpen(!isOpen)} | ||
| tabIndex={ | ||
| !isNested ? undefined : activeIndex === index ? 0 : -1 | ||
| } | ||
|
|
||
| ref={Floater.useMergeRefs([refs.setReference, ref])} | ||
| {...getReferenceProps()} | ||
| > | ||
| {children} | ||
| </button> | ||
| ); | ||
| }; | ||
| export default MenuPrimitiveTrigger; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix React hooks rules violation.
The component violates the Rules of Hooks by calling hooks after a conditional early return. Floater.useListItem() (line 14) and Floater.useMergeRefs() (line 24) are called after the early return on line 12.
Restructure the component to call all hooks before any conditional logic:
const MenuPrimitiveTrigger = ({ children, className }: MenuPrimitiveTriggerProps) => {
+ const { ref, index } = Floater.useListItem();
const context = useContext(MenuPrimitiveRootContext);
- if (!context) return null;
+
+ if (!context) return null;
+
const { isOpen, setIsOpen, activeIndex, refs, floatingStyles, getReferenceProps, isNested } = context;
- const { ref, index } = Floater.useListItem();
return (
<button
className={className}
onClick={() => setIsOpen(!isOpen)}
tabIndex={
!isNested ? undefined : activeIndex === index ? 0 : -1
}
ref={Floater.useMergeRefs([refs.setReference, ref])}
{...getReferenceProps()}
>
{children}
</button>
);
};Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (2.1.2)
[error] 14-14: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 24-24: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🤖 Prompt for AI Agents
In src/core/primitives/Menu/fragments/MenuPrimitiveTrigger.tsx around lines 10
to 31, the hooks Floater.useListItem() and Floater.useMergeRefs() are called
after an early return, violating React hooks rules. To fix this, move all hook
calls, including useContext and Floater hooks, to the top of the component
before any conditional returns. Then, perform the conditional check on context
and return null if needed, ensuring hooks are always called in the same order.
| import MenuPrimitiveTrigger, { MenuPrimitiveTriggerProps } from './fragments/MenuPrimitiveTrigger'; | ||
| import MenuPrimitiveContent, { MenuPrimitiveContentProps } from './fragments/MenuPrimitiveContent'; | ||
| import MenuPrimitiveSub, { MenuPrimitiveSubProps } from './fragments/MenuPrimitiveSub'; | ||
| import MenuPrimitivePortal from './fragments/MenuPrimitivePortal'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing props type for MenuPrimitivePortal
The MenuPrimitivePortal component is imported without its corresponding props type, creating an inconsistency with the other imports. This also means there's no Portal type in the MenuPrimitiveProps namespace.
If MenuPrimitivePortal has props, import them:
-import MenuPrimitivePortal from './fragments/MenuPrimitivePortal';
+import MenuPrimitivePortal, { MenuPrimitivePortalProps } from './fragments/MenuPrimitivePortal';And add to the namespace:
export namespace MenuPrimitiveProps {
export type Root = MenuPrimitiveRootProps;
export type Item = MenuPrimitiveItemProps;
export type Trigger = MenuPrimitiveTriggerProps;
export type Content = MenuPrimitiveContentProps;
export type Sub = MenuPrimitiveSubProps;
+ export type Portal = MenuPrimitivePortalProps;
}If MenuPrimitivePortal doesn't have props, consider documenting this or using a type alias for consistency.
Also applies to: 20-26
🤖 Prompt for AI Agents
In src/core/primitives/Menu/MenuPrimitive.tsx around line 6 and also lines 20 to
26, the import of MenuPrimitivePortal lacks its corresponding props type,
causing inconsistency with other imports and missing the Portal type in the
MenuPrimitiveProps namespace. To fix this, check if MenuPrimitivePortal has
defined props; if yes, import its props type alongside the component and add
that type to the MenuPrimitiveProps namespace. If it does not have props, add a
type alias or document this explicitly for consistency with other components.
Screen.Recording.2025-07-27.232612.mp4
Summary by CodeRabbit
New Features
Style
Documentation