feat(ui): add DropdownInput component and update components#288
feat(ui): add DropdownInput component and update components#288
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 PR updates the React UI library by adding peer dependency for React Query, introducing a new DropdownInput component with remote data fetching, enhancing existing components (Calendar, DatePicker, DateTimeRangePicker, Input, KpiCard) with new features and styling refinements, and expanding Storybook documentation with interaction examples. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant DI as DropdownInput
participant Debounce as Debounce<br/>(memoized)
participant RQ as useQuery<br/>(React Query)
participant QFn as queryFn
participant UI as Dropdown Menu
User->>DI: Type in input
DI->>DI: Update inputValue state
DI->>Debounce: Trigger debounce
par Debounce Processing
Debounce->>Debounce: Wait debounceMs
Debounce->>DI: Update debouncedQuery
end
DI->>DI: Check minQueryLength met?
alt Query length valid
DI->>RQ: Trigger query (debouncedQuery)
RQ->>QFn: Call queryFn(debouncedQuery)
QFn-->>RQ: Return DropdownInputItem[]
RQ-->>DI: isPending=false, data=items
end
DI->>UI: Render dropdown with items
UI-->>User: Display filtered suggestions
User->>UI: Click item or press Enter
DI->>DI: Update selected item
DI->>DI: Emit onSelect & onValueChange
DI->>UI: Close popover (if closeOnSelect)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a new DropdownInput component for async searchable dropdowns and enhances several existing UI components with improved functionality and styling. The changes add React Query as a peer dependency to support the new component's data fetching capabilities.
- Adds a new
DropdownInputcomponent with keyboard navigation, debounced search, and async data fetching - Enhances the
Inputcomponent to support left and right icons with forwardRef implementation - Improves calendar components with better styling, weekday formatting, and max range days constraint
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Updates react-hook-form to 7.70.0 and adds @tanstack/react-query 5.90.16 with corresponding lock file changes |
| libs/react/ui/package.json | Adds @tanstack/react-query as peer dependency and updates react-hook-form version requirement |
| libs/react/ui/src/components/input/input.tsx | Refactors Input to use forwardRef and adds support for left/right icons with wrapper div and dynamic padding |
| libs/react/ui/src/components/input/input.stories.tsx | Adds new WithIcons story demonstrating icon usage in Input component |
| libs/react/ui/src/components/index.ts | Exports the new dropdown-input component |
| libs/react/ui/src/components/dropdown-input/index.ts | Barrel export for dropdown-input component |
| libs/react/ui/src/components/dropdown-input/dropdown-input.tsx | Implements new DropdownInput component with React Query integration, keyboard navigation, and async search |
| libs/react/ui/src/components/dropdown-input/dropdown-input.stories.tsx | Adds comprehensive story with modal integration demonstrating DropdownInput usage |
| libs/react/ui/src/components/date-time-range-picker/date-time-range-picker.tsx | Adds maxRangeDays prop to limit date range selection and improves date handling logic |
| libs/react/ui/src/components/date-picker/date-picker.tsx | Adds custom weekday name formatter to show abbreviated weekday names |
| libs/react/ui/src/components/calendar/calendar.tsx | Updates styling for improved visual design, including borders, spacing, and icon changes |
| libs/react/ui/src/components/dashboard/components/kpi-card.tsx | Enhances value rendering to support ReactNode in addition to string and number types |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
libs/react/ui/src/components/date-time-range-picker/date-time-range-picker.tsx
Show resolved
Hide resolved
![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 (4)
libs/react/ui/src/components/dashboard/components/kpi-card.tsx (1)
42-48: Consider whether text styling should apply to ReactNode wrapper.When rendering ReactNode values (line 47), the wrapper div applies text-specific classes (
text-sm font-medium text-foreground-neutral-base). If the ReactNode is a custom component with its own styling, these classes might conflict or be redundant.Consider whether the wrapper should remain unstyled or have minimal layout-only classes, allowing the ReactNode to fully control its own appearance.
Alternative approach without wrapper styling
- ) : ( - <div className="text-sm font-medium text-foreground-neutral-base">{value}</div> + ) : ( + <div>{value}</div>Or apply only layout classes if needed:
- ) : ( - <div className="text-sm font-medium text-foreground-neutral-base">{value}</div> + ) : ( + <div className="flex items-center">{value}</div>libs/react/ui/src/components/dropdown-input/dropdown-input.tsx (2)
256-258: Hardcoded label should be configurable.The "Select a repository" text is hardcoded, making this component less reusable for other use cases. Consider adding a prop for customization:
export type DropdownInputProps<T = unknown> = Omit<...> & { ... emptyPlaceholder?: string; + resultsLabel?: string; ... };-<div className="px-8 py-6 text-xs leading-20 text-foreground-neutral-muted"> - Select a repository -</div> +{resultsLabel && ( + <div className="px-8 py-6 text-xs leading-20 text-foreground-neutral-muted"> + {resultsLabel} + </div> +)}
86-100: Consider cleanup for debounced function on unmount.The debounced function may fire after the component unmounts. While React's
useStatesetters are safe to call post-unmount, adding cleanup would be cleaner:+// In utils/debounce.ts, return a cancel function +export function debounce<T extends (...args: never[]) => void>(fn: T, delay: number) { + let timeoutId: ReturnType<typeof setTimeout> | null = null; + const debounced = (...args: Parameters<T>) => { ... }; + debounced.cancel = () => { if (timeoutId) clearTimeout(timeoutId); }; + return debounced; +}Then add cleanup:
useEffect(() => { + return () => debouncedSetQuery.cancel?.(); -}, [value, minQueryLength, debouncedSetQuery]); +}, [value, minQueryLength, debouncedSetQuery]);libs/react/ui/src/components/dropdown-input/dropdown-input.stories.tsx (1)
133-133: Consider linking the modal description for better accessibility.The
aria-describedby={undefined}is explicitly set, which prevents warnings but misses an opportunity to link the descriptive text on lines 137-140 to the dialog. Consider adding anidto the Text component and referencing it here.Optional improvement for accessibility
- <ModalContent aria-describedby={undefined} overlayClassName="bg-background-modal-overlay"> + <ModalContent aria-describedby="import-description" overlayClassName="bg-background-modal-overlay"> <ModalTitle className="sr-only">Import past jobs from GitHub</ModalTitle> <ModalHeader title="Import past jobs from GitHub" /> <ModalBody className="gap-20"> - <Text size="sm" className="text-foreground-neutral-subtle w-full"> + <Text id="import-description" size="sm" className="text-foreground-neutral-subtle w-full"> Backfill your CI history by importing past runs from your GitHub repo. We'll handle the rest by creating a background task to import the data for you. </Text>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
libs/react/ui/package.jsonlibs/react/ui/src/components/calendar/calendar.tsxlibs/react/ui/src/components/dashboard/components/kpi-card.tsxlibs/react/ui/src/components/date-picker/date-picker.tsxlibs/react/ui/src/components/date-time-range-picker/date-time-range-picker.tsxlibs/react/ui/src/components/dropdown-input/dropdown-input.stories.tsxlibs/react/ui/src/components/dropdown-input/dropdown-input.tsxlibs/react/ui/src/components/dropdown-input/index.tslibs/react/ui/src/components/index.tslibs/react/ui/src/components/input/input.stories.tsxlibs/react/ui/src/components/input/input.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/index.tslibs/react/ui/package.jsonlibs/react/ui/src/components/dropdown-input/index.tslibs/react/ui/src/components/input/input.stories.tsxlibs/react/ui/src/components/date-picker/date-picker.tsxlibs/react/ui/src/components/dropdown-input/dropdown-input.stories.tsxlibs/react/ui/src/components/input/input.tsxlibs/react/ui/src/components/calendar/calendar.tsxlibs/react/ui/src/components/dropdown-input/dropdown-input.tsxlibs/react/ui/src/components/date-time-range-picker/date-time-range-picker.tsxlibs/react/ui/src/components/dashboard/components/kpi-card.tsx
🧬 Code graph analysis (4)
libs/react/ui/src/components/input/input.stories.tsx (2)
libs/react/ui/src/components/input/input.tsx (1)
Input(29-65)libs/react/ui/src/components/icon/icon.tsx (1)
Icon(86-90)
libs/react/ui/src/components/date-picker/date-picker.tsx (1)
libs/react/ui/src/components/calendar/calendar.tsx (1)
Calendar(8-90)
libs/react/ui/src/components/dropdown-input/dropdown-input.tsx (3)
libs/react/ui/src/components/input/input.tsx (2)
inputVariants(6-21)Input(29-65)libs/react/ui/src/utils/debounce.ts (1)
debounce(1-15)libs/react/ui/src/components/icon/icon.tsx (1)
Icon(86-90)
libs/react/ui/src/components/dashboard/components/kpi-card.tsx (1)
libs/react/ui/src/components/typography/text.tsx (1)
Text(27-50)
⏰ 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 (17)
libs/react/ui/src/components/date-picker/date-picker.tsx (1)
172-179: LGTM!The
formatWeekdayNameformatter using'EEEEE'for narrow weekday names (single letter) is consistent with the same pattern applied inDateTimeRangePicker. This creates a unified weekday display across date-related components.libs/react/ui/src/components/input/input.tsx (1)
29-65: LGTM!The refactoring to
forwardRefwith icon support is well-implemented:
- The wrapper div maintains
w-fullfor consistent layout- Icon containers use
pointer-events-noneensuring input remains interactive- Conditional padding (
pl-32/pr-32) prevents text-icon overlapNote: The ref is correctly forwarded to the actual
<input>element, so consumers calling methods likefocus()will work as expected.libs/react/ui/src/components/input/input.stories.tsx (1)
141-177: LGTM!The
WithIconsstory provides good coverage of the new icon props, demonstrating left-only, right-only, and both icons configurations. This will help document the feature for consumers.libs/react/ui/src/components/date-time-range-picker/date-time-range-picker.tsx (2)
99-107: VerifymaxRangeDayssemantics: days from start vs range length.The current logic (
daysFromStart > maxRangeDays) allows selecting dates wheredifferenceInDaysequalsmaxRangeDays. This meansmaxRangeDays=7permits a range of 8 calendar days (start date + 7 more).If the intent is to limit the range to exactly
maxRangeDayscalendar days, consider:return (date: Date) => { const daysFromStart = differenceInDays(date, startDate); - return daysFromStart > maxRangeDays; + return daysFromStart >= maxRangeDays; };Otherwise, renaming to
maxDaysFromStartwould clarify the current behavior.
227-238: LGTM!The Calendar integration with
defaultMonth,disabled(formaxRangeDays), andformattersis well-structured. TheformatWeekdayNameformatter is consistent with theDatePickercomponent.libs/react/ui/src/components/calendar/calendar.tsx (2)
20-20: Verifyfixedpositioning on navigation is intentional.The
navclass usesfixed left-0 top-16 w-full z-10. Fixed positioning is relative to the viewport, not the Calendar container. If the Calendar is rendered inside a scrollable popover or modal, the nav buttons may appear misaligned.If the intent is to position relative to the calendar, consider using
absoluteinstead:-nav: 'flex items-center gap-4 fixed left-0 top-16 w-full z-10', +nav: 'flex items-center gap-4 absolute left-0 top-16 w-full z-10',
68-72: LGTM!The "today" indicator redesign using a subtle dot pseudo-element is a nice visual refinement. The positioning with
after:bottom-[6px]and centered translation creates an unobtrusive but clear indicator.libs/react/ui/src/components/dropdown-input/dropdown-input.tsx (3)
158-181: LGTM!The keyboard navigation implementation is solid:
- Arrow keys with wrap-around for circular navigation
- Enter only triggers selection when an item is focused
- Escape properly closes and blurs the input
preventDefaultcorrectly used to prevent default browser behavior
183-202: LGTM!The blur handling correctly uses a timeout pattern to allow dropdown item clicks to complete before potentially closing the popover. The cleanup effect at lines 196-202 properly clears pending timeouts on unmount.
220-295: LGTM!The render implementation is well-structured:
onMouseDownwithpreventDefaultcorrectly prevents the input blur before click handlers fireonMouseEntersyncs keyboard and mouse focus statesscrollIntoViewwithblock: 'nearest'(line 206) keeps focused items visible- Button elements with proper
type="button"prevent form submissionlibs/react/ui/package.json (1)
63-63: @tanstack/react-query ^5.90.5 is a valid peer dependency.The DropdownInput component imports and uses
useQueryfrom @tanstack/react-query, justifying the peer dependency addition. Version 5.90.5 exists on npm and has no known security vulnerabilities.libs/react/ui/src/components/index.ts (1)
17-17: LGTM! Export addition follows the established pattern.The new dropdown-input export is correctly placed in alphabetical order and follows the existing barrel export pattern used throughout the file.
libs/react/ui/src/components/dropdown-input/index.ts (1)
1-1: LGTM! Standard barrel export pattern.The barrel export follows the established pattern used throughout the component library, enabling clean imports for the new DropdownInput component.
libs/react/ui/src/components/dropdown-input/dropdown-input.stories.tsx (4)
1-54: LGTM! Well-structured story setup with proper mock data.The imports, constants, mock data, and async fetch helpers are well-organized. The fetch functions appropriately simulate network delay and filtering logic for demonstration purposes.
56-84: LGTM! Proper QueryClient configuration for Storybook.The decorator correctly sets up a QueryClientProvider with appropriate options for Storybook (disabled retries and refetch on focus). The QueryClient is properly memoized to avoid unnecessary recreations.
89-115: LGTM! Well-structured play function for automated testing.The play function appropriately orchestrates the test interaction, opening the modal and waiting for the UI to settle before capturing the screenshot for visual regression testing.
116-211: LGTM! Comprehensive demonstration of the DropdownInput component.The render function provides an excellent real-world example integrating DropdownInput with other components. The state management correctly handles:
- Input value changes
- Selection state synchronization (clearing selection when value is manually edited)
- Integration with DateTimeRangePicker
The modal structure effectively demonstrates the component in a realistic use case.
|
No need to change it this time, but this PR should have been 3 PRs. There is no specific reason here to make the updates to Input, DatePicker and the creation of the DoropDown to be done in a single change. |
…put and update related components
…of 5 and adjust related logic
…h results and enhance empty state messaging
…siveness and add disabled state handling
…us and change events
…update related logic
| onFocusedIndexChange: (index: number) => void; | ||
| selectedItem?: DropdownInputItem<T> | null; | ||
| dropdownClassName?: string; | ||
| contactSupportHref?: string; |
There was a problem hiding this comment.
I guess that this is a bit too specific to be in there
There was a problem hiding this comment.
Looks like it's not really used neither
2 participants
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.