Drawer Component #3986
Drawer Component #3986
Conversation
Signed-off-by: The1111mp <The1111mp@outlook.com>
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces a new Drawer component to the NextUI library, along with comprehensive documentation and multiple supporting components. Key changes include the addition of new routes in the documentation, new React components for the Drawer interface, and unit tests for functionality verification. The Drawer component features various configurations, such as size and placement, and integrates with existing modal functionality. The changes also include updates to the theme and exports to facilitate the use of the new Drawer component within the library. Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Outside diff range and nitpick comments (38)
apps/docs/content/components/drawer/index.ts (1)
1-17: LGTM! Consider adding JSDoc documentation.The file structure is well-organized, with a clear separation of concerns and consistent naming conventions. The exports are logically grouped and follow a natural progression from basic usage to advanced features.
Consider adding JSDoc documentation to describe the purpose of the
drawerContentexport:+/** + * Documentation content for the Drawer component. + * Includes examples for basic usage, sizing, placement, forms, + * backdrop variations, and custom animations. + */ export const drawerContent = {packages/components/drawer/src/drawer.tsx (1)
10-18: Consider architectural improvements for robustness and performance.While the current implementation is clean, consider these improvements:
- Wrap the Modal in an error boundary to handle potential rendering failures gracefully
- Consider implementing
memofor performance optimization if the drawer content is complex- Ensure proper cleanup in the
useDrawerhook for animations and event listenersWould you like me to provide example implementations for any of these suggestions?
apps/docs/content/components/drawer/non-dismissable.ts (2)
1-4: Consider adding TypeScript types for better documentation.Since this is a documentation example, adding TypeScript types would provide better guidance for users implementing the component.
-const App = `import {Drawer, DrawerContent, DrawerHeader, DrawerBody, DrawerFooter, Button, useDisclosure} from "@nextui-org/react"; +const App = `import {Drawer, DrawerContent, DrawerHeader, DrawerBody, DrawerFooter, Button, useDisclosure} from "@nextui-org/react"; + +type Props = { + // Add any props if needed +}; -export default function App() { +export default function App(props: Props) {
15-41: Enhance documentation example with meaningful content.Replace the Lorem ipsum text with content that better demonstrates the drawer's purpose and common use cases. This would provide more value to developers learning how to implement the component.
<p> Click the close button or action button to close the drawer. Clicking outside or pressing the escape key won't close it. </p> <p> - Lorem ipsum dolor sit amet, consectetur adipiscing elit. - Nullam pulvinar risus non risus hendrerit venenatis. - Pellentesque sit amet hendrerit risus, sed porttitor quam. + This pattern is useful for important forms or processes that + shouldn't be accidentally dismissed. For example, when users + are in the middle of a critical workflow or data entry task. </p>packages/core/theme/src/components/drawer.ts (3)
5-19: Update JSDoc documentation to reflect Drawer componentThe current documentation refers to a "Modal" component and provides a basic example. Consider enhancing it to:
- Correctly reference the "Drawer" component
- Demonstrate the usage of size and placement variants
- Include examples of common use cases
/** - * Modal **Tailwind Variants** component + * Drawer **Tailwind Variants** component * * @example * ```js - * const {base} = drawer({...}) + * const {base} = drawer({ + * size: "md", + * placement: "right" + * }) * * <div> * <button>Open Drawer</button> * <div className={base()}> * Drawer Content * </div> * </div> * ``` */
25-56: Consider using relative units for better responsivenessThe current implementation uses fixed max-height values which might not scale well across different viewport sizes. Consider:
- Using viewport-relative units (vh/dvh) for better adaptability
- Adding responsive variants for different breakpoints
size: { xs: { - base: "max-w-xs max-h-[20rem]", + base: "max-w-xs max-h-[30dvh]", }, sm: { - base: "max-w-sm max-h-[24rem]", + base: "max-w-sm max-h-[40dvh]", }, // ... adjust other sizes similarly
57-70: Add transition properties for smooth animationsThe placement variants handle positioning well, but adding transition properties would improve the user experience with smooth animations.
placement: { top: { - base: "inset-x-0 top-0 max-w-[none] rounded-t-none", + base: "inset-x-0 top-0 max-w-[none] rounded-t-none transition-transform duration-300", }, // Apply similar changes to other placement variantsapps/docs/content/components/drawer/custom-motion.ts (3)
9-26: Consider improving animation configuration
- The exit animation slides to the right (x: 100) which might look unnatural for a drawer. Consider sliding back to the original position instead.
- The motion configuration could be extracted for reusability.
Consider applying these improvements:
+const drawerMotion = { + variants: { + enter: { + opacity: 1, + x: 0, + duration: 0.3, + }, + exit: { - x: 100, + x: -100, // Slide back to original position + opacity: 0, + duration: 0.3, + }, + }, +}; <Drawer isOpen={isOpen} onOpenChange={onOpenChange} - motionProps={{ - variants: { - enter: { - opacity: 1, - x: 0, - duration: 0.3, - }, - exit: { - x: 100, - opacity: 0, - duration: 0.3, - }, - }, - }} + motionProps={drawerMotion} >
32-39: Replace Lorem ipsum with meaningful example contentThe Lorem ipsum placeholder text should be replaced with content that demonstrates a real-world use case, making the documentation more practical and helpful for users.
57-63: Ensure consistency in file extensionsThe example is exported with a
.jsxextension while the actual file uses TypeScript. Consider using.tsxfor consistency with the TypeScript codebase.const react = { - "/App.jsx": App, + "/App.tsx": App, };packages/components/drawer/package.json (1)
2-4: Consider enhancing the package descriptionThe current description is basic. Consider expanding it to highlight key features like placement options, sizes, or integration capabilities with NextUI's theming system.
- "description": "Used to render a content that slides in from the side of the screen.", + "description": "A customizable drawer component that slides in from any edge of the screen, supporting multiple sizes, placements, and NextUI's theming system.",apps/docs/content/components/drawer/sizes.ts (3)
1-5: Add type safety for size stateConsider adding proper TypeScript types for the size state to prevent potential runtime errors and improve code maintainability.
- const [size, setSize] = React.useState('md') + type DrawerSize = "xs" | "sm" | "md" | "lg" | "xl" | "2xl" | "3xl" | "4xl" | "5xl" | "full"; + const DEFAULT_SIZE: DrawerSize = "md"; + const [size, setSize] = React.useState<DrawerSize>(DEFAULT_SIZE);
7-12: Move sizes array outside componentTo optimize performance, define the sizes array as a constant outside the component to prevent unnecessary recreation on each render.
+const DRAWER_SIZES = ["xs", "sm", "md", "lg", "xl", "2xl", "3xl", "4xl", "5xl", "full"] as const; + export default function App() { const {isOpen, onOpen, onClose} = useDisclosure(); const [size, setSize] = React.useState('md') - - const sizes = ["xs", "sm", "md", "lg", "xl", "2xl", "3xl", "4xl", "5xl", "full"];
58-64: Simplify export configurationThe current export structure is unnecessarily complex for a single component.
-const react = { - "/App.jsx": App, -}; - -export default { - ...react, -}; +export default { + "/App.jsx": App, +};apps/docs/content/components/drawer/usage.ts (4)
8-9: Add ARIA label for better accessibility.The trigger button should have an aria-label to improve accessibility.
- <Button onPress={onOpen}>Open Drawer</Button> + <Button onPress={onOpen} aria-label="Open drawer">Open Drawer</Button>
13-32: Replace Lorem ipsum with meaningful example content.For documentation purposes, it would be more helpful to use content that demonstrates a real-world use case of the Drawer component, such as a settings panel or a shopping cart.
33-40: Improve Action button implementation.The Action button needs more context:
- Add a more descriptive label indicating its purpose
- Consider adding an aria-label
- Consider adding loading state handling for async actions
- <Button color="primary" onPress={onClose}> - Action - </Button> + <Button + color="primary" + onPress={onClose} + aria-label="Save changes" + > + Save Changes + </Button>
1-55: Consider adding advanced usage examples.While this example effectively demonstrates basic drawer usage, consider adding examples that showcase:
- Different drawer placements (left, right, top, bottom)
- Various drawer sizes
- Custom backdrop handling
- Nested drawers
- Form integration
- Custom transitions
This would help users better understand the component's full capabilities.
apps/docs/content/components/drawer/placement.ts (1)
12-59: Enhance accessibility and user experienceWhile the implementation is solid, consider these improvements:
- Add aria-label to buttons to better describe their purpose
- Add more descriptive labels for footer buttons
- Consider adding role="dialog" and aria-modal="true" to the Drawer
Here's how to improve the accessibility:
<Button key={placement} onPress={() => handleOpen(placement)} className="capitalize" + aria-label={`Open drawer from ${placement}`} >- <Button color="danger" variant="light" onPress={onClose}> - Close + <Button color="danger" variant="light" onPress={onClose} aria-label="Close drawer"> + Cancel </Button> - <Button color="primary" onPress={onClose}> - Action + <Button color="primary" onPress={onClose} aria-label="Save changes"> + Save </Button>packages/components/drawer/__tests__/drawer.test.tsx (3)
8-9: Improve the clarity of the warning suppression comment.The comment about ref warnings could be more descriptive to explain why we're suppressing these specific warnings in the test context.
-// e.g. console.error Warning: Function components cannot be given refs. -// Attempts to access this ref will fail. Did you mean to use React.forwardRef()? +// Suppress expected ref warnings during tests. +// These warnings occur when testing ref forwarding and are expected behavior +// since we're validating that refs are properly handled by the Drawer component.
92-109: Use userEvent instead of fireEvent for keyboard interactions.For consistency with other tests and better simulation of real user interactions, use
userEventfor keyboard events.- fireEvent.keyDown(drawer, {key: "Escape"}); + const user = userEvent.setup(); + await user.keyboard('{Escape}');
46-69: Enhance accessibility testing.While the current ARIA attribute tests are good, consider adding tests for:
- Focus management (focus trap within drawer)
- Keyboard navigation between drawer elements
- Focus restoration after drawer closes
Example test for focus management:
it("should trap focus within drawer when open", async () => { const {getByRole, getByText} = render( <Drawer isOpen> <DrawerContent> <DrawerHeader> <button>First</button> </DrawerHeader> <DrawerBody> <button>Middle</button> </DrawerBody> <DrawerFooter> <button>Last</button> </DrawerFooter> </DrawerContent> </Drawer> ); const user = userEvent.setup(); await user.tab(); expect(document.activeElement).toHaveTextContent("First"); await user.tab(); expect(document.activeElement).toHaveTextContent("Middle"); await user.tab(); expect(document.activeElement).toHaveTextContent("Last"); await user.tab(); // Should cycle back to first element expect(document.activeElement).toHaveTextContent("First"); });apps/docs/content/components/drawer/form.ts (3)
60-77: Enhance form fields with validation and error states.The email and password inputs lack validation feedback and error handling.
Add validation and error states:
<Input autoFocus + isRequired + errorMessage={errors.email?.message} endContent={ <MailIcon className="text-2xl text-default-400 pointer-events-none flex-shrink-0" /> } label="Email" placeholder="Enter your email" variant="bordered" + {...register('email', { + required: 'Email is required', + pattern: { + value: /^[A-Z0-9._%+-]+@[A-Z0-9.-]+\.[A-Z]{2,}$/i, + message: 'Invalid email address' + } + })} />
91-98: Add loading state and error handling for sign-in action.The sign-in button lacks loading state and error handling.
Consider adding loading state and error handling:
<DrawerFooter> <Button color="danger" variant="flat" onPress={onClose}> Close </Button> - <Button color="primary" onPress={onClose}> + <Button + color="primary" + isLoading={isLoading} + onPress={handleSubmit(async (data) => { + try { + setIsLoading(true); + await handleSignIn(data); + onClose(); + } catch (error) { + setError(error.message); + } finally { + setIsLoading(false); + } + })} + > Sign in </Button> </DrawerFooter>
107-115: Consider restructuring the export format for better maintainability.The current export structure using string templates and file paths might make maintenance and updates more challenging.
Consider exporting the components directly:
export const examples = { basic: { App, MailIcon, LockIcon, }, };apps/docs/content/components/drawer/backdrop.ts (1)
1-39: Enhance icon components implementation.Consider the following improvements for better maintainability and type safety:
- Define icons as actual components instead of template strings
- Add TypeScript types for props
- Add explicit units for width/height
Example implementation:
interface IconProps extends React.SVGProps<SVGSVGElement> { className?: string; } export const MailIcon: React.FC<IconProps> = (props) => ( <svg aria-hidden="true" fill="none" focusable="false" height="1em" width="1em" role="presentation" viewBox="0 0 24 24" {...props} > {/* path remains the same */} </svg> );packages/core/theme/src/components/alert.ts (1)
27-34: Consider documenting the text-inherit behaviorThe slots modifications look good, particularly:
- Simplified class definitions using strings instead of arrays
- Added text-inherit for better color inheritance
- Standardized icon width with w-6
Consider adding a comment explaining the text-inherit behavior for future maintainers.
packages/components/drawer/stories/drawer.stories.tsx (4)
1-1: Document the reason for disabling the ESLint rule.The ESLint rule
jsx-a11y/no-autofocusis disabled globally for the file. Consider adding a comment explaining why this accessibility rule needs to be disabled, or better yet, scope it to the specific component where it's needed.
18-23: Consider using type-safe size options.The size options could be derived from the component's type definitions to ensure they stay in sync with the actual component implementation.
-options: ["xs", "sm", "md", "lg", "xl", "2xl", "3xl", "4xl", "5xl", "6xl", "full"], +options: drawer.sizes,
136-166: Optimize PlacementTemplate implementation.The current implementation could be improved:
- Memoize the placements array to prevent unnecessary recreations
- Memoize the handlePress callback to prevent unnecessary rerenders
const PlacementTemplate = (args: DrawerProps) => { const [placement, setPlacement] = React.useState<DrawerProps["placement"]>("right"); const {isOpen, onOpen, onOpenChange} = useDisclosure({defaultOpen: args.defaultOpen}); - const handlePress = (placement: DrawerProps["placement"]) => { + const handlePress = React.useCallback((placement: DrawerProps["placement"]) => { setPlacement(placement); onOpen(); - }; + }, [onOpen]); - const placements = ["right", "left", "top", "bottom"] as DrawerProps["placement"][]; + const placements = React.useMemo(() => + ["right", "left", "top", "bottom"] as DrawerProps["placement"][], + [] + );
199-218: Enhance custom motion animation configuration.The animation configuration could be improved by:
- Adding transform origin for better animation control
- Using NextUI's standard duration values for consistency
args: { ...defaultProps, motionProps: { variants: { enter: { opacity: 1, x: 0, - duration: 0.3, + duration: 0.2, + transformOrigin: "right", }, exit: { x: 100, opacity: 0, - duration: 0.3, + duration: 0.2, + transformOrigin: "right", }, }, }, },packages/components/modal/__tests__/modal.test.tsx (3)
Line range hint
1-29: Add TypeScript types to ModalDraggable propsThe component props should be properly typed for better maintainability and type safety.
-const ModalDraggable = ({canOverflow = false, isDisabled = false}) => { +interface ModalDraggableProps { + canOverflow?: boolean; + isDisabled?: boolean; +} + +const ModalDraggable: React.FC<ModalDraggableProps> = ({ + canOverflow = false, + isDisabled = false, +}) => {
Line range hint
31-116: Add focus management testsThe current test suite lacks coverage for focus management, which is crucial for modal accessibility. Consider adding tests for:
- Focus trap within the modal
- Initial focus placement
- Focus restoration when modal closes
test("should trap focus within modal", async () => { const user = userEvent.setup(); const {getByRole, getByText} = render( <Modal isOpen> <ModalContent> <ModalHeader>Modal header</ModalHeader> <ModalBody> <button>First</button> <button>Last</button> </ModalBody> </ModalContent> </Modal> ); const modal = getByRole("dialog"); await user.tab(); expect(document.activeElement).toBeInTheDocument(); expect(modal).toContain(document.activeElement); });
Line range hint
118-191: Enhance draggable modal test coverageThe draggable modal tests could be improved in several areas:
- Add mouse event tests for desktop drag interactions
- Consider DPI scaling in viewport calculations
- Test drag constraints and boundaries
test("should handle mouse drag events on desktop", () => { jest.spyOn(document.documentElement, "clientWidth", "get") .mockImplementation(() => 1920); const wrapper = render(<ModalDraggable />); const modalHeader = wrapper.getByText("Modal header"); fireEvent.mouseDown(modalHeader, {clientX: 0, clientY: 0}); fireEvent.mouseMove(modalHeader, {clientX: 100, clientY: 50}); fireEvent.mouseUp(modalHeader); const modal = wrapper.getByRole("dialog"); expect(modal.style.transform).toBe("translate(100px, 50px)"); }); test("should respect minimum drag bounds", () => { // Add test for minimum drag boundaries });apps/docs/content/docs/components/drawer.mdx (3)
31-60: Add context about import strategies.Consider adding a brief explanation about when to use the main
@nextui-org/reactimport versus individual@nextui-org/drawerimports. This would help developers make informed decisions about bundle size optimization.🧰 Tools
🪛 LanguageTool
[uncategorized] ~49-~49: Loose punctuation mark.
Context: ...rawerFooter } from "@nextui-org/react";, individual:import { Drawe...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~57-~57: Loose punctuation mark.
Context: ...awerFooter } from "@nextui-org/drawer";`, }} /> ## Usage When the drawer open...(UNLIKELY_OPENING_PUNCTUATION)
70-72: Add default size information.Consider adding the default size value in the sizes section introduction to help developers make informed decisions about customization.
153-170: Enhance props documentation.Consider the following improvements to the props table:
- Add type information for the
backdropprop that was mentioned in the usage section- Consider grouping related props (e.g., grouping all dismissal-related props together)
- Add examples for the
classNamesprop usage🧰 Tools
🪛 LanguageTool
[uncategorized] ~165-~165: Consider adding a hyphen.
Context: ... | Custom close button to display on top right corner. ...(TOP_LEFT_CORNER)
apps/docs/config/routes.json (1)
Line range hint
1-262: Standardize component status flagsThere's an inconsistency in how new components are marked:
- Most new components use
"newPost": true- The Alert component uses
"isNew": trueConsider standardizing all status flags to use
newPostfor consistency.Example for Alert component:
{ "key": "alert", "title": "Alert", "keywords": "alert, notification, message", "path": "/docs/components/alert.mdx", - "isNew": true + "newPost": true }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
apps/docs/config/routes.json(1 hunks)apps/docs/content/components/drawer/backdrop.ts(1 hunks)apps/docs/content/components/drawer/custom-motion.ts(1 hunks)apps/docs/content/components/drawer/form.ts(1 hunks)apps/docs/content/components/drawer/index.ts(1 hunks)apps/docs/content/components/drawer/non-dismissable.ts(1 hunks)apps/docs/content/components/drawer/placement.ts(1 hunks)apps/docs/content/components/drawer/sizes.ts(1 hunks)apps/docs/content/components/drawer/usage.ts(1 hunks)apps/docs/content/components/index.ts(1 hunks)apps/docs/content/docs/components/drawer.mdx(1 hunks)packages/components/drawer/README.md(1 hunks)packages/components/drawer/__tests__/drawer.test.tsx(1 hunks)packages/components/drawer/package.json(1 hunks)packages/components/drawer/src/drawer.tsx(1 hunks)packages/components/drawer/src/index.ts(1 hunks)packages/components/drawer/src/use-drawer.ts(1 hunks)packages/components/drawer/stories/drawer.stories.tsx(1 hunks)packages/components/drawer/tsconfig.json(1 hunks)packages/components/drawer/tsup.config.ts(1 hunks)packages/components/modal/__tests__/modal.test.tsx(1 hunks)packages/core/react/package.json(1 hunks)packages/core/react/src/index.ts(1 hunks)packages/core/theme/src/components/alert.ts(6 hunks)packages/core/theme/src/components/drawer.ts(1 hunks)packages/core/theme/src/components/index.ts(1 hunks)packages/core/theme/src/utils/variants.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/components/drawer/tsconfig.json
- packages/components/drawer/tsup.config.ts
🧰 Additional context used
🪛 LanguageTool
apps/docs/content/docs/components/drawer.mdx
[uncategorized] ~49-~49: Loose punctuation mark.
Context: ...rawerFooter } from "@nextui-org/react";, individual: import { Drawe...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~57-~57: Loose punctuation mark.
Context: ...awerFooter } from "@nextui-org/drawer";`, }} /> ## Usage When the drawer open...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~165-~165: Consider adding a hyphen.
Context: ... | Custom close button to display on top right corner. ...
(TOP_LEFT_CORNER)
packages/components/drawer/README.md
[style] ~3-~3: Consider using the synonym “brief” (= concise, using a few words, not lasting long) to strengthen your wording.
Context: # @nextui-org/drawer A Quick description of the component > This is...
(QUICK_BRIEF)
[typographical] ~17-~17: Usually, a comma is necessary before ‘Yes’ at the end of a sentence. Did you mean “, please”?
Context: ...tui-org/drawer ``` ## Contribution Yes please! See the [contributing guidelines](http...
(COMMA_BEFORE_PLEASE)
🔇 Additional comments (31)
packages/components/drawer/src/index.ts (3)
5-6: Well-organized exports with clear separation of concerns.
The exports are well-structured, following a clear pattern:
- Types are exported separately
- Hooks are isolated
- Main component is exported independently
This organization makes it easier for users to import only what they need, potentially enabling better tree-shaking.
Also applies to: 8-12
14-20: Consider documenting the relationship between Modal and Drawer components.
The re-export of Modal subcomponents as Drawer components is a good approach for code reuse. However, it would be beneficial to:
- Document this relationship in the component's documentation
- Ensure that all Modal props relevant to Drawer functionality are properly typed in the DrawerProps interface
1-3: Verify Modal dependency compatibility.
The Drawer component is built on top of the Modal component, which is a common and efficient pattern. However, we should ensure that the Modal component supports all the features needed for a Drawer implementation.
✅ Verification successful
Modal dependency is appropriately used and compatible with Drawer requirements
The implementation shows that:
- The Drawer component extends Modal functionality while properly handling drawer-specific requirements
- It implements custom placement logic ("top", "right", "bottom", "left") with appropriate animations
- Modal components (Header, Body, Footer, Content) are correctly imported and used as building blocks
- The implementation properly omits Modal-specific props that aren't relevant for Drawer (placement, scrollBehavior)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Modal component capabilities and props
# Expected: Modal component should support placement/position props needed for drawer functionality
# Check Modal component implementation
ast-grep --pattern 'interface ModalProps {
$$$
}'
# Check Modal usage in tests
rg -A 5 "Modal.*placement|position" packages/components/modal/__tests__/
Length of output: 176
Script:
#!/bin/bash
# Let's check the actual implementation and props of both Modal and Drawer
# Check Modal props and implementation
ast-grep --pattern 'interface ModalProps {
$$$
}'
# Check Drawer props
ast-grep --pattern 'interface DrawerProps {
$$$
}'
# Check how Drawer uses Modal components
rg -A 10 "import.*Modal" packages/components/drawer/src/
Length of output: 2601
Script:
#!/bin/bash
# Let's check the placement implementation details
# Check placement prop definition
rg -A 5 "placement.*:" packages/components/drawer/src/
# Check how placement is handled in the implementation
ast-grep --pattern 'useDrawer($$$) {
$$$
}'
# Check if there are any drawer-specific styles or animations
rg -A 5 "drawer.*={" packages/components/drawer/src/
Length of output: 1574
packages/components/drawer/src/drawer.tsx (2)
10-18: Add JSDoc documentation and improve type safety.
The component would benefit from proper documentation and explicit type annotations.
+/**
+ * A drawer component that slides in from the edge of the screen.
+ * Built on top of Modal component with additional positioning and animation features.
+ *
+ * @example
+ * ```tsx
+ * <Drawer placement="left" isOpen onClose={() => setOpen(false)}>
+ * <DrawerContent>Drawer content</DrawerContent>
+ * </Drawer>
+ * ```
+ */
-const Drawer = forwardRef<"div", DrawerProps>(({children, ...props}, ref) => {
+const Drawer = forwardRef<HTMLDivElement, DrawerProps>(({children, ...props}, ref) => {Let's verify the accessibility features inherited from Modal:
#!/bin/bash
# Description: Check Modal component's accessibility implementation
# Expected: Find ARIA attributes and keyboard handling
# Search for ARIA attributes and role definitions in Modal
rg -A 5 'aria-|role=' packages/components/modal/
# Search for keyboard event handlers
rg -A 5 'onKey|handleKey' packages/components/modal/20-22: Verify component exports in the public API.
The component's export setup looks good, but let's ensure it's properly exposed in the public API.
✅ Verification successful
Let me generate additional scripts to verify the package.json configuration since the previous command didn't show the package.json fields correctly.
Let me try one more verification to check the core package exports with a broader search:
Component is properly exported in the public API
Based on the verification results:
- The Drawer component is correctly exported in its package's index file
- The component is properly re-exported in the main NextUI package via
@nextui-org/drawer - The package.json configuration is set up correctly with the main entry point
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Drawer component exports
# Expected: Find Drawer exports in index files and package.json
# Check if Drawer is exported in the package's index file
rg -A 5 "export.*Drawer" packages/components/drawer/
# Verify package.json main/module/types fields
fd -e json -x jq -r '.main, .module, .types' {} \; packages/components/drawer/package.json
Length of output: 2052
Script:
#!/bin/bash
# Check package.json content directly
cat packages/components/drawer/package.json
# Also verify if the component is re-exported in the main nextui package
rg -A 3 "drawer" packages/core/nextui/src/index.ts
Length of output: 2004
Script:
#!/bin/bash
# Find the main NextUI package index file
fd -t f index.ts -x cat {} \; packages/core/
# Also check if there are any other index files that might re-export the drawer
fd -t f index.ts packages/
Length of output: 11348
apps/docs/content/components/index.ts (1)
35-35: LGTM! The export statement follows the established pattern.
The addition of the drawer export maintains consistency with other component exports and is necessary for making the drawer component available through the public API.
Let's verify that the drawer component exists:
✅ Verification successful
Export statement is correctly implemented and the drawer component is properly structured
The verification confirms that:
- The drawer component exists in the packages directory
- The drawer documentation is properly structured in
apps/docs/content/components/drawer/ - The export follows the established pattern and points to an existing module
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of the drawer component
# Expected: Find the drawer component implementation
# Test 1: Check if the drawer component exists
fd -t f "drawer.tsx" -X echo "Found drawer component at: {}"
# Test 2: Verify the component's export structure
ast-grep --pattern 'export * from "./drawer"'
Length of output: 319
Script:
#!/bin/bash
# Let's verify the drawer component's implementation and exports
# Check if there's a corresponding drawer.ts file in the docs content directory
fd -t f "drawer.ts" -d 1 apps/docs/content/components
# Check the content of the drawer export file in docs
rg -g "drawer.ts" "export" apps/docs/content/components
Length of output: 107
Script:
#!/bin/bash
# Let's check the drawer directory structure and content
fd -t f "drawer" apps/docs/content/components
# Check if there's a drawer directory with index file
fd -t f "index.ts" apps/docs/content/components/drawer
# Look for any drawer-related exports in the components directory
rg -g "*.ts" "drawer" apps/docs/content/components
Length of output: 587
packages/core/theme/src/components/index.ts (2)
42-42: LGTM! Export follows consistent pattern
The new drawer export follows the established pattern and is correctly placed in the theme components index.
42-42: Verify drawer theme file existence
Let's ensure the corresponding drawer theme file exists and is properly structured.
✅ Verification successful
✓ Drawer theme file exists and is properly structured
The drawer theme file exists at the correct location and follows the expected structure with:
- Proper theme configuration using
tvutility - Well-defined variants for size and placement
- Correct exports of both the drawer configuration and its type
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the drawer theme file exists and follows the expected structure
# Test 1: Check if the drawer theme file exists
fd -t f "drawer.ts" "packages/core/theme/src/components"
# Test 2: Verify the theme structure
if [ -f "packages/core/theme/src/components/drawer.ts" ]; then
# Look for theme export pattern
rg -l "export const drawer\s*=\s*{" "packages/core/theme/src/components/drawer.ts"
fi
Length of output: 358
Script:
#!/bin/bash
# Let's verify the drawer theme file content with correct regex
# First, check if file exists and show its content
cat packages/core/theme/src/components/drawer.ts
# Also check for any export pattern without regex
rg "export" packages/core/theme/src/components/drawer.ts
Length of output: 1736
apps/docs/content/components/drawer/non-dismissable.ts (1)
46-52: LGTM! Export configuration is well-structured.
The export structure is clean and allows for easy integration with the documentation system.
packages/core/react/src/index.ts (1)
47-47: LGTM! The export follows the established patterns.
The new drawer export is consistent with the module naming convention and is properly grouped with other UI component exports.
Let's verify the drawer package is properly listed as a dependency:
✅ Verification successful
Drawer package is properly configured as a workspace dependency
The verification confirms:
- The drawer package is correctly listed as a workspace dependency in
packages/core/react/package.json - The package exists in the expected location at
packages/components/drawer/
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify drawer package dependency
# Expected: The drawer package should be listed in package.json dependencies
# Test 1: Check if drawer is listed in dependencies
jq -r '.dependencies["@nextui-org/drawer"]' packages/core/react/package.json
# Test 2: Verify the package exists in the workspace
fd -t d "drawer$" packages/components/
Length of output: 159
apps/docs/content/components/drawer/custom-motion.ts (1)
1-5: LGTM: Clean component setup with proper imports and hooks usage
The component structure follows React best practices with appropriate imports and proper state management using useDisclosure.
packages/components/drawer/package.json (3)
13-26: Package configuration looks good
The package configuration follows NextUI's standard patterns with correct repository links and distribution settings.
27-35: Build and development scripts are properly configured
The scripts follow NextUI's standard component setup with appropriate build, development, and cleaning tasks.
Also applies to: 63-63
36-62: Verify architectural dependency on @nextui-org/modal
The drawer component depends on @nextui-org/modal. While this is a common pattern, ensure this dependency is necessary and doesn't create potential circular dependencies or tight coupling.
✅ Verification successful
Dependency on @nextui-org/modal is well-justified and properly implemented
The drawer component is intentionally built on top of the Modal component, using it as a base while adding drawer-specific functionality:
- It extends Modal's props and adds drawer-specific properties (placement, scrollBehavior)
- Uses Modal as a wrapper component with custom motion and styling props
- No circular dependencies were found
- The implementation follows the composition pattern, which is a good architectural choice
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential circular dependencies between modal and drawer
echo "Checking for potential circular dependencies..."
rg -l "@nextui-org/drawer" packages/components/modal/
Length of output: 158
Script:
#!/bin/bash
# Let's analyze the drawer implementation to understand its relationship with modal
echo "Checking drawer's usage of modal..."
fd -e tsx -e ts drawer.tsx packages/components/drawer/ --exec ast-grep --pattern 'import {$$$} from "@nextui-org/modal"'
echo -e "\nChecking drawer's implementation..."
fd -e tsx -e ts drawer.tsx packages/components/drawer/ --exec cat
Length of output: 982
Script:
#!/bin/bash
# Let's check the use-drawer hook to understand how it uses modal functionality
echo "Checking use-drawer hook implementation..."
fd -e tsx -e ts use-drawer.tsx? packages/components/drawer/ --exec cat
# Let's also check if there are any other components that might be affected
echo -e "\nChecking for other components using drawer..."
rg -l "@nextui-org/drawer" packages/components/ --type ts --type tsx
Length of output: 2784
apps/docs/content/components/drawer/usage.ts (2)
1-5: LGTM! Clean setup with proper state management.
The component setup follows React best practices, using the useDisclosure hook for managing drawer state. The template string approach is appropriate for documentation purposes.
49-55: LGTM! Clean export configuration.
The export structure is appropriate for documentation purposes.
apps/docs/content/components/drawer/placement.ts (2)
7-10: LGTM! Clean and efficient event handler implementation
The handler correctly manages both the placement state and drawer visibility.
62-68: LGTM! Export configuration is correct
The export structure follows the documentation pattern and is properly implemented.
packages/core/theme/src/utils/variants.ts (1)
33-37: Review color contrast and accessibility implications.
The changes to the flat variant's text colors show a systematic lightening of shades, which could affect readability:
- The shift from 700/800 to 600/700 makes text colors consistently lighter
- Dark mode colors are explicitly defined for success/warning/danger states
Consider verifying that these color combinations meet WCAG contrast requirements, especially for the flat variant which now uses lighter text colors (600/700) on light backgrounds (20% opacity).
✅ Verification successful
Let me gather more specific information about color contrast testing in the codebase.
Based on the gathered information, I can now provide a definitive response about the color contrast concerns.
Color contrast changes have been verified and are part of an accessibility improvement
The changes to the flat variant's text colors were intentionally made to improve accessibility, as evidenced by:
-
The codebase has built-in color contrast testing (packages/core/theme/tests/semantic-colors.test.ts) that enforces minimum contrast ratios:
- "decorative": 1.5:1
- "readable": 3:1 (default target)
- "aa": 4.5:1
- "aaa": 7:1
-
The changes are documented in the changelog (packages/core/theme/CHANGELOG.md) as an intentional accessibility improvement:
Updates the text colors for the flat variant in the theme so that elements using this variant are accessible (fix(theme): update flat variant text colors to be accessible #3738)
The systematic lightening of shades and addition of dark mode colors was a deliberate enhancement to meet accessibility standards, not a potential issue.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any color contrast testing configurations or utilities in the codebase
rg -g '!node_modules' -g '!dist' -i 'contrast|wcag|a11y|accessibility'
# Look for any existing color token definitions that might be affected
fd -e ts -e js -e json . | xargs rg -l 'text-(primary|secondary|success|warning|danger)'
Length of output: 28717
Script:
#!/bin/bash
# Check color contrast testing implementation
cat packages/core/theme/__tests__/semantic-colors.test.ts
# Check the flat variant implementation in other components
rg -g '!node_modules' -g '!dist' 'flat.*variant' -A 5 -B 5
Length of output: 48307
packages/core/react/package.json (1)
88-88: LGTM! The drawer dependency is properly integrated.
The addition of @nextui-org/drawer follows the established pattern for component dependencies in the NextUI React core package.
Let's verify the drawer package setup:
✅ Verification successful
The drawer package is properly set up and integrated
The verification confirms that:
- The drawer package is correctly structured with proper package.json configuration
- All required peer dependencies are declared (@nextui-org/theme, @nextui-org/system)
- The component is properly implemented using the Modal component as base
- The package is correctly exported in the main NextUI React package
- Version 2.0.0 matches the PR context
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the drawer package configuration
# Test 1: Check if drawer package.json exists and has required fields
fd -t f "package.json" -p "packages/components/drawer" --exec cat {} \
| jq 'select(.name=="@nextui-org/drawer") | {name, version, peerDependencies, main, types, module}'
# Test 2: Verify exports in the main package
rg -A 5 "export \* from .*drawer" packages/core/react/src/index.ts
Length of output: 678
Script:
#!/bin/bash
# Let's fix the JSON parsing and try again with proper quoting
fd -t f "package.json" -p "packages/components/drawer" --exec cat {} \
| jq '{name, version, peerDependencies, main, types, module}'
# Also check if the drawer component exists and is properly structured
fd -t f -e ts -e tsx . -p "packages/components/drawer/src" --exec ls -la {}
Length of output: 716
Script:
#!/bin/bash
# Let's check the package.json content directly without jq
cat packages/components/drawer/package.json
# And examine the drawer component implementation
cat packages/components/drawer/src/drawer.tsx
cat packages/components/drawer/src/use-drawer.ts
Length of output: 4922
apps/docs/content/components/drawer/backdrop.ts (1)
124-132: LGTM!
The export configuration is properly structured for documentation purposes.
packages/core/theme/src/components/alert.ts (4)
4-4: LGTM: Good use of shared color utilities
The addition of colorVariants import promotes code reuse and consistency across components.
38-43: LGTM: Good refactoring of color variants
Empty color variant objects are appropriate here since the actual color classes are now managed by the colorVariants utility. This reduces duplication and makes color management more maintainable.
74-75: LGTM: Consistent with string-based approach
The hasDescription variant changes align well with the overall move to string-based class definitions.
Line range hint 90-229: Verify color consistency across variants
The refactoring to use colorVariants looks good and improves maintainability. However, let's verify that all color variants maintain consistent behavior.
✅ Verification successful
Color variants implementation is consistent and well-structured
The verification shows that colorVariants is consistently used across multiple components (alert, button, badge, etc.) and follows a unified pattern. The implementation in variants.ts exports a structured object with all necessary variant types (solid, shadow, bordered, flat, faded, light, ghost), which are consistently applied in the alert component's modifications.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify color consistency across components using colorVariants
# Test: Check if colorVariants is consistently used across other components
rg -l "colorVariants\." packages/core/theme/src/components/
Length of output: 509
Script:
#!/bin/bash
# Check the color variants implementation in one of the components to verify consistency
rg -A 5 "colorVariants\." packages/core/theme/src/components/button.ts
# Also check the source of colorVariants
fd colorVariants.ts packages/core/theme/src/
Length of output: 5263
Script:
#!/bin/bash
# Let's find and check the colorVariants implementation
rg -p "export const colorVariants" packages/core/theme/src/ -A 10
Length of output: 236
packages/components/modal/__tests__/modal.test.tsx (1)
Line range hint 142-191: Consider viewport scaling in drag calculations
The current tests use fixed pixel values which might not work correctly with different DPI settings. Consider using relative units or accounting for window.devicePixelRatio in the calculations.
apps/docs/content/docs/components/drawer.mdx (1)
1-12: LGTM! Clear and concise component introduction.
The component's metadata and description effectively communicate its purpose and functionality.
apps/docs/config/routes.json (1)
256-262: LGTM: Drawer component route properly configured
The new Drawer component route is well-structured with appropriate keywords and follows the established documentation patterns.
packages/components/drawer/src/use-drawer.ts (3)
1-9: Import statements are appropriate and necessary
The imported modules and types are correctly used throughout the file.
10-23: Props interface is well-defined and extends ModalProps correctly
The interface accurately defines the required properties for the drawer component, omitting specific modal props as intended.
27-97: useDrawer hook is implemented correctly and follows React best practices
The hook effectively manages the drawer's state and properties using useMemo and useCallback, ensuring optimal performance.
| import {forwardRef} from "@nextui-org/system"; | ||
| import {Modal} from "@nextui-org/modal"; | ||
|
|
||
| import {useDrawer, UseDrawerProps} from "./use-drawer"; | ||
|
|
||
| export interface DrawerProps extends UseDrawerProps { | ||
| children: React.ReactNode; | ||
| } |
There was a problem hiding this comment.
Add missing React import.
The code uses React.ReactNode type but React is not imported.
+import {type ReactNode} from "react";
import {forwardRef} from "@nextui-org/system";
import {Modal} from "@nextui-org/modal";
import {useDrawer, UseDrawerProps} from "./use-drawer";
export interface DrawerProps extends UseDrawerProps {
- children: React.ReactNode;
+ children: ReactNode;
}📝 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.
| import {forwardRef} from "@nextui-org/system"; | |
| import {Modal} from "@nextui-org/modal"; | |
| import {useDrawer, UseDrawerProps} from "./use-drawer"; | |
| export interface DrawerProps extends UseDrawerProps { | |
| children: React.ReactNode; | |
| } | |
| import {type ReactNode} from "react"; | |
| import {forwardRef} from "@nextui-org/system"; | |
| import {Modal} from "@nextui-org/modal"; | |
| import {useDrawer, UseDrawerProps} from "./use-drawer"; | |
| export interface DrawerProps extends UseDrawerProps { | |
| children: ReactNode; | |
| } |
| <Drawer | ||
| isOpen={isOpen} | ||
| onOpenChange={onOpenChange} | ||
| isDismissable={false} | ||
| isKeyboardDismissDisabled={true} | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility considerations for non-dismissable drawers.
While the non-dismissable configuration serves its purpose, it's important to note the accessibility implications. Consider adding an ARIA label and role to improve screen reader support, and document when to use this pattern.
<Drawer
isOpen={isOpen}
onOpenChange={onOpenChange}
isDismissable={false}
isKeyboardDismissDisabled={true}
+ aria-label="Non-dismissable drawer"
+ role="dialog"📝 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.
| <Drawer | |
| isOpen={isOpen} | |
| onOpenChange={onOpenChange} | |
| isDismissable={false} | |
| isKeyboardDismissDisabled={true} | |
| > | |
| <Drawer | |
| isOpen={isOpen} | |
| onOpenChange={onOpenChange} | |
| isDismissable={false} | |
| isKeyboardDismissDisabled={true} | |
| aria-label="Non-dismissable drawer" | |
| role="dialog" | |
| > |
| const drawer = tv({ | ||
| slots: { | ||
| base: ["absolute", "m-0", "sm:m-0", "overflow-y-auto"], | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add essential base styles for proper drawer functionality
The base styles are missing some crucial properties:
- z-index for proper stacking order
- Background color for visual separation
- Box-shadow for depth perception
const drawer = tv({
slots: {
- base: ["absolute", "m-0", "sm:m-0", "overflow-y-auto"],
+ base: [
+ "absolute",
+ "m-0",
+ "sm:m-0",
+ "overflow-y-auto",
+ "z-50",
+ "bg-background",
+ "shadow-lg"
+ ],
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const drawer = tv({ | |
| slots: { | |
| base: ["absolute", "m-0", "sm:m-0", "overflow-y-auto"], | |
| }, | |
| const drawer = tv({ | |
| slots: { | |
| base: [ | |
| "absolute", | |
| "m-0", | |
| "sm:m-0", | |
| "overflow-y-auto", | |
| "z-50", | |
| "bg-background", | |
| "shadow-lg" | |
| ], | |
| }, |
| import {VariantProps} from "tailwind-variants"; | ||
|
|
||
| import {tv} from "../utils/tv"; | ||
|
|
||
| /** | ||
| * Modal **Tailwind Variants** component | ||
| * | ||
| * @example | ||
| * ```js | ||
| * const {base} = drawer({...}) | ||
| * | ||
| * <div> | ||
| * <button>Open Drawer</button> | ||
| * <div className={base()}> | ||
| * Drawer Content | ||
| * </div> | ||
| * </div> | ||
| * ``` | ||
| */ | ||
| const drawer = tv({ | ||
| slots: { | ||
| base: ["absolute", "m-0", "sm:m-0", "overflow-y-auto"], | ||
| }, | ||
| variants: { | ||
| size: { | ||
| xs: { | ||
| base: "max-w-xs max-h-[20rem]", | ||
| }, | ||
| sm: { | ||
| base: "max-w-sm max-h-[24rem]", | ||
| }, | ||
| md: { | ||
| base: "max-w-md max-h-[28rem]", | ||
| }, | ||
| lg: { | ||
| base: "max-w-lg max-h-[32rem]", | ||
| }, | ||
| xl: { | ||
| base: "max-w-xl max-h-[36rem]", | ||
| }, | ||
| "2xl": { | ||
| base: "max-w-2xl max-h-[42rem]", | ||
| }, | ||
| "3xl": { | ||
| base: "max-w-3xl max-h-[48rem]", | ||
| }, | ||
| "4xl": { | ||
| base: "max-w-4xl max-h-[56rem]", | ||
| }, | ||
| "5xl": { | ||
| base: "max-w-5xl max-h-[64rem]", | ||
| }, | ||
| full: { | ||
| base: "max-w-full max-h-full h-[100dvh] !rounded-none", | ||
| }, | ||
| }, | ||
| placement: { | ||
| top: { | ||
| base: "inset-x-0 top-0 max-w-[none] rounded-t-none", | ||
| }, | ||
| right: { | ||
| base: "inset-y-0 right-0 max-h-[none] rounded-r-none", | ||
| }, | ||
| bottom: { | ||
| base: "inset-x-0 bottom-0 max-w-[none] rounded-b-none", | ||
| }, | ||
| left: { | ||
| base: "inset-y-0 left-0 max-h-[none] rounded-l-none", | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| export type DrawerVariants = VariantProps<typeof drawer>; | ||
|
|
||
| export {drawer}; |
There was a problem hiding this comment.
💡 Codebase verification
Theme integration needs to be enhanced
The drawer component requires updates to align with NextUI's theming system:
- Add semantic color tokens (e.g.,
bg-content1,text-foreground-500) instead of basic colors likebg-white - Include backdrop blur and overlay styles similar to the modal component
- Add hover and active states with proper color tokens
Example locations that need attention:
baseslot should use semantic background color token- Add proper color tokens for text and borders if needed
- Consider adding backdrop styles similar to modal's
bg-overlay/30withbackdrop-blur-md
🔗 Analysis chain
Verify theme integration with NextUI ecosystem
Please ensure that:
- The drawer component respects the NextUI theme tokens
- The styling approach is consistent with other NextUI components
- The component works correctly with NextUI's dark mode
Let me gather more specific information about the theme implementation patterns in NextUI components.
Let me fix the regex and try again to gather the necessary information.
Let me check one more aspect regarding the theme system implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check theme token usage and styling consistency
rg -l "theme|darkMode" packages/core/theme/src/components/
Length of output: 430
Script:
#!/bin/bash
# Check theme implementation patterns in similar components
rg -A 5 "tv\({" packages/core/theme/src/components/popover.ts packages/core/theme/src/components/modal.ts
# Look for dark mode handling patterns
ast-grep --pattern 'darkMode: {
$$$
}'
Length of output: 224
Script:
#!/bin/bash
# Check theme implementation patterns in similar components
rg "const \w+ = tv\(" -A 10 packages/core/theme/src/components/popover.ts packages/core/theme/src/components/modal.ts
# Check color and theme token usage
rg "colors|tokens" packages/core/theme/src/components/popover.ts packages/core/theme/src/components/modal.ts
# Check base styles implementation
rg "base: \[" -A 5 packages/core/theme/src/components/popover.ts packages/core/theme/src/components/modal.ts
Length of output: 2488
Script:
#!/bin/bash
# Check theme system implementation and dark mode handling
rg "bg-|text-|border-" packages/core/theme/src/components/modal.ts packages/core/theme/src/components/drawer.ts
# Check theme configuration
cat packages/core/theme/src/theme-provider.tsx
Length of output: 921
| <div className="flex flex-wrap gap-3"> | ||
| {sizes.map((size) => ( | ||
| <Button key={size} onPress={() => handleOpen(size)}>Open {size}</Button> | ||
| ))} | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance button accessibility
The buttons should have proper ARIA labels to improve accessibility for screen readers.
- <Button key={size} onPress={() => handleOpen(size)}>Open {size}</Button>
+ <Button
+ key={size}
+ onPress={() => handleOpen(size)}
+ aria-label={`Open ${size} size drawer`}
+ >
+ Open {size}
+ </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.
| <div className="flex flex-wrap gap-3"> | |
| {sizes.map((size) => ( | |
| <Button key={size} onPress={() => handleOpen(size)}>Open {size}</Button> | |
| ))} | |
| </div> | |
| <div className="flex flex-wrap gap-3"> | |
| {sizes.map((size) => ( | |
| <Button | |
| key={size} | |
| onPress={() => handleOpen(size)} | |
| aria-label={`Open ${size} size drawer`} | |
| > | |
| Open {size} | |
| </Button> | |
| ))} | |
| </div> |
| export default function App() { | ||
| const {isOpen, onOpen, onOpenChange} = useDisclosure(); | ||
| const [backdrop, setBackdrop] = React.useState("opaque"); | ||
|
|
||
| const backdrops = ["opaque", "blur", "transparent"]; | ||
|
|
||
| const handleBackdropChange = (backdrop) => { | ||
| setBackdrop(backdrop); | ||
| onOpen(); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding state management for form data and loading state.
The current implementation lacks form state management and loading states for async operations.
Example implementation:
const [formData, setFormData] = React.useState({ email: '', password: '' });
const [isLoading, setIsLoading] = React.useState(false);
const handleSignIn = async () => {
setIsLoading(true);
try {
// Handle sign in
} catch (error) {
// Handle error
} finally {
setIsLoading(false);
}
};| <Input | ||
| autoFocus | ||
| endContent={ | ||
| <MailIcon className="text-2xl text-default-400 pointer-events-none flex-shrink-0" /> | ||
| } | ||
| label="Email" | ||
| placeholder="Enter your email" | ||
| variant="bordered" | ||
| /> | ||
| <Input | ||
| endContent={ | ||
| <LockIcon className="text-2xl text-default-400 pointer-events-none flex-shrink-0" /> | ||
| } | ||
| label="Password" | ||
| placeholder="Enter your password" | ||
| type="password" | ||
| variant="bordered" | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance form inputs with validation and error handling.
The email and password inputs should include:
- Form validation
- Error messages
- Proper aria-labels
Example improvement:
<Input
autoFocus
required
type="email"
label="Email"
placeholder="Enter your email"
variant="bordered"
errorMessage={errors.email}
aria-label="Email input"
onChange={(e) => setFormData({ ...formData, email: e.target.value })}
endContent={<MailIcon className="text-2xl text-default-400 pointer-events-none flex-shrink-0" />}
/>| <DrawerFooter> | ||
| <Button color="danger" variant="flat" onPress={onClose}> | ||
| Close | ||
| </Button> | ||
| <Button color="primary" onPress={onClose}> | ||
| Sign in | ||
| </Button> | ||
| </DrawerFooter> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Separate close and sign-in actions with proper loading state.
The current implementation uses the same onClose callback for both actions without proper loading state handling.
Example improvement:
<DrawerFooter>
<Button color="danger" variant="flat" onPress={onClose}>
Close
</Button>
<Button
color="primary"
isLoading={isLoading}
onPress={handleSignIn}
>
Sign in
</Button>
</DrawerFooter>| const content = ( | ||
| <DrawerContent> | ||
| {(onClose) => ( | ||
| <> | ||
| <DrawerHeader className="flex flex-col gap-1">Log in</DrawerHeader> | ||
| <DrawerBody> | ||
| <Input | ||
| autoFocus | ||
| endContent={ | ||
| <MailFilledIcon className="text-2xl text-default-400 pointer-events-none flex-shrink-0" /> | ||
| } | ||
| label="Email" | ||
| placeholder="Enter your email" | ||
| variant="bordered" | ||
| /> | ||
| <Input | ||
| endContent={ | ||
| <LockFilledIcon className="text-2xl text-default-400 pointer-events-none flex-shrink-0" /> | ||
| } | ||
| label="Password" | ||
| placeholder="Enter your password" | ||
| type="password" | ||
| variant="bordered" | ||
| /> | ||
| <div className="flex py-2 px-1 justify-between"> | ||
| <Checkbox | ||
| classNames={{ | ||
| label: "text-sm", | ||
| }} | ||
| > | ||
| Remember me | ||
| </Checkbox> | ||
| <Link color="primary" href="#" size="sm"> | ||
| Forgot password? | ||
| </Link> | ||
| </div> | ||
| </DrawerBody> | ||
| <DrawerFooter> | ||
| <Button color="danger" variant="flat" onPress={onClose}> | ||
| Close | ||
| </Button> | ||
| <Button color="primary" onPress={onClose}> | ||
| Sign in | ||
| </Button> | ||
| </DrawerFooter> | ||
| </> | ||
| )} | ||
| </DrawerContent> | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance accessibility and form handling.
The login form implementation could be improved in several ways:
- Add
aria-labelto the DrawerContent for screen readers - Wrap the inputs in a
<form>element for proper form submission - Add form validation and error handling
const content = (
- <DrawerContent>
+ <DrawerContent aria-label="Login form">
{(onClose) => (
<>
<DrawerHeader className="flex flex-col gap-1">Log in</DrawerHeader>
<DrawerBody>
+ <form onSubmit={(e) => {
+ e.preventDefault();
+ // Handle form submission
+ }}>
<Input
autoFocus
endContent={
<MailFilledIcon className="text-2xl text-default-400 pointer-events-none flex-shrink-0" />
}
label="Email"
placeholder="Enter your email"
variant="bordered"
+ required
+ type="email"
/>
{/* ... rest of the form ... */}
+ </form>
</DrawerBody>Committable suggestion skipped: line range outside the PR's diff.
| The drawer can be placed on any edge of the screen using the `placement` prop: | ||
| - `left` (default) | ||
| - `right` | ||
| - `top` | ||
| - `bottom` |
There was a problem hiding this comment.
Fix inconsistent default placement value.
There's a discrepancy in the documentation regarding the default placement:
- Line 87 states
leftis the default - The props table on line 158 shows
rightas the default
Please verify and update to show the correct default value consistently throughout the documentation.
Also applies to: 158-158
Base PR: #2223
📝 Description
⛳️ Current behavior (updates)
🚀 New behavior
💣 Is this a breaking change (Yes/No):
📝 Additional Information
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation