Conversation
kylengn
requested review from
EnzalRad,
dvxam and
noe-charmet
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughA new DropdownMenu component based on Radix UI is introduced with full composable subcomponents, styling variants, and comprehensive Storybook stories demonstrating various menu configurations. The DynamicItem component is updated to accept ReactNode values for descriptions, enabling richer content rendering alongside prop type clarifications. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
libs/react/ui/src/components/dropdown-menu/dropdown-menu.tsx (2)
24-26: Consider removing standalone DropdownMenuPortal export.
DropdownMenuContentalready renders its children inside aDropdownMenuPrimitive.Portal(line 67). Exporting a standaloneDropdownMenuPortalmay confuse consumers and lead to double-portaling. Additionally,data-sloton the Portal primitive won't render to the DOM since Portal doesn't create an element.If there's a valid use case for standalone portal usage, consider documenting it; otherwise, removing this export simplifies the API.
369-379: Consider extracting shared base styles between Content and SubContent.The animation and appearance classes here largely duplicate those in
dropdownMenuContentVariants(lines 29-37). Extracting a shared base constant would reduce duplication and ensure consistent updates.const dropdownMenuPopoverBase = [ 'overflow-hidden rounded-10 p-4', 'bg-background-neutral-overlay text-foreground-neutral-subtle', 'shadow-tooltip', 'data-[state=open]:animate-in data-[state=closed]:animate-out', 'data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0', 'data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95', 'data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2', 'data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2', ];libs/react/ui/src/components/dropdown-menu/dropdown-menu.stories.tsx (1)
53-61: Hardcoded timeouts may cause flaky tests.The arbitrary delays (300ms, 100ms) could lead to flakiness if animations or rendering take longer on slower CI runners. Consider using
waitForor polling for a stable state if this becomes problematic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
libs/react/ui/src/components/dropdown-menu/dropdown-menu.stories.tsx(1 hunks)libs/react/ui/src/components/dropdown-menu/dropdown-menu.tsx(1 hunks)libs/react/ui/src/components/dropdown-menu/index.ts(1 hunks)libs/react/ui/src/components/dynamic-item/dynamic-item.stories.tsx(1 hunks)libs/react/ui/src/components/dynamic-item/dynamic-item.tsx(2 hunks)libs/react/ui/src/components/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
We handle errors at the edge of our applications in most cases. Do not recommend to add error handling around every single function. We prefer them to bubble up and be handled at upper layers.
Files:
libs/react/ui/src/components/dropdown-menu/index.tslibs/react/ui/src/components/dynamic-item/dynamic-item.tsxlibs/react/ui/src/components/dropdown-menu/dropdown-menu.tsxlibs/react/ui/src/components/index.tslibs/react/ui/src/components/dropdown-menu/dropdown-menu.stories.tsxlibs/react/ui/src/components/dynamic-item/dynamic-item.stories.tsx
🧬 Code graph analysis (2)
libs/react/ui/src/components/dynamic-item/dynamic-item.tsx (1)
libs/react/ui/src/components/item/item.tsx (2)
ItemTitle(156-169)ItemDescription(84-97)
libs/react/ui/src/components/dynamic-item/dynamic-item.stories.tsx (1)
libs/react/ui/src/components/icon/icon.tsx (1)
Icon(83-86)
⏰ 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). (2)
- GitHub Check: Agent
- GitHub Check: Continuous integration
🔇 Additional comments (11)
libs/react/ui/src/components/dynamic-item/dynamic-item.stories.tsx (1)
147-152: LGTM!The JSX-based description with inline icon demonstrates the new ReactNode capability cleanly, using consistent styling tokens and the existing Icon component.
libs/react/ui/src/components/dynamic-item/dynamic-item.tsx (1)
52-59: LGTM!The conditional rendering correctly differentiates between string and ReactNode types, wrapping strings in the appropriate styled components while allowing custom ReactNode content to render directly.
libs/react/ui/src/components/dropdown-menu/dropdown-menu.tsx (1)
1-9: Well-structured Radix UI wrapper.Clean implementation with proper TypeScript typing, data-slot attributes for testing, and flexible props like
closeOnSelectandcontainer. The component API is comprehensive and follows established patterns.libs/react/ui/src/components/index.ts (1)
8-8: LGTM!New dropdown-menu export is properly placed in alphabetical order, maintaining consistency with the existing export structure.
libs/react/ui/src/components/dropdown-menu/index.ts (1)
1-29: LGTM!Explicit named exports provide a clear public API surface and enable better tree-shaking. Clean separation of type and value exports.
libs/react/ui/src/components/dropdown-menu/dropdown-menu.stories.tsx (6)
70-128: LGTM!Well-structured meta configuration with comprehensive argTypes, proper TypeScript usage with
satisfies, and good documentation for each control.
133-159: LGTM!Good implementation of the Default story with the container ref pattern for consistent visual regression testing and a clear demonstration of basic dropdown functionality.
161-193: LGTM!Good demonstration of the keyboard shortcuts feature with appropriate Mac-style symbols.
195-223: LGTM!Well-structured helper components that demonstrate realistic content within the dropdown menu stories.
225-298: LGTM!Comprehensive demonstration of the submenu functionality with appropriate visual regression testing for both the main menu and submenu states.
300-384: LGTM!Excellent comprehensive example demonstrating the full feature set including checkbox items, radio groups, and proper use of
closeOnSelect={false}for multi-selection scenarios. This serves as great documentation for complex dropdown menu implementations.
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
There was a problem hiding this comment.
Pull request overview
This PR adds a new DropdownMenu component to the UI library, built on Radix UI primitives with comprehensive support for menu items, checkboxes, radio groups, submenus, and keyboard shortcuts. Additionally, it enhances the existing DynamicItem component to accept ReactNode for the description prop instead of just strings.
Key Changes:
- New DropdownMenu component with full-featured menu functionality including items, groups, separators, checkboxes, radio items, submenus, and keyboard shortcuts
- Enhanced DynamicItem component to support ReactNode descriptions for richer content display
- Comprehensive Storybook stories demonstrating various menu patterns and use cases
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/react/ui/src/components/index.ts | Exports the new dropdown-menu component |
| libs/react/ui/src/components/dropdown-menu/index.ts | Re-exports all DropdownMenu components and types following codebase conventions |
| libs/react/ui/src/components/dropdown-menu/dropdown-menu.tsx | Implements the complete DropdownMenu component with variants, icons, shortcuts, and submenu support |
| libs/react/ui/src/components/dropdown-menu/dropdown-menu.stories.tsx | Provides Storybook stories with visual regression tests for various menu scenarios |
| libs/react/ui/src/components/dynamic-item/dynamic-item.tsx | Updates description prop type to accept ReactNode and handles conditional rendering |
| libs/react/ui/src/components/dynamic-item/dynamic-item.stories.tsx | Demonstrates the enhanced description prop with a ReactNode example |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.