New components #1007
New components #1007
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces a variety of new React components and modifies existing ones to enhance the UI component library. Key changes include restructuring import paths, creating customizable components for alerts, avatars, badges, checkboxes, and more, while also implementing responsive layouts such as sidebars and navigation bars. Additionally, modifications to the Husky pre-commit hook script disable linting checks during commits, potentially impacting code quality assurance. Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Outside diff range, codebase verification and nitpick comments (2)
components/Catalyst/sidebar-layout.tsx (1)
51-92: Missing CSS Class Definitions in SidebarLayout ComponentThe following CSS classes used in the
SidebarLayoutcomponent are not found in the codebase:isolate,min-h-svh,max-lg:flex-col,lg:bg-zinc-100, anddark:lg:bg-zinc-950. Please verify their definitions or add them to ensure the component is styled correctly.
- Missing Classes:
isolate,min-h-svh,max-lg:flex-col,lg:bg-zinc-100,dark:lg:bg-zinc-950Analysis chain
Component Structure and State Management are Correct
The
SidebarLayoutcomponent is well-structured with appropriate use of React state for managing sidebar visibility. Verify the CSS classes for correctness and ensure they are defined properly in the project's stylesheets.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify CSS classes in SidebarLayout component. # Test: Search for CSS classes related to layout. Expect: Proper usage and definitions in CSS files. rg --type css "relative|isolate|flex|min-h-svh|bg-white|dark:bg-zinc-900|max-lg:flex-col|lg:bg-zinc-100|dark:lg:bg-zinc-950"Length of output: 2230
Script:
#!/bin/bash # Description: Search for missing CSS classes in the entire codebase. # Test: Search for missing CSS classes. Expect: Definitions in CSS or JS files. rg --type css --type js "relative|isolate|min-h-svh|max-lg:flex-col|lg:bg-zinc-100|dark:lg:bg-zinc-950"Length of output: 190
components/Catalyst/sidebar.tsx (1)
112-198: Ensure ARIA roles and attributes for accessibility in Sidebar componentsThe
SidebarLabeland related components incomponents/Catalyst/sidebar.tsxcurrently lack comprehensive ARIA roles and attributes, which are essential for accessibility. Consider reviewing these components to ensure that appropriate ARIA roles and attributes are applied, especially for interactive elements. This will enhance accessibility for users relying on assistive technologies.
- Ensure ARIA roles and attributes are consistently applied in
SidebarLabeland related components.Analysis chain
Ensure accessibility and consistency in SidebarLabel and related components.
While the
SidebarLabeland related components are implemented effectively, ensure that accessibility considerations such as ARIA roles and attributes are consistently applied, especially in components that involve interactive elements or complex layouts.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent accessibility attributes in Sidebar components. # Test: Search for ARIA roles and attributes in Sidebar components. rg --type typescript -A 5 $'aria-|role'Length of output: 75
Script:
#!/bin/bash # Description: Check for consistent accessibility attributes in Sidebar components. # Test: Search for ARIA roles and attributes in .tsx files. rg --glob '*.tsx' -A 5 'aria-|role'Length of output: 48191
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (19)
- .husky/pre-commit (1 hunks)
- app/(app)/alpha/additional-details/_client.tsx (1 hunks)
- components/Catalyst/alert.tsx (1 hunks)
- components/Catalyst/avatar.tsx (1 hunks)
- components/Catalyst/badge.tsx (1 hunks)
- components/Catalyst/checkbox.tsx (1 hunks)
- components/Catalyst/description-list.tsx (1 hunks)
- components/Catalyst/dialog.tsx (1 hunks)
- components/Catalyst/dropdown.tsx (1 hunks)
- components/Catalyst/listbox.tsx (1 hunks)
- components/Catalyst/navbar.tsx (1 hunks)
- components/Catalyst/pagination.tsx (1 hunks)
- components/Catalyst/radio.tsx (1 hunks)
- components/Catalyst/sidebar-layout.tsx (1 hunks)
- components/Catalyst/sidebar.tsx (1 hunks)
- components/Catalyst/stacked-layout.tsx (1 hunks)
- components/Catalyst/switch.tsx (1 hunks)
- components/Catalyst/table.tsx (1 hunks)
- components/Catalyst/textarea.tsx (1 hunks)
Files skipped from review due to trivial changes (2)
- .husky/pre-commit
- app/(app)/alpha/additional-details/_client.tsx
Additional comments not posted (51)
components/Catalyst/description-list.tsx (3)
3-12: Approved: Well-implemented DescriptionList component.The
DescriptionListcomponent is correctly implemented with appropriate use ofclsxfor class name binding and props spreading on the<dl>element.
15-25: Approved: Consistent and correct implementation of DescriptionTerm.The
DescriptionTermcomponent follows the same pattern asDescriptionList, correctly applying clsx for dynamic class name management and props spreading on the<dt>element.
27-37: Approved: Correct implementation of DescriptionDetails.The
DescriptionDetailscomponent is implemented consistently with the other components in this file, usingclsxeffectively for class name management and props spreading on the<dd>element.components/Catalyst/textarea.tsx (1)
5-54: Approved: Advanced and flexible Textarea component implementation.The
Textareacomponent is well-implemented, utilizingforwardReffor ref forwarding andclsxfor extensive conditional styling. The integration with Headless UI'sTextareaand the handling of various UI states are commendable.components/Catalyst/avatar.tsx (2)
15-51: Approved: Well-implemented Avatar component with good accessibility practices.The
Avatarcomponent is flexible, supporting both text and image representations. It uses conditional styling effectively and includes accessibility features likearia-hiddenfor decorative elements. The implementation is robust and follows best practices.
53-84: Approved: Flexible AvatarButton component implementation.The
AvatarButtoncomponent is well-designed, capable of rendering as either a Link or a Button based on the props. This flexibility, combined with the correct use offorwardRefand conditional styling, makes it a robust component suitable for various use cases.components/Catalyst/alert.tsx (2)
81-95: Enhance the AlertActions for better mobile responsiveness.The
AlertActionscomponent uses flex properties that might not render well on all mobile devices. Consider testing this component on various devices to ensure that the layout remains consistent and functional.Verification successful
Recommendation to Test AlertActions Component on Various Devices
The
AlertActionscomponent uses responsive design techniques, such as changing flex direction and element widths based on screen size. It is advisable to test this component on various devices to ensure that the layout remains consistent and functional across different screen sizes. This will help verify the effectiveness of the responsive design.
- File:
components/Catalyst/alert.tsx- Lines: 81-95
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the mobile responsiveness of the AlertActions component. # Test: Search for flex properties and test on mobile devices. rg --type typescript -A 5 $'AlertActions'Length of output: 75
Script:
#!/bin/bash # Use ast-grep to find the AlertActions function definition and inspect its implementation. ast-grep --lang typescript --pattern $'export function AlertActions({ className, ...props }: React.ComponentPropsWithoutRef<\'div\'>) { $$$ }' --jsonLength of output: 159
Script:
#!/bin/bash # Use rg to search for the AlertActions function by name across the codebase. rg 'function AlertActions' -A 5Length of output: 387
Script:
#!/bin/bash # Use rg to search for the complete implementation of the AlertActions function to inspect all applied classes. rg 'export function AlertActions' -A 10 components/Catalyst/alert.tsxLength of output: 376
18-51: Ensure accessibility and responsiveness in the Alert component.The
Alertcomponent is well-structured with appropriate use of Headless UI components for accessibility. However, ensure that the responsive and accessibility features are tested across different devices and screen readers to maintain a high level of usability.components/Catalyst/dialog.tsx (3)
18-51: Review the Dialog component for modularity and reusability.The
Dialogcomponent is modular and uses Headless UI effectively. Ensure that it is reusable in various parts of the application without modifications.
88-107: Ensure DialogActions are flexible and responsive.The
DialogActionsshould be tested for responsiveness, especially in flex layouts that might behave differently on various screen sizes.Verification successful
DialogActions Component Appears Responsive
The
DialogActionscomponent uses responsive utility classes, such assm:flex-rowandsm:*:w-auto, which suggest it is designed to adapt to different screen sizes. These classes indicate a default column layout that switches to a row layout on small screens, addressing the review comment's concern about responsiveness.
components/Catalyst/dialog.tsx: Lines 88-107Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the responsiveness of the DialogActions component. # Test: Search for flex properties and test on various devices. rg --type typescript -A 5 $'DialogActions'Length of output: 76
Script:
#!/bin/bash # Description: Verify the responsiveness of the DialogActions component. # Corrected Test: Search for the DialogActions component in .tsx files and include additional context. rg --type-add 'tsx:*.tsx' --type tsx -A 10 'DialogActions'Length of output: 689
6-16: Consistency check: Ensure size definitions match across components.The size definitions in the
Dialogcomponent should be consistent with those in theAlertcomponent to maintain uniformity across the UI library.components/Catalyst/stacked-layout.tsx (2)
51-89: Review the StackedLayout for overall responsiveness and modularity.The
StackedLayoutis designed to be modular and responsive. Ensure that it functions correctly in different layouts and screen sizes.
23-49: Ensure MobileSidebar is accessible and responsive.The
MobileSidebaruses Headless UI components for accessibility, but ensure that it is fully accessible and responsive by testing on various devices and with screen readers.Verification successful
MobileSidebar is accessible and responsive.
The
MobileSidebarcomponent includes ARIA attributes for accessibility and uses responsive design techniques such as thelg:hiddenclass to adapt to different screen sizes. These elements ensure that the component is both accessible and responsive.
- ARIA attributes:
aria-hidden,aria-label- Responsive design:
lg:hiddenclassScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the accessibility and responsiveness of the MobileSidebar component. # Test: Search for accessibility attributes and test on various devices. rg --type typescript -A 5 $'MobileSidebar'Length of output: 76
Script:
#!/bin/bash # Corrected command to search for the MobileSidebar component in TypeScript files. rg --type ts -A 10 'MobileSidebar' components/Catalyst/stacked-layout.tsxLength of output: 926
Script:
#!/bin/bash # Search for ARIA attributes and responsive design elements in the MobileSidebar component. rg --type ts 'aria-' components/Catalyst/stacked-layout.tsx # Check for media queries or responsive CSS classes in the MobileSidebar component. rg --type ts 'className=".*lg:hidden.*"' components/Catalyst/stacked-layout.tsxLength of output: 510
components/Catalyst/sidebar-layout.tsx (3)
7-13: SVG Icon Implementation is CorrectThe
OpenMenuIconcomponent is implemented correctly with appropriate accessibility attributes.
15-21: SVG Icon Implementation is CorrectThe
CloseMenuIconcomponent is implemented correctly with appropriate accessibility attributes.
23-49: Verify Mobile Sidebar FunctionalityThe
MobileSidebarcomponent usesHeadless.Dialogappropriately for modal behavior. Ensure that the responsiveness and transitions are tested across different devices and screen sizes.Verification successful
Verified Use of Tailwind CSS Utility Classes
The
MobileSidebarcomponent utilizes Tailwind CSS utility classes for responsiveness and transitions. The presence oftailwind.config.jsconfirms the use of Tailwind CSS in the project, which explains the utility classes applied directly in the JSX. No further action is required regarding the CSS classes in this component.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify responsiveness and transitions in MobileSidebar component. # Test: Search for CSS classes related to responsiveness and transitions. Expect: Proper usage and definitions in CSS files. rg --type css "fixed|inset-0|bg-black/30|transition|duration-300|ease-in-out"Length of output: 644
Script:
#!/bin/bash # Check for Tailwind CSS configuration or imports in the project. # Look for Tailwind CSS configuration file. fd tailwind.config.js # Search for Tailwind CSS imports in JavaScript/TypeScript files. rg "import 'tailwindcss/tailwind.css'" --type js --type tsLength of output: 109
components/Catalyst/pagination.tsx (5)
5-17: Proper Use of clsx and Accessibility FeaturesThe
Paginationcomponent correctly usesclsxfor dynamic class application and includes accessibility features such asaria-label.
19-49: Conditional Rendering and Accessibility are Handled WellThe
PaginationPreviouscomponent handles conditional rendering based on thehrefprop effectively, ensuring that the component is accessible and functional.
51-81: Consistent Implementation with Previous ComponentsThe
PaginationNextcomponent is implemented consistently withPaginationPrevious, effectively handling conditional rendering and accessibility.
83-93: Simple and Effective ImplementationThe
PaginationListcomponent usesclsxeffectively for conditional display based on screen size, which is a good practice for responsive design.
95-120: Accessibility and Dynamic Styling are Well-HandledThe
PaginationPagecomponent effectively usesaria-currentto indicate the current page andclsxfor dynamic styling, aligning with accessibility best practices.components/Catalyst/navbar.tsx (6)
10-20: Simple and Effective Navbar ImplementationThe
Navbarcomponent usesclsxeffectively for dynamic class application, ensuring a flexible and maintainable codebase.
22-33: Proper Use of Accessibility AttributesThe
NavbarDividercomponent is implemented correctly with thearia-hiddenattribute, ensuring it is not read by screen readers, which is appropriate for purely decorative elements.
35-46: Effective Use of Animation and Layout ManagementThe
NavbarSectioncomponent usesLayoutGroupeffectively to manage animations and layout changes, enhancing the user experience with smooth transitions.
48-59: Simple and Accessible Spacer ImplementationThe
NavbarSpacercomponent is implemented correctly with thearia-hiddenattribute, ensuring it is not read by screen readers, which is appropriate for spacers.
61-121: Complex Conditional Rendering Handled WellThe
NavbarItemcomponent handles conditional rendering and ref forwarding effectively, making it versatile for different use cases. The detailed accessibility features and dynamic class application are implemented correctly.
123-128: Simple and Effective Label ImplementationThe
NavbarLabelcomponent usesclsxeffectively for dynamic class application, ensuring a flexible and maintainable codebase.components/Catalyst/badge.tsx (4)
1-6: Imports and dependencies are well-organized.The imports are clear and well-organized, which is good for maintainability. The use of
forwardRefis appropriate for components that might need ref forwarding, especially when dealing with UI libraries like@headlessui/react.
7-35: Color mapping object is comprehensive and well-defined.The
colorsobject is extensive and covers a wide range of colors with specific styles for both light and dark themes. This approach ensures that theBadgecomponent can be easily customized while maintaining consistency across different themes.
37-50: Badge component implementation is clean and efficient.The
Badgecomponent is implemented efficiently using functional component syntax and destructuring for props. The use ofclsxfor conditional class application is appropriate and enhances the readability and maintainability of the component.
52-82: Ensure accessibility in conditional rendering within BadgeButton.The
BadgeButtoncomponent handles conditional rendering well, but ensure that accessibility considerations, such as keyboard focus and ARIA attributes, are maintained when switching betweenLinkandHeadless.Button. It might be beneficial to verify that the accessibility features are consistent in both rendered elements.components/Catalyst/table.tsx (5)
1-18: Context setup for table properties is well-implemented.The
TableContextis set up effectively to manage table properties such asbleed,dense,grid, andstriped. This approach allows for flexible configuration of the table's appearance and behavior, which can be easily adjusted through context providers.
20-63: Table component structure is modular and well-organized.The
Tablecomponent and its context provider are well-structured, providing a clear and modular approach to building table UIs. The use ofclsxfor conditional class application based on context values is a good practice, enhancing the component's flexibility and maintainability.
66-80: TableBody component is succinct and functional.The
TableBodycomponent is implemented succinctly, serving as a straightforward container for table rows. This minimalistic approach is appropriate for components that primarily act as structural placeholders.
82-126: TableRow component handles complex conditional styling effectively.The
TableRowcomponent demonstrates an effective use of context to apply conditional styling based on thestripedproperty and the presence of anhref. This implementation ensures that the row's appearance can dynamically respond to its context and properties, which is crucial for interactive table elements.
128-184: TableCell component's dynamic styling and accessibility features are commendable.The
TableCellcomponent's implementation includes dynamic styling based on multiple context values and conditional rendering of a link for navigable cells. The use ofuseStatefor managing a reference to the cell element is a thoughtful addition for managing focus, especially in accessibility contexts.components/Catalyst/sidebar.tsx (3)
1-20: Sidebar component setup is clean and effective.The
Sidebarcomponent is implemented effectively using functional component syntax and props destructuring. The use ofclsxfor conditional class application based on theclassNameprop enhances the component's flexibility and maintainability.
22-50: Sidebar sub-components are well-defined and modular.The
SidebarHeader,SidebarBody, andSidebarFootercomponents are well-defined and modular, each handling specific sections of the sidebar layout. This separation enhances the readability and maintainability of the sidebar component structure.
52-110: SidebarItem and related components demonstrate good use of advanced React patterns.The
SidebarItemcomponent, along withSidebarDividerandSidebarSpacer, demonstrates good use of advanced React patterns such asforwardRefand conditional rendering. The use ofclsxfor dynamic class application and theuseIdhook for managing unique identifiers are particularly commendable.components/Catalyst/dropdown.tsx (5)
9-11: Approved: Simple and effective implementation of the Dropdown component.The function correctly utilizes the Headless UI library components and TypeScript props, ensuring type safety and simplicity.
13-18: Approved: Well-implemented DropdownButton with flexible component type.The use of TypeScript generics here allows for flexible component customization while maintaining type safety. This is a good practice in React component design.
53-90: Approved: Effective use of conditional rendering in DropdownItem.The function smartly handles the rendering of different elements based on props, which is a good practice for reusable UI components. The complex class management is well-handled, though it could benefit from further comments for clarity.
92-102: Approved: Simple and clean implementation of DropdownHeader.The function is straightforward and effectively uses
clsxfor class management, adhering to best practices for simplicity and maintainability.
104-121: Approved: Well-structured DropdownSection for layout management.The function effectively manages layout with conditional classes and maintains consistency with the rest of the components in terms of styling and structure.
components/Catalyst/checkbox.tsx (3)
5-18: Well-implemented CheckboxGroup component.The use of
clsxfor conditional styling and the spread of props are handled well. Ensure that props are validated appropriately to avoid passing invalid HTML attributes to thedivelement.
21-44: CheckboxField component is correctly implemented.The component correctly omits conflicting props and uses
clsxeffectively for dynamic class binding. This is a good practice to ensure that the component remains flexible and reusable.
117-157: Checkbox component is well-designed with effective theming.The use of CSS variables for theming enhances the flexibility and reusability of the component. The structure of the component adheres to best practices, making it maintainable and scalable.
components/Catalyst/switch.tsx (3)
5-20: SwitchGroup component is consistently implemented.The component follows similar patterns as seen in the checkbox components, using
clsxfor dynamic class binding and spreading props. Ensure that props are validated to prevent invalid HTML attributes.
24-45: SwitchField component is correctly implemented.This component effectively uses
clsxfor dynamic class binding and omits potentially conflicting props, which is a good practice for maintaining component isolation and reusability.
142-198: Switch component is well-designed with effective theming.The use of CSS variables for theming enhances the flexibility and reusability of the component. The structure of the component adheres to best practices, making it maintainable and scalable.
components/Catalyst/listbox.tsx (1)
161-177: Approve implementation of utility components.The
ListboxLabelandListboxDescriptioncomponents are implemented appropriately, providing essential functionality to the listbox with a clear and concise code style. The use of clsx for dynamic class names is consistent with the rest of the components.
components/Catalyst/alert.tsx
Outdated
| const sizes = { | ||
| xs: 'sm:max-w-xs', | ||
| sm: 'sm:max-w-sm', | ||
| md: 'sm:max-w-md', | ||
| lg: 'sm:max-w-lg', | ||
| xl: 'sm:max-w-xl', | ||
| '2xl': 'sm:max-w-2xl', | ||
| '3xl': 'sm:max-w-3xl', | ||
| '4xl': 'sm:max-w-4xl', | ||
| '5xl': 'sm:max-w-5xl', | ||
| } |
There was a problem hiding this comment.
Consider using TypeScript enums for size definitions.
The current implementation uses a plain object for size mappings. Using TypeScript enums or a const enum could provide better autocompletion and error checking during development.
Consider refactoring the size definitions:
enum Size {
Xs = 'sm:max-w-xs',
Sm = 'sm:max-w-sm',
Md = 'sm:max-w-md',
Lg = 'sm:max-w-lg',
Xl = 'sm:max-w-xl',
'2Xl' = 'sm:max-w-2xl',
'3Xl' = 'sm:max-w-3xl',
'4Xl' = 'sm:max-w-4xl',
'5Xl' = 'sm:max-w-5xl',
}
components/Catalyst/alert.tsx
Outdated
| export function AlertTitle({ | ||
| className, | ||
| ...props | ||
| }: { className?: string } & Omit<Headless.DialogTitleProps, 'as' | 'className'>) { | ||
| return ( | ||
| <Headless.DialogTitle | ||
| {...props} | ||
| className={clsx( | ||
| className, | ||
| 'text-balance text-center text-base/6 font-semibold text-zinc-950 sm:text-wrap sm:text-left sm:text-sm/6 dark:text-white' | ||
| )} | ||
| /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Review the AlertTitle component for text scaling.
The AlertTitle uses fixed text sizes which might not scale well on all devices. Consider using responsive units like rem or percentages for text sizes to enhance readability across devices.
Propose a change to use responsive text sizes:
className={clsx(
className,
'text-balance text-center text-base font-semibold text-zinc-950 sm:text-wrap sm:text-left sm:text-base dark:text-white'
)}
components/Catalyst/alert.tsx
Outdated
| export function AlertDescription({ | ||
| className, | ||
| ...props | ||
| }: { className?: string } & Omit<Headless.DescriptionProps<typeof Text>, 'as' | 'className'>) { | ||
| return ( | ||
| <Headless.Description | ||
| as={Text} | ||
| {...props} | ||
| className={clsx(className, 'mt-2 text-pretty text-center sm:text-left')} | ||
| /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Optimize AlertDescription for better text alignment and readability.
The AlertDescription component could benefit from better text alignment properties, especially in smaller screens where center alignment might not be ideal.
Suggest a change for better text alignment:
className={clsx(className, 'mt-2 text-pretty text-left sm:text-left')}
components/Catalyst/dialog.tsx
Outdated
| export function DialogTitle({ | ||
| className, | ||
| ...props | ||
| }: { className?: string } & Omit< | ||
| Headless.DialogTitleProps, | ||
| "as" | "className" | ||
| >) { | ||
| return ( | ||
| <Headless.DialogTitle | ||
| {...props} | ||
| className={clsx( | ||
| className, | ||
| "text-balance text-lg/6 font-semibold text-zinc-950 dark:text-white sm:text-base/6", | ||
| )} | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Check the DialogTitle for appropriate text scaling and accessibility.
Similar to the AlertTitle, ensure that the DialogTitle uses responsive text sizes and is accessible.
Suggest using responsive text sizes for better accessibility:
className={clsx(
className,
'text-balance text-lg font-semibold text-zinc-950 dark:text-white sm:text-base'
)}
components/Catalyst/dialog.tsx
Outdated
| export function DialogDescription({ | ||
| className, | ||
| ...props | ||
| }: { className?: string } & Omit< | ||
| Headless.DescriptionProps<typeof Text>, | ||
| "as" | "className" | ||
| >) { | ||
| return ( | ||
| <Headless.Description | ||
| as={Text} | ||
| {...props} | ||
| className={clsx(className, "mt-2 text-pretty")} | ||
| /> | ||
| ); |
There was a problem hiding this comment.
Optimize DialogDescription for better text readability.
The DialogDescription should use more flexible text alignment properties to improve readability on various devices.
Suggest changes for better text alignment:
className={clsx(className, 'mt-2 text-pretty text-left')}
components/Catalyst/listbox.tsx
Outdated
| export function Listbox<T>({ | ||
| className, | ||
| placeholder, | ||
| autoFocus, | ||
| 'aria-label': ariaLabel, | ||
| children: options, | ||
| ...props | ||
| }: { | ||
| className?: string | ||
| placeholder?: React.ReactNode | ||
| autoFocus?: boolean | ||
| 'aria-label'?: string | ||
| children?: React.ReactNode | ||
| } & Omit<Headless.ListboxProps<typeof Fragment, T>, 'as' | 'multiple'>) { | ||
| return ( | ||
| <Headless.Listbox {...props} multiple={false}> | ||
| <Headless.ListboxButton | ||
| autoFocus={autoFocus} | ||
| data-slot="control" | ||
| aria-label={ariaLabel} | ||
| className={clsx([ | ||
| className, | ||
| // Basic layout | ||
| 'group relative block w-full', | ||
| // Background color + shadow applied to inset pseudo element, so shadow blends with border in light mode | ||
| 'before:absolute before:inset-px before:rounded-[calc(theme(borderRadius.lg)-1px)] before:bg-white before:shadow', | ||
| // Background color is moved to control and shadow is removed in dark mode so hide `before` pseudo | ||
| 'dark:before:hidden', | ||
| // Hide default focus styles | ||
| 'focus:outline-none', | ||
| // Focus ring | ||
| 'after:pointer-events-none after:absolute after:inset-0 after:rounded-lg after:ring-inset after:ring-transparent after:data-[focus]:ring-2 after:data-[focus]:ring-blue-500', | ||
| // Disabled state | ||
| 'data-[disabled]:opacity-50 before:data-[disabled]:bg-zinc-950/5 before:data-[disabled]:shadow-none', | ||
| ])} | ||
| > | ||
| <Headless.ListboxSelectedOption | ||
| as="span" | ||
| options={options} | ||
| placeholder={placeholder && <span className="block truncate text-zinc-500">{placeholder}</span>} | ||
| className={clsx([ | ||
| // Basic layout | ||
| 'relative block w-full appearance-none rounded-lg py-[calc(theme(spacing[2.5])-1px)] sm:py-[calc(theme(spacing[1.5])-1px)]', | ||
| // Set minimum height for when no value is selected | ||
| 'min-h-11 sm:min-h-9', | ||
| // Horizontal padding | ||
| 'pl-[calc(theme(spacing[3.5])-1px)] pr-[calc(theme(spacing.7)-1px)] sm:pl-[calc(theme(spacing.3)-1px)]', | ||
| // Typography | ||
| 'text-left text-base/6 text-zinc-950 placeholder:text-zinc-500 sm:text-sm/6 dark:text-white forced-colors:text-[CanvasText]', | ||
| // Border | ||
| 'border border-zinc-950/10 group-data-[active]:border-zinc-950/20 group-data-[hover]:border-zinc-950/20 dark:border-white/10 dark:group-data-[active]:border-white/20 dark:group-data-[hover]:border-white/20', | ||
| // Background color | ||
| 'bg-transparent dark:bg-white/5', | ||
| // Invalid state | ||
| 'group-data-[invalid]:border-red-500 group-data-[invalid]:group-data-[hover]:border-red-500 group-data-[invalid]:dark:border-red-600 group-data-[invalid]:data-[hover]:dark:border-red-600', | ||
| // Disabled state | ||
| 'group-data-[disabled]:border-zinc-950/20 group-data-[disabled]:opacity-100 group-data-[disabled]:dark:border-white/15 group-data-[disabled]:dark:bg-white/[2.5%] dark:data-[hover]:group-data-[disabled]:border-white/15', | ||
| ])} | ||
| /> | ||
| <span className="pointer-events-none absolute inset-y-0 right-0 flex items-center pr-2"> | ||
| <svg | ||
| className="size-5 stroke-zinc-500 group-data-[disabled]:stroke-zinc-600 sm:size-4 dark:stroke-zinc-400 forced-colors:stroke-[CanvasText]" | ||
| viewBox="0 0 16 16" | ||
| aria-hidden="true" | ||
| fill="none" | ||
| > | ||
| <path d="M5.75 10.75L8 13L10.25 10.75" strokeWidth={1.5} strokeLinecap="round" strokeLinejoin="round" /> | ||
| <path d="M10.25 5.25L8 3L5.75 5.25" strokeWidth={1.5} strokeLinecap="round" strokeLinejoin="round" /> | ||
| </svg> | ||
| </span> | ||
| </Headless.ListboxButton> | ||
| <Headless.ListboxOptions | ||
| transition | ||
| anchor="selection start" | ||
| className={clsx( | ||
| // Anchor positioning | ||
| '[--anchor-offset:-1.625rem] [--anchor-padding:theme(spacing.4)] sm:[--anchor-offset:-1.375rem]', | ||
| // Base styles | ||
| 'isolate w-max min-w-[calc(var(--button-width)+1.75rem)] select-none scroll-py-1 rounded-xl p-1', | ||
| // Invisible border that is only visible in `forced-colors` mode for accessibility purposes | ||
| 'outline outline-1 outline-transparent focus:outline-none', | ||
| // Handle scrolling when menu won't fit in viewport | ||
| 'overflow-y-scroll overscroll-contain', | ||
| // Popover background | ||
| 'bg-white/75 backdrop-blur-xl dark:bg-zinc-800/75', | ||
| // Shadows | ||
| 'shadow-lg ring-1 ring-zinc-950/10 dark:ring-inset dark:ring-white/10', | ||
| // Transitions | ||
| 'transition-opacity duration-100 ease-in data-[transition]:pointer-events-none data-[closed]:data-[leave]:opacity-0' | ||
| )} | ||
| > | ||
| {options} | ||
| </Headless.ListboxOptions> | ||
| </Headless.Listbox> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Approve use of generics and dynamic class names; suggest CSS simplification.
The use of TypeScript generics enhances the component's reusability, and the dynamic class names are well-handled using the clsx library. However, the complex CSS might be hard to maintain. Consider simplifying the CSS or documenting the styling choices more thoroughly to aid future development.
components/Catalyst/listbox.tsx
Outdated
| export function ListboxOption<T>({ | ||
| children, | ||
| className, | ||
| ...props | ||
| }: { className?: string; children?: React.ReactNode } & Omit< | ||
| Headless.ListboxOptionProps<'div', T>, | ||
| 'as' | 'className' | ||
| >) { | ||
| let sharedClasses = clsx( | ||
| // Base | ||
| 'flex min-w-0 items-center', | ||
| // Icons | ||
| '[&>[data-slot=icon]]:size-5 [&>[data-slot=icon]]:shrink-0 sm:[&>[data-slot=icon]]:size-4', | ||
| '[&>[data-slot=icon]]:text-zinc-500 [&>[data-slot=icon]]:group-data-[focus]/option:text-white [&>[data-slot=icon]]:dark:text-zinc-400', | ||
| 'forced-colors:[&>[data-slot=icon]]:text-[CanvasText] forced-colors:[&>[data-slot=icon]]:group-data-[focus]/option:text-[Canvas]', | ||
| // Avatars | ||
| '[&>[data-slot=avatar]]:-mx-0.5 [&>[data-slot=avatar]]:size-6 sm:[&>[data-slot=avatar]]:size-5' | ||
| ) | ||
|
|
||
| return ( | ||
| <Headless.ListboxOption as={Fragment} {...props}> | ||
| {({ selectedOption }) => { | ||
| if (selectedOption) { | ||
| return <div className={clsx(className, sharedClasses)}>{children}</div> | ||
| } | ||
|
|
||
| return ( | ||
| <div | ||
| className={clsx( | ||
| // Basic layout | ||
| 'group/option grid cursor-default grid-cols-[theme(spacing.5),1fr] items-baseline gap-x-2 rounded-lg py-2.5 pl-2 pr-3.5 sm:grid-cols-[theme(spacing.4),1fr] sm:py-1.5 sm:pl-1.5 sm:pr-3', | ||
| // Typography | ||
| 'text-base/6 text-zinc-950 sm:text-sm/6 dark:text-white forced-colors:text-[CanvasText]', | ||
| // Focus | ||
| 'outline-none data-[focus]:bg-blue-500 data-[focus]:text-white', | ||
| // Forced colors mode | ||
| 'forced-color-adjust-none forced-colors:data-[focus]:bg-[Highlight] forced-colors:data-[focus]:text-[HighlightText]', | ||
| // Disabled | ||
| 'data-[disabled]:opacity-50' | ||
| )} | ||
| > | ||
| <svg | ||
| className="relative hidden size-5 self-center stroke-current group-data-[selected]/option:inline sm:size-4" | ||
| viewBox="0 0 16 16" | ||
| fill="none" | ||
| aria-hidden="true" | ||
| > | ||
| <path d="M4 8.5l3 3L12 4" strokeWidth={1.5} strokeLinecap="round" strokeLinejoin="round" /> | ||
| </svg> | ||
| <span className={clsx(className, sharedClasses, 'col-start-2')}>{children}</span> | ||
| </div> | ||
| ) | ||
| }} | ||
| </Headless.ListboxOption> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Approve implementation; suggest adding unit tests for conditional rendering.
The implementation of ListboxOption is consistent with the main component and uses TypeScript generics effectively. The conditional rendering logic is clear. Consider adding unit tests to verify that the rendering behaves as expected under different conditions.
Would you like help setting up the unit tests for this component?
components/Catalyst/radio.tsx
Outdated
| export function RadioGroup({ | ||
| className, | ||
| ...props | ||
| }: { className?: string } & Omit<Headless.RadioGroupProps, 'as' | 'className'>) { | ||
| return ( | ||
| <Headless.RadioGroup | ||
| data-slot="control" | ||
| {...props} | ||
| className={clsx( | ||
| className, | ||
| // Basic groups | ||
| 'space-y-3 [&_[data-slot=label]]:font-normal', | ||
| // With descriptions | ||
| 'has-[[data-slot=description]]:space-y-6 [&_[data-slot=label]]:has-[[data-slot=description]]:font-medium' | ||
| )} | ||
| /> | ||
| ) |
There was a problem hiding this comment.
Approve implementation; suggest CSS simplification.
The RadioGroup component is implemented well, using modern React practices and effective use of clsx for dynamic class names. Consider simplifying the CSS to improve maintainability and ensure that it is easier for other developers to understand and modify.
components/Catalyst/radio.tsx
Outdated
| export function RadioField({ | ||
| className, | ||
| ...props | ||
| }: { className?: string } & Omit<Headless.FieldProps, 'as' | 'className'>) { | ||
| return ( | ||
| <Headless.Field | ||
| data-slot="field" | ||
| {...props} | ||
| className={clsx( | ||
| className, | ||
| // Base layout | ||
| 'grid grid-cols-[1.125rem_1fr] items-center gap-x-4 gap-y-1 sm:grid-cols-[1rem_1fr]', | ||
| // Control layout | ||
| '[&>[data-slot=control]]:col-start-1 [&>[data-slot=control]]:row-start-1 [&>[data-slot=control]]:justify-self-center', | ||
| // Label layout | ||
| '[&>[data-slot=label]]:col-start-2 [&>[data-slot=label]]:row-start-1 [&>[data-slot=label]]:justify-self-start', | ||
| // Description layout | ||
| '[&>[data-slot=description]]:col-start-2 [&>[data-slot=description]]:row-start-2', | ||
| // With description | ||
| '[&_[data-slot=label]]:has-[[data-slot=description]]:font-medium' | ||
| )} | ||
| /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Approve implementation; suggest CSS simplification.
The RadioField component is implemented appropriately, using a grid layout for responsive design. The use of clsx for dynamic class names is consistent with the rest of the components. Consider simplifying the CSS to enhance maintainability.
components/Catalyst/radio.tsx
Outdated
| export function Radio({ | ||
| color = 'dark/zinc', | ||
| className, | ||
| ...props | ||
| }: { color?: Color; className?: string } & Omit<Headless.RadioProps, 'as' | 'className' | 'children'>) { | ||
| return ( | ||
| <Headless.Radio data-slot="control" {...props} className={clsx(className, 'group inline-flex focus:outline-none')}> | ||
| <span className={clsx([base, colors[color]])}> | ||
| <span | ||
| className={clsx( | ||
| 'size-full rounded-full border-[4.5px] border-transparent bg-[--radio-indicator] bg-clip-padding', | ||
| // Forced colors mode | ||
| 'forced-colors:border-[Canvas] forced-colors:group-data-[checked]:border-[Highlight]' | ||
| )} | ||
| /> | ||
| </span> | ||
| </Headless.Radio> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Approve implementation; suggest documenting CSS custom properties.
The Radio component is implemented well, using custom CSS properties for theming, which enhances the flexibility and reusability of the component. Consider documenting the use of these custom properties to ensure that they are easy to understand and maintain across the application.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (17)
- components/Catalyst/alert.tsx (1 hunks)
- components/Catalyst/avatar.tsx (1 hunks)
- components/Catalyst/badge.tsx (1 hunks)
- components/Catalyst/checkbox.tsx (1 hunks)
- components/Catalyst/description-list.tsx (1 hunks)
- components/Catalyst/dialog.tsx (1 hunks)
- components/Catalyst/dropdown.tsx (1 hunks)
- components/Catalyst/listbox.tsx (1 hunks)
- components/Catalyst/navbar.tsx (1 hunks)
- components/Catalyst/pagination.tsx (1 hunks)
- components/Catalyst/radio.tsx (1 hunks)
- components/Catalyst/sidebar-layout.tsx (1 hunks)
- components/Catalyst/sidebar.tsx (1 hunks)
- components/Catalyst/stacked-layout.tsx (1 hunks)
- components/Catalyst/switch.tsx (1 hunks)
- components/Catalyst/table.tsx (1 hunks)
- components/Catalyst/textarea.tsx (1 hunks)
Files skipped from review due to trivial changes (4)
- components/Catalyst/description-list.tsx
- components/Catalyst/listbox.tsx
- components/Catalyst/pagination.tsx
- components/Catalyst/textarea.tsx
Files skipped from review as they are similar to previous changes (13)
- components/Catalyst/alert.tsx
- components/Catalyst/avatar.tsx
- components/Catalyst/badge.tsx
- components/Catalyst/checkbox.tsx
- components/Catalyst/dialog.tsx
- components/Catalyst/dropdown.tsx
- components/Catalyst/navbar.tsx
- components/Catalyst/radio.tsx
- components/Catalyst/sidebar-layout.tsx
- components/Catalyst/sidebar.tsx
- components/Catalyst/stacked-layout.tsx
- components/Catalyst/switch.tsx
- components/Catalyst/table.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (4)
components/ui-components/dropdown.tsx (2)
20-51: LGTM, but consider extracting the inline styles.The code changes are approved.
However, the component has a lot of inline styles which can be extracted to a separate CSS file to improve readability and maintainability.
Consider extracting the inline styles to a separate CSS file and importing it in the component. For example:
import "./DropdownMenu.css"; export function DropdownMenu({ ... }) { return ( <Headless.MenuItems {...props} className={clsx(className, "dropdown-menu")} /> ); }And in
DropdownMenu.css:.dropdown-menu { /* Anchor positioning */ --anchor-gap: theme(spacing.2); --anchor-padding: theme(spacing.1); ... }
53-90: LGTM, but consider extracting the inline styles.The code changes are approved.
However, the component has a lot of inline styles which can be extracted to a separate CSS file to improve readability and maintainability.
Consider extracting the inline styles to a separate CSS file and importing it in the component. For example:
import "./DropdownItem.css"; export function DropdownItem({ ... }) { return ( <Headless.MenuItem> {"href" in props ? ( <Link {...props} className={clsx(className, "dropdown-item")} /> ) : ( <button type="button" {...props} className={clsx(className, "dropdown-item")} /> )} </Headless.MenuItem> ); }And in
DropdownItem.css:.dropdown-item { /* Base styles */ @apply group cursor-default rounded-lg px-3.5 py-2.5 focus:outline-none sm:px-3 sm:py-1.5; ... }components/ui-components/radio.tsx (1)
123-148: LGTM, but consider extracting the inline styles.The code changes are approved.
However, the component has a lot of inline styles which can be extracted to a separate CSS file to improve readability and maintainability.
Consider extracting the inline styles to a separate CSS file and importing it in the component. For example:
import "./Radio.css"; export function Radio({ color = "dark/zinc", ... }) { return ( <Headless.Radio className={clsx(className, "radio", `radio-${color}`)} > <span className="radio__inner"> <span className="radio__indicator" /> </span> </Headless.Radio> ); }And in
Radio.css:.radio { @apply group inline-flex focus:outline-none; } .radio__inner { /* Base styles */ @apply relative isolate flex size-[1.1875rem] shrink-0 rounded-full sm:size-[1.0625rem]; ... } .radio__indicator { @apply size-full rounded-full border-[4.5px] border-transparent bg-[--radio-indicator] bg-clip-padding; ... } /* Color variants */ .radio-dark\/zinc { --radio-checked-bg: theme(colors.zinc.900); --radio-checked-border: theme(colors.zinc.950/90%); --radio-checked-indicator: theme(colors.white); ... } ...components/ui-components/checkbox.tsx (1)
120-160: LGTM, but consider extracting the inline styles.The code changes are approved.
However, the component has a lot of inline styles which can be extracted to a separate CSS file to improve readability and maintainability.
Consider extracting the inline styles to a separate CSS file and importing it in the component. For example:
import "./Checkbox </blockquote></details> </blockquote></details> <details> <summary>Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** <details> <summary>Commits</summary> Files that changed from the base of the PR and between c1e43f750b35b5e6d4a60ae29eaa7229d5c6592d and 66ac481aa8cc34c1b3c4db11681065308bc4354a. </details> <details> <summary>Files selected for processing (18)</summary> * app/(app)/alpha/additional-details/_client.tsx (1 hunks) * components/ui-components/alert.tsx (1 hunks) * components/ui-components/avatar.tsx (1 hunks) * components/ui-components/badge.tsx (1 hunks) * components/ui-components/checkbox.tsx (1 hunks) * components/ui-components/description-list.tsx (1 hunks) * components/ui-components/dialog.tsx (1 hunks) * components/ui-components/dropdown.tsx (1 hunks) * components/ui-components/listbox.tsx (1 hunks) * components/ui-components/navbar.tsx (1 hunks) * components/ui-components/pagination.tsx (1 hunks) * components/ui-components/radio.tsx (1 hunks) * components/ui-components/sidebar-layout.tsx (1 hunks) * components/ui-components/sidebar.tsx (1 hunks) * components/ui-components/stacked-layout.tsx (1 hunks) * components/ui-components/switch.tsx (1 hunks) * components/ui-components/table.tsx (1 hunks) * components/ui-components/textarea.tsx (1 hunks) </details> <details> <summary>Files skipped from review as they are similar to previous changes (1)</summary> * app/(app)/alpha/additional-details/_client.tsx </details> <details> <summary>Additional comments not posted (57)</summary><blockquote> <details> <summary>components/ui-components/description-list.tsx (3)</summary><blockquote> `3-16`: **LGTM!** The code changes are approved. --- `18-31`: **LGTM!** The code changes are approved. --- `33-46`: **LGTM!** The code changes are approved. </blockquote></details> <details> <summary>components/ui-components/textarea.tsx (1)</summary><blockquote> `5-57`: **LGTM!** The code changes are approved. </blockquote></details> <details> <summary>components/ui-components/avatar.tsx (2)</summary><blockquote> `15-60`: **LGTM!** The code changes are approved. --- `62-100`: **LGTM!** The code changes are approved. </blockquote></details> <details> <summary>components/ui-components/alert.tsx (1)</summary><blockquote> `1-108`: **LGTM!** The `Alert` component and its sub-components are well-structured and follow best practices: - Modular design with separate main component and sub-components. - Utilizes the `@headlessui/react` library for accessibility and interactivity. - Allows for customizable alert sizes through the `sizes` object. - Applies appropriate styling classes using `clsx` for conditional class merging. - Provides a consistent structure for the alert title, description, body, and actions. - Handles transitions and applies responsive styles for different screen sizes. The code changes are approved. </blockquote></details> <details> <summary>components/ui-components/dialog.tsx (1)</summary><blockquote> `1-108`: **LGTM!** The `Dialog` component and its sub-components are well-structured and follow best practices: - Modular design with separate main component and sub-components. - Utilizes the `@headlessui/react` library for accessibility and interactivity. - Allows for customizable dialog sizes through the `sizes` object. - Applies appropriate styling classes using `clsx` for conditional class merging. - Provides a consistent structure for the dialog title, description, body, and actions. - Handles transitions and applies responsive styles for different screen sizes. The code changes are approved. </blockquote></details> <details> <summary>components/ui-components/stacked-layout.tsx (1)</summary><blockquote> `1-89`: **LGTM!** The `StackedLayout` component and its sub-components are well-structured and provide a responsive layout: - Utilizes the `@headlessui/react` library for accessibility and interactivity of the mobile sidebar. - Defines `OpenMenuIcon` and `CloseMenuIcon` components for rendering the menu icons. - Manages the state for showing/hiding the mobile sidebar using the `useState` hook. - Structures the layout with a navbar, sidebar (on mobile), and main content area. - Applies appropriate styling classes for different screen sizes and dark mode. The code changes are approved. </blockquote></details> <details> <summary>components/ui-components/sidebar-layout.tsx (3)</summary><blockquote> `7-13`: **LGTM!** The code changes are approved. --- `15-21`: **LGTM!** The code changes are approved. --- `51-92`: **LGTM!** The code changes are approved. The `SidebarLayout` component is correctly implemented and uses sub-components to render the sidebar and navbar on mobile. It also uses the `useState` hook to manage the state of the sidebar on mobile. </blockquote></details> <details> <summary>components/ui-components/pagination.tsx (5)</summary><blockquote> `5-17`: **LGTM!** The code changes are approved. --- `19-49`: **LGTM!** The code changes are approved. The `PaginationPrevious` component is correctly implemented and uses the `Button` component to render the button. --- `51-81`: **LGTM!** The code changes are approved. The `PaginationNext` component is correctly implemented and uses the `Button` component to render the button. --- `83-93`: **LGTM!** The code changes are approved. --- `95-120`: **LGTM!** The code changes are approved. The `PaginationPage` component is correctly implemented and uses the `Button` component to render the button. </blockquote></details> <details> <summary>components/ui-components/navbar.tsx (6)</summary><blockquote> `10-20`: **LGTM!** The code changes are approved. --- `22-33`: **LGTM!** The code changes are approved. --- `35-46`: **LGTM!** The code changes are approved. The `NavbarSection` component is correctly implemented and uses the `LayoutGroup` component from the `framer-motion` library to group the items in the section. --- `48-59`: **LGTM!** The code changes are approved. --- `61-121`: **LGTM!** The code changes are approved. The `NavbarItem` component is correctly implemented and uses the `Link` component to render a link and the `Headless.Button` component to render a button. It also uses the `motion` component from the `framer-motion` library to animate the current indicator. --- `123-128`: **LGTM!** The code changes are approved. </blockquote></details> <details> <summary>components/ui-components/badge.tsx (1)</summary><blockquote> `39-54`: **LGTM!** The `Badge` component is well-implemented and follows best practices. </blockquote></details> <details> <summary>components/ui-components/table.tsx (7)</summary><blockquote> `20-64`: **LGTM!** The `Table` component is well-implemented and follows best practices. --- `66-76`: **LGTM!** The `TableHead` component is correctly implemented. --- `78-80`: **LGTM!** The `TableBody` component is correctly implemented. --- `82-90`: **LGTM!** The `TableRowContext` is correctly implemented. --- `92-126`: **LGTM!** The `TableRow` component is well-implemented and follows best practices. --- `128-146`: **LGTM!** The `TableHeader` component is well-implemented and follows best practices. --- `148-184`: **LGTM!** The `TableCell` component is well-implemented and follows best practices. </blockquote></details> <details> <summary>components/ui-components/sidebar.tsx (8)</summary><blockquote> `11-21`: **LGTM!** The `Sidebar` component is correctly implemented. --- `23-36`: **LGTM!** The `SidebarHeader` component is correctly implemented. --- `38-51`: **LGTM!** The `SidebarBody` component is correctly implemented. --- `53-66`: **LGTM!** The `SidebarFooter` component is correctly implemented. --- `68-83`: **LGTM!** The `SidebarSection` component is correctly implemented. --- `85-98`: **LGTM!** The `SidebarDivider` component is correctly implemented. --- `100-111`: **LGTM!** The `SidebarSpacer` component is correctly implemented. --- `113-126`: **LGTM!** The `SidebarHeading` component is correctly implemented. </blockquote></details> <details> <summary>components/ui-components/dropdown.tsx (6)</summary><blockquote> `9-11`: **LGTM!** The code changes are approved. --- `13-18`: **LGTM!** The code changes are approved. --- `92-102`: **LGTM!** The code changes are approved. --- `104-121`: **LGTM!** The code changes are approved. --- `123-139`: **LGTM!** The code changes are approved. --- `141-157`: **LGTM!** The code changes are approved. </blockquote></details> <details> <summary>components/ui-components/radio.tsx (2)</summary><blockquote> `4-24`: **LGTM!** The code changes are approved. --- `26-49`: **LGTM!** The code changes are approved. </blockquote></details> <details> <summary>components/ui-components/checkbox.tsx (2)</summary><blockquote> `5-22`: **LGTM!** The code changes are approved. --- `24-47`: **LGTM!** The code changes are approved. </blockquote></details> <details> <summary>components/ui-components/listbox.tsx (4)</summary><blockquote> `9-120`: **Listbox component looks good!** The `Listbox` component is well-implemented and utilizes the `@headlessui/react` library effectively. The prop handling and CSS classes cover various scenarios and provide flexibility in customizing the listbox. --- `122-186`: **ListboxOption component looks good!** The `ListboxOption` component is well-implemented and handles the rendering of options effectively. The conditional rendering based on the selected state is a good approach, and the CSS classes cover various states and provide flexibility in styling the options. --- `188-201`: **ListboxLabel component looks good!** The `ListboxLabel` component is a simple and straightforward component that renders a label for the listbox. The CSS classes handle margin and truncation appropriately. --- `203-219`: **ListboxDescription component looks good!** The `ListboxDescription` component is a useful addition for rendering descriptions for options within the listbox. The CSS classes handle text color, spacing, and truncation appropriately. </blockquote></details> <details> <summary>components/ui-components/switch.tsx (4)</summary><blockquote> `5-22`: **SwitchGroup component looks good!** The `SwitchGroup` component is a useful wrapper for the switch and its label. The CSS classes handle spacing and typography appropriately. --- `24-47`: **SwitchField component looks good!** The `SwitchField` component provides a structured way to represent a switch field with its associated label and description. The CSS classes handle the layout and spacing effectively. --- `49-138`: **`colors` object looks good!** The `colors` object provides a convenient way to define color-specific styles for the switch component. The color schemes cover a wide range of options, allowing for flexibility in customizing the switch appearance. The styles are well-organized and follow a consistent pattern. --- `142-198`: **Switch component looks good!** The `Switch` component is well-implemented and utilizes the `@headlessui/react` library effectively. The prop handling and CSS classes cover various states and scenarios, providing flexibility in customizing the switch appearance. The use of the `colors` object for color-specific styles is a good approach. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| export const BadgeButton = forwardRef(function BadgeButton( | ||
| { | ||
| color = "zinc", | ||
| className, | ||
| children, | ||
| ...props | ||
| }: BadgeProps & { className?: string; children: React.ReactNode } & ( | ||
| | Omit<Headless.ButtonProps, "as" | "className"> | ||
| | Omit<React.ComponentPropsWithoutRef<typeof Link>, "className"> | ||
| ), | ||
| ref: React.ForwardedRef<HTMLElement>, | ||
| ) { | ||
| const classes = clsx( | ||
| className, | ||
| "group relative inline-flex rounded-md focus:outline-none data-[focus]:outline data-[focus]:outline-2 data-[focus]:outline-offset-2 data-[focus]:outline-blue-500", | ||
| ); | ||
|
|
||
| return "href" in props ? ( | ||
| <Link | ||
| {...props} | ||
| className={classes} | ||
| ref={ref as React.ForwardedRef<HTMLAnchorElement>} | ||
| > | ||
| <TouchTarget> | ||
| <Badge color={color}>{children}</Badge> | ||
| </TouchTarget> | ||
| </Link> | ||
| ) : ( | ||
| <Headless.Button {...props} className={classes} ref={ref}> | ||
| <TouchTarget> | ||
| <Badge color={color}>{children}</Badge> | ||
| </TouchTarget> | ||
| </Headless.Button> | ||
| ); | ||
| }); |
There was a problem hiding this comment.
LGTM, but consider a minor refactor.
The BadgeButton component is well-implemented and follows best practices.
However, consider extracting the classes variable to a separate function to improve readability and reusability.
+const getBadgeButtonClasses = (className: string) =>
+ clsx(
+ className,
+ "group relative inline-flex rounded-md focus:outline-none data-[focus]:outline data-[focus]:outline-2 data-[focus]:outline-offset-2 data-[focus]:outline-blue-500",
+ );
+
export const BadgeButton = forwardRef(function BadgeButton(
{
color = "zinc",
className,
children,
...props
}: BadgeProps & { className?: string; children: React.ReactNode } & (
| Omit<Headless.ButtonProps, "as" | "className">
| Omit<React.ComponentPropsWithoutRef<typeof Link>, "className">
),
ref: React.ForwardedRef<HTMLElement>,
) {
- const classes = clsx(
- className,
- "group relative inline-flex rounded-md focus:outline-none data-[focus]:outline data-[focus]:outline-2 data-[focus]:outline-offset-2 data-[focus]:outline-blue-500",
- );
+ const classes = getBadgeButtonClasses(className);
return "href" in props ? (
<Link
{...props}
className={classes}
ref={ref as React.ForwardedRef<HTMLAnchorElement>}
>
<TouchTarget>
<Badge color={color}>{children}</Badge>
</TouchTarget>
</Link>
) : (
<Headless.Button {...props} className={classes} ref={ref}>
<TouchTarget>
<Badge color={color}>{children}</Badge>
</TouchTarget>
</Headless.Button>
);
});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.
| export const BadgeButton = forwardRef(function BadgeButton( | |
| { | |
| color = "zinc", | |
| className, | |
| children, | |
| ...props | |
| }: BadgeProps & { className?: string; children: React.ReactNode } & ( | |
| | Omit<Headless.ButtonProps, "as" | "className"> | |
| | Omit<React.ComponentPropsWithoutRef<typeof Link>, "className"> | |
| ), | |
| ref: React.ForwardedRef<HTMLElement>, | |
| ) { | |
| const classes = clsx( | |
| className, | |
| "group relative inline-flex rounded-md focus:outline-none data-[focus]:outline data-[focus]:outline-2 data-[focus]:outline-offset-2 data-[focus]:outline-blue-500", | |
| ); | |
| return "href" in props ? ( | |
| <Link | |
| {...props} | |
| className={classes} | |
| ref={ref as React.ForwardedRef<HTMLAnchorElement>} | |
| > | |
| <TouchTarget> | |
| <Badge color={color}>{children}</Badge> | |
| </TouchTarget> | |
| </Link> | |
| ) : ( | |
| <Headless.Button {...props} className={classes} ref={ref}> | |
| <TouchTarget> | |
| <Badge color={color}>{children}</Badge> | |
| </TouchTarget> | |
| </Headless.Button> | |
| ); | |
| }); | |
| const getBadgeButtonClasses = (className: string) => | |
| clsx( | |
| className, | |
| "group relative inline-flex rounded-md focus:outline-none data-[focus]:outline data-[focus]:outline-2 data-[focus]:outline-offset-2 data-[focus]:outline-blue-500", | |
| ); | |
| export const BadgeButton = forwardRef(function BadgeButton( | |
| { | |
| color = "zinc", | |
| className, | |
| children, | |
| ...props | |
| }: BadgeProps & { className?: string; children: React.ReactNode } & ( | |
| | Omit<Headless.ButtonProps, "as" | "className"> | |
| | Omit<React.ComponentPropsWithoutRef<typeof Link>, "className"> | |
| ), | |
| ref: React.ForwardedRef<HTMLElement>, | |
| ) { | |
| const classes = getBadgeButtonClasses(className); | |
| return "href" in props ? ( | |
| <Link | |
| {...props} | |
| className={classes} | |
| ref={ref as React.ForwardedRef<HTMLAnchorElement>} | |
| > | |
| <TouchTarget> | |
| <Badge color={color}>{children}</Badge> | |
| </TouchTarget> | |
| </Link> | |
| ) : ( | |
| <Headless.Button {...props} className={classes} ref={ref}> | |
| <TouchTarget> | |
| <Badge color={color}>{children}</Badge> | |
| </TouchTarget> | |
| </Headless.Button> | |
| ); | |
| }); |
| @@ -1,4 +1,4 @@ | |||
| #!/usr/bin/env sh | |||
| . "$(dirname -- "$0")/_/husky.sh" | |||
There was a problem hiding this comment.
Delete this entire file and references to Husky if it's not being used 🤖
There was a problem hiding this comment.
Oh yeah.
I didn't delete it because I thought there may be other uses for it?
I actually thought we might use it to run prettier on pre commit.
There was a problem hiding this comment.
I don't see anywhere else we need it so get rid of it everywhere I guess.
…odu into addition/catalyst-components especially if it merges an updated upstream into a topic branch.
✨ Codu Pull Request 💻
Disable husky pre commit linting. husky has not been removed, the command has been commented out.
Remove Catalyst components from Alpha
Add all Catalyst components to components>catalyst