Conversation
|
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 WalkthroughThis pull request introduces two new UI components (Combobox and ScrollArea) to the react-ui library, enhances the Command component with loading state support, and provides Storybook documentation for Combobox. Barrel exports are updated to expose the new components publicly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Combobox
participant Popover
participant Command
participant Selection
User->>Combobox: Click trigger / focus
Combobox->>Popover: Open
Popover->>Command: Render options list
User->>Command: Type search query
Command->>Command: Filter options via useMemo
Command->>Command: Render filtered results
User->>Command: Click/select option
Command->>Selection: Update value state
Selection->>Combobox: Trigger onValueChange callback
Combobox->>Popover: Close
Combobox->>User: Display selected label
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @libs/react/ui/src/components/combobox/combobox.stories.tsx:
- Around line 27-52: Default story incorrectly uses the selected `value` as the
search query causing filtered options to match the selection; update the
`Default` story in the `Default` export by removing the `useMemo` filter that
uses `value` (the `items` computed from `useMemo` and the dependency on
`value`), and instead pass the unfiltered `sampleItems` to `Combobox` (or
introduce a separate `search` state used for filtering if you intend to demo
external filtering), keeping `value` and `setValue` only for the selected
option.
🧹 Nitpick comments (3)
libs/react/ui/src/components/combobox/combobox.tsx (2)
17-34: Consider removing duplicateisLoadingdeclaration.
ComboboxPropsextendsCommandTriggerPropswhich already includesisLoading?: boolean(added in command.tsx line 39). The explicit redeclaration on line 33 is redundant and could diverge if the base type changes.♻️ Suggested fix
export type ComboboxProps = Omit<CommandTriggerProps, 'children' | 'placeholder'> & { options: ComboboxOption[]; value?: string; onValueChange?: (value: string) => void; placeholder?: string; emptyState?: string | React.ReactNode; searchPlaceholder?: string; className?: string; popoverClassName?: string; align?: 'start' | 'center' | 'end'; sideOffset?: number; - isLoading?: boolean; };
94-118: Consider potential nested scrolling conflict.
CommandListalready appliesoverflow-y-auto(per command.tsx line 183), whileScrollAreaintroduces its own scroll viewport. This nesting may cause double scrollbars or unexpected scroll behavior.Consider either:
- Removing the
ScrollAreawrapper and relying onCommandList's native scrolling, or- Overriding
CommandList's overflow styling when wrapped inScrollArea♻️ Option 2: Override CommandList overflow
<ScrollArea> - <CommandList className="max-h-300"> + <CommandList className="max-h-300 overflow-visible"> <CommandEmpty>{emptyState}</CommandEmpty>libs/react/ui/src/components/scroll-area/scroll-area.tsx (1)
5-26: Consider allowing className customization on Viewport.The
classNameprop is applied toRootbut not toViewport. If consumers need to customize viewport styling (padding, dimensions), they currently cannot.♻️ Optional: Add viewportClassName prop
function ScrollArea({ className, children, + viewportClassName, ...props -}: ComponentProps<typeof ScrollAreaPrimitive.Root>) { +}: ComponentProps<typeof ScrollAreaPrimitive.Root> & { viewportClassName?: string }) { return ( <ScrollAreaPrimitive.Root data-slot="scroll-area" className={cn('relative overflow-hidden', className)} {...props} > <ScrollAreaPrimitive.Viewport data-slot="scroll-area-viewport" - className="size-full rounded-[inherit] transition-[color,box-shadow] outline-none focus-visible:ring-3 focus-visible:ring-border-interactive-with-active/50 focus-visible:outline-1" + className={cn("size-full rounded-[inherit] transition-[color,box-shadow] outline-none focus-visible:ring-3 focus-visible:ring-border-interactive-with-active/50 focus-visible:outline-1", viewportClassName)} >
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.changeset/deep-glasses-play.mdlibs/react/ui/src/components/combobox/combobox.stories.tsxlibs/react/ui/src/components/combobox/combobox.tsxlibs/react/ui/src/components/combobox/index.tslibs/react/ui/src/components/command/command.tsxlibs/react/ui/src/components/index.tslibs/react/ui/src/components/scroll-area/index.tslibs/react/ui/src/components/scroll-area/scroll-area.tsx
🧰 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/scroll-area/scroll-area.tsxlibs/react/ui/src/components/combobox/index.tslibs/react/ui/src/components/scroll-area/index.tslibs/react/ui/src/components/combobox/combobox.tsxlibs/react/ui/src/components/index.tslibs/react/ui/src/components/combobox/combobox.stories.tsxlibs/react/ui/src/components/command/command.tsx
🧬 Code graph analysis (3)
libs/react/ui/src/components/combobox/combobox.tsx (4)
libs/react/ui/src/components/command/command.tsx (8)
CommandTriggerProps(279-279)CommandTrigger(267-267)Command(266-266)CommandInput(269-269)CommandList(270-270)CommandEmpty(271-271)CommandGroup(272-272)CommandItem(273-273)libs/react/ui/src/components/popover/popover.tsx (3)
Popover(60-60)PopoverTrigger(60-60)PopoverContent(60-60)libs/react/ui/src/components/scroll-area/scroll-area.tsx (1)
ScrollArea(53-53)libs/react/ui/src/components/icon/icon.tsx (1)
Icon(86-90)
libs/react/ui/src/components/combobox/combobox.stories.tsx (2)
libs/react/ui/src/components/combobox/combobox.tsx (2)
ComboboxOption(17-20)Combobox(36-124)libs/react/ui/src/components/label/label.tsx (1)
Label(13-15)
libs/react/ui/src/components/command/command.tsx (1)
libs/react/ui/src/components/icon/icon.tsx (1)
Icon(86-90)
⏰ 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 (12)
.changeset/deep-glasses-play.md (1)
1-5: LGTM!The changeset is correctly formatted and appropriately uses a minor version bump for adding new non-breaking features.
libs/react/ui/src/components/command/command.tsx (3)
36-43: LGTM!The
isLoadingprop addition toCommandTriggerPropsis cleanly typed and properly destructured. This enables downstream components like Combobox to leverage loading state display.
189-201: LGTM!Explicitly destructuring and rendering
childrenensures custom empty state content is properly displayed. This aligns with the Combobox usage pattern.
60-67: The "spinner" and "expandUpDownLine" icons are properly defined and valid.The conditional rendering correctly toggles between the spinner icon during loading and the expand icon otherwise. Both icon names are properly mapped in the icons registry and resolve correctly.
libs/react/ui/src/components/combobox/combobox.tsx (3)
52-66: LGTM!The controlled/uncontrolled value pattern is correctly implemented, matching the established pattern in
CommandInput.
98-116: Verify search filtering behavior with thevalueprop.The
CommandItemusesoption.valuefor thevalueprop, which cmdk uses for filtering. Ifoption.valuediffers fromoption.label(e.g., value is an ID), users may not find items when typing the visible label text.Consider using
option.labelor a combined value for better search UX:♻️ Alternative approach using label for searchability
<CommandItem key={option.value} - value={option.value} + value={option.label} onSelect={(selectedValue) => { - handleValueChange(currentValue === selectedValue ? '' : selectedValue); + const selected = options.find((o) => o.label === selectedValue); + if (selected) { + handleValueChange(currentValue === selected.value ? '' : selected.value); + } setOpen(false); }} >
70-91: LGTM!The Popover composition with event propagation handling is appropriate for preventing scroll leakage to parent containers.
libs/react/ui/src/components/index.ts (2)
10-10: LGTM!The combobox export is correctly placed in alphabetical order.
32-32: LGTM!The scroll-area export maintains alphabetical ordering.
libs/react/ui/src/components/scroll-area/index.ts (1)
1-1: LGTM!Standard barrel export pattern following project conventions.
libs/react/ui/src/components/combobox/index.ts (1)
1-1: LGTM!Standard barrel export pattern following project conventions.
libs/react/ui/src/components/scroll-area/scroll-area.tsx (1)
28-53: LGTM!The
ScrollBarcomponent cleanly handles orientation variants with appropriate styling for both vertical and horizontal scrollbars.
There was a problem hiding this comment.
Pull request overview
This PR adds two new UI components to the React UI library: a Combobox component and a ScrollArea component. The Combobox provides a searchable dropdown selection interface built on top of the Command component, while the ScrollArea offers a styled scrollable container using Radix UI primitives.
Key changes:
- New ScrollArea component with customizable scrollbar styling
- New Combobox component with search, loading states, and controlled/uncontrolled modes
- Enhanced Command component with loading state support and icon updates
- Comprehensive Storybook stories demonstrating various component states
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
libs/react/ui/src/components/scroll-area/scroll-area.tsx |
Implements ScrollArea and ScrollBar components using @radix-ui/react-scroll-area primitives |
libs/react/ui/src/components/scroll-area/index.ts |
Exports ScrollArea components |
libs/react/ui/src/components/combobox/combobox.tsx |
Implements Combobox with search, filtering, and state management |
libs/react/ui/src/components/combobox/index.ts |
Exports Combobox component |
libs/react/ui/src/components/combobox/combobox.stories.tsx |
Provides Storybook stories for Default, Empty, Loading, and Disabled states |
libs/react/ui/src/components/command/command.tsx |
Adds isLoading prop to CommandTrigger, updates icon from arrowDownSLine to expandUpDownLine, and adds children support to CommandEmpty |
libs/react/ui/src/components/index.ts |
Exports new combobox and scroll-area components |
.changeset/deep-glasses-play.md |
Documents the minor version bump for adding new components |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| > | ||
| <ScrollAreaPrimitive.Viewport | ||
| data-slot="scroll-area-viewport" | ||
| className="size-full rounded-[inherit] transition-[color,box-shadow] outline-none focus-visible:ring-2 focus-visible:ring-border-interactive-with-active/50 focus-visible:outline-1" |
There was a problem hiding this comment.
I feel like this is a lot of design for what should be a utility component.
Should this design not be hoisted up to the ComboBox component and passed down to the ScrollArea ?
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.