Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces a new Drawer component built on vaul library primitives, featuring multiple subcomponents (Trigger, Portal, Overlay, Close, Content, Header, Footer, Title, Description). The implementation includes configuration registration, two example variants, comprehensive documentation, registry entries, and a vaul dependency addition. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
public/r/bar-chart.json (1)
13-13: Consider renaming stackId from "strokeId" to "stack".The stackId value is named
"strokeId", which is misleading since it controls stacking behavior rather than stroke styling. Consider using a more descriptive name like"stack"or"stackGroup".🔎 Proposed refactor
- stackId={stacked ? "strokeId" : undefined} /> + stackId={stacked ? "stack" : undefined} />components/retroui/Drawer.tsx (1)
48-73: Minor redundancy and UX consideration.
Line 54: The
data-slot="drawer-portal"prop is redundant sinceDrawerPortalalready sets this attribute internally (line 23).Line 68: The draggable handle indicator only appears for bottom-direction drawers. Confirm this is intentional, as users might expect visual affordances for dragging in other directions as well.
🔎 Proposed fix for the redundant prop
- <DrawerPortal data-slot="drawer-portal"> + <DrawerPortal> <DrawerOverlay />preview/components/drawer-style-right-direction.tsx (1)
20-29: Consider adding form submission handler.The form element lacks an
onSubmithandler, and the Submit button (line 32) is outside the form element. For proper form functionality and accessibility, either move the Submit button inside the form or add anonSubmithandler with proper form validation.🔎 Proposed refactor to properly handle form submission
- <form className="flex flex-col space-y-4 p-4"> + <form className="flex flex-col space-y-4 p-4" onSubmit={(e) => { e.preventDefault(); /* handle form submission */ }}> <div className="flex flex-col space-y-2"> <Label>Email</Label> <Input placeholder="Enter your email" /> </div> <div className="flex flex-col space-y-2"> <Label>Comment</Label> <Textarea rows={4} placeholder="Enter your comment" /> </div> + <div className="flex justify-start gap-3"> + <Button type="submit">Submit</Button> + <Drawer.Close asChild> + <Button variant="outline" type="button">Cancel</Button> + </Drawer.Close> + </div> </form> - <Drawer.Footer> - <div className="flex justify-start gap-3"> - <Button>Submit</Button> - <Drawer.Close> - <Button variant="outline">Cancel</Button> - </Drawer.Close> - </div> - </Drawer.Footer>preview/components/drawer-style-default.tsx (1)
9-14: Clarify the semantic relationship between trigger and drawer content.The trigger button labeled "Submit" opens a confirmation drawer asking "Are you absolutely sure?", which creates a confusing flow. Typically, a confirmation drawer appears after a submit action, not as the primary trigger. Consider either:
- Renaming the trigger to better reflect it opens a confirmation dialog (e.g., "Open Drawer" or "Show Confirmation")
- Or, if this is meant to demonstrate a confirmation pattern, add a comment explaining the intended use case
🔎 Suggested improvements for clarity
Option 1: Rename trigger to be more descriptive
<Drawer.Trigger asChild> - <Button>Submit</Button> + <Button>Delete Account</Button> </Drawer.Trigger> <Drawer.Content> <Drawer.Header> - <Drawer.Title>Are you absolutely sure?</Drawer.Title> - <Drawer.Description>This action cannot be undone.</Drawer.Description> + <Drawer.Title>Delete Account</Drawer.Title> + <Drawer.Description>Are you absolutely sure? This action cannot be undone.</Drawer.Description> </Drawer.Header>Option 2: Make it a generic example
<Drawer.Trigger asChild> - <Button>Submit</Button> + <Button>Open Drawer</Button> </Drawer.Trigger>config/components.ts (1)
272-281: Consider relocating drawer examples for better organization.The drawer examples are inserted between bar-chart and button examples. While this might be intentional, most other component examples in this file appear to be grouped or alphabetically sorted. Consider moving these entries to be near the dialog examples (around line 512+) for better consistency and maintainability.
This is a minor organizational suggestion and doesn't affect functionality. The current placement works correctly.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
components/retroui/Drawer.tsxconfig/components.tsconfig/navigation.tscontent/docs/components/drawer.mdxpackage.jsonpreview/components/drawer-style-default.tsxpreview/components/drawer-style-right-direction.tsxpublic/r/bar-chart.jsonpublic/r/drawer.jsonpublic/r/registry.jsonregistry.json
🧰 Additional context used
🧬 Code graph analysis (4)
preview/components/drawer-style-right-direction.tsx (4)
components/retroui/Button.tsx (1)
Button(40-63)components/retroui/Label.tsx (1)
Label(11-16)components/retroui/Input.tsx (1)
Input(7-25)components/retroui/Textarea.tsx (1)
Textarea(4-21)
preview/components/drawer-style-default.tsx (1)
components/retroui/Button.tsx (1)
Button(40-63)
config/navigation.ts (2)
components/SideNav.tsx (3)
SideNav(12-53)item(21-49)child(25-46)components/TopNav.tsx (1)
TopNav(11-93)
components/retroui/Drawer.tsx (1)
lib/utils.ts (1)
cn(4-6)
🔇 Additional comments (12)
components/retroui/Drawer.tsx (5)
1-18: LGTM!The imports and basic wrapper components follow good patterns. The "use client" directive is appropriate for interactive components, and the consistent use of data-slot attributes aids component identification.
20-46: LGTM!The DrawerPortal, DrawerClose, and DrawerOverlay components are well-structured. The overlay animations and positioning are appropriate for a modal-style component.
75-96: LGTM!The DrawerHeader and DrawerFooter components are well-implemented with appropriate responsive styling and layout considerations.
98-122: LGTM!The DrawerTitle and DrawerDescription components properly wrap vaul primitives while applying consistent RetroUI typography styles.
124-136: LGTM!The composite component pattern using Object.assign provides a clean API surface, allowing ergonomic usage like
<Drawer.Trigger>,<Drawer.Content>, etc. This pattern is consistent with modern React component libraries.public/r/registry.json (1)
218-233: LGTM!The registry entry for the Drawer component is properly structured with correct dependencies, file paths, and metadata.
content/docs/components/drawer.mdx (1)
9-46: LGTM!The documentation structure, installation instructions, and examples are well-organized and provide clear guidance for users implementing the Drawer component.
config/navigation.ts (1)
55-56: LGTM!The navigation updates correctly remove the "New" tag from Calendar and Carousel components while adding the Drawer entry with appropriate "New" tagging. The alphabetical ordering is maintained.
Also applies to: 60-60
package.json (1)
60-60: No action required. The vaul package version ^1.1.2 is the latest stable release as of January 2026 with no known security vulnerabilities.registry.json (1)
266-281: LGTM!The registry entry for the Drawer component is well-structured and consistent with other component entries. The dependency on
vaulis correctly specified, and the description accurately describes the component's functionality.config/components.ts (1)
83-86: LGTM!The core drawer configuration entry is correctly structured and placed in alphabetical order after "dialog".
public/r/drawer.json (1)
1-18: LGTM!The public registry file for the Drawer component is correctly structured with all required fields. The inline component implementation follows the established pattern used by the vaul library, properly composing all necessary subcomponents (Trigger, Portal, Overlay, Close, Content, Header, Footer, Title, Description) and exposing them as a single Drawer object.
| --- | ||
| title: Drawer | ||
| description: A component that can slide in from any side of the screen. | ||
| lastUpdated: 21 Aug, 2025 | ||
| links: | ||
| source: https://github.com/Logging-Stuff/RetroUI/blob/main/components/retroui/Command.tsx | ||
| --- |
There was a problem hiding this comment.
Fix incorrect source link and review the lastUpdated date.
-
Line 6: The source link incorrectly points to
Command.tsxinstead ofDrawer.tsx. This will mislead users trying to view the component source. -
Line 4: The
lastUpdateddate is "21 Aug, 2025", which is in the past. If this component is being newly added in January 2026, this date should be updated to reflect the current date.
🔎 Proposed fix for the source link
links:
- source: https://github.com/Logging-Stuff/RetroUI/blob/main/components/retroui/Command.tsx
+ source: https://github.com/Logging-Stuff/RetroUI/blob/main/components/retroui/Drawer.tsx📝 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.
| --- | |
| title: Drawer | |
| description: A component that can slide in from any side of the screen. | |
| lastUpdated: 21 Aug, 2025 | |
| links: | |
| source: https://github.com/Logging-Stuff/RetroUI/blob/main/components/retroui/Command.tsx | |
| --- | |
| --- | |
| title: Drawer | |
| description: A component that can slide in from any side of the screen. | |
| lastUpdated: 21 Aug, 2025 | |
| links: | |
| source: https://github.com/Logging-Stuff/RetroUI/blob/main/components/retroui/Drawer.tsx | |
| --- |
🤖 Prompt for AI Agents
In content/docs/components/drawer.mdx around lines 1 to 7, the frontmatter has
an incorrect source link and an outdated lastUpdated date; update the
links.source value to point to the Drawer component (replace Command.tsx with
Drawer.tsx in the GitHub path) and set lastUpdated to the current date (e.g., 4
Jan, 2026) so the metadata correctly reflects the file and when it was last
updated.
| <Button>Review Us</Button> | ||
| </Drawer.Trigger> | ||
| <Drawer.Content> | ||
| <Drawer.Header> | ||
| <Drawer.Title>Edit Profile</Drawer.Title> | ||
| <Drawer.Description>Make changes to your profile here. Click save when you're done.</Drawer.Description> |
There was a problem hiding this comment.
Semantic mismatch between trigger text and drawer content.
The trigger button says "Review Us" but the drawer title is "Edit Profile" with a description about making profile changes. This creates a confusing user experience where the button label doesn't match the action being performed.
🔎 Suggested fix to align the trigger text with the drawer content
<Drawer.Trigger asChild>
- <Button>Review Us</Button>
+ <Button>Edit Profile</Button>
</Drawer.Trigger>📝 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.
| <Button>Review Us</Button> | |
| </Drawer.Trigger> | |
| <Drawer.Content> | |
| <Drawer.Header> | |
| <Drawer.Title>Edit Profile</Drawer.Title> | |
| <Drawer.Description>Make changes to your profile here. Click save when you're done.</Drawer.Description> | |
| <Drawer.Trigger asChild> | |
| <Button>Edit Profile</Button> | |
| </Drawer.Trigger> | |
| <Drawer.Content> | |
| <Drawer.Header> | |
| <Drawer.Title>Edit Profile</Drawer.Title> | |
| <Drawer.Description>Make changes to your profile here. Click save when you're done.</Drawer.Description> |
🤖 Prompt for AI Agents
In preview/components/drawer-style-right-direction.tsx around lines 12–17, the
Drawer.Trigger button text ("Review Us") does not match the Drawer.Content
(title "Edit Profile" and profile-edit description); change the trigger label to
"Edit Profile" (or the appropriate localized string) so the action matches the
drawer content, update any i18n keys if used, and verify the rendered button,
aria-labels and visual layout still read correctly in the UI.
| { | ||
| "path": "components/retroui/charts/BarChart.tsx", | ||
| "content": "\"use client\"\n\nimport { cn } from \"@/lib/utils\"\nimport React from \"react\"\nimport {\n Bar,\n BarChart as RechartsBarChart,\n CartesianGrid,\n ResponsiveContainer,\n Tooltip,\n XAxis,\n YAxis,\n} from \"recharts\"\n\ninterface BarChartProps extends React.HTMLAttributes<HTMLDivElement> {\n data: Record<string, any>[]\n index: string\n categories: string[]\n strokeColors?: string[]\n fillColors?: string[]\n tooltipBgColor?: string\n tooltipBorderColor?: string\n gridColor?: string\n valueFormatter?: (value: number) => string\n showGrid?: boolean\n showTooltip?: boolean\n alignment?: \"vertical\" | \"horizontal\"\n className?: string\n}\n\nconst BarChart = React.forwardRef<HTMLDivElement, BarChartProps>(\n (\n {\n data = [],\n index,\n categories = [],\n strokeColors = [\"var(--foreground)\"],\n fillColors = [\"var(--primary)\"],\n tooltipBgColor = \"var(--background)\",\n tooltipBorderColor = \"var(--border)\",\n gridColor = \"var(--muted)\",\n valueFormatter = (value: number) => value.toString(),\n showGrid = true,\n showTooltip = true,\n alignment = \"vertical\",\n className,\n ...props\n },\n ref\n ) => {\n return (\n <div ref={ref} className={cn(\"h-80 w-full\", className)} {...props}>\n <ResponsiveContainer width=\"100%\" height=\"100%\">\n <RechartsBarChart \n data={data} \n layout={alignment === \"horizontal\" ? \"vertical\" : undefined}\n margin={{ top: 10, right: 30, left: 0, bottom: 0 }}\n >\n {showGrid && (\n <CartesianGrid strokeDasharray=\"3 3\" stroke={gridColor} />\n )}\n \n {alignment === \"horizontal\" ? (\n <>\n <XAxis \n type=\"number\"\n axisLine={false}\n tickLine={false}\n className=\"text-xs fill-muted-foreground\"\n tickFormatter={valueFormatter}\n />\n \n <YAxis\n type=\"category\"\n dataKey={index}\n axisLine={false}\n tickLine={false}\n className=\"text-xs fill-muted-foreground\"\n width={80}\n />\n </>\n ) : (\n <>\n <XAxis \n dataKey={index}\n axisLine={false}\n tickLine={false}\n className=\"text-xs fill-muted-foreground\"\n />\n \n <YAxis\n axisLine={false}\n tickLine={false}\n className=\"text-xs fill-muted-foreground\"\n tickFormatter={valueFormatter}\n />\n </>\n )}\n \n {showTooltip && (\n <Tooltip\n content={({ active, payload, label }) => {\n if (!active || !payload?.length) return null\n \n return (\n <div \n className=\"border p-2 shadow\"\n style={{ \n backgroundColor: tooltipBgColor,\n borderColor: tooltipBorderColor \n }}\n >\n <div className=\"grid grid-cols-2 gap-2\">\n <div className=\"flex flex-col\">\n <span className=\"text-[0.70rem] uppercase text-muted-foreground\">\n {index}\n </span>\n <span className=\"font-bold text-muted-foreground\">\n {label}\n </span>\n </div>\n {payload.map((entry, index) => (\n <div key={index} className=\"flex flex-col\">\n <span className=\"text-[0.70rem] uppercase text-muted-foreground\">\n {entry.dataKey}\n </span>\n <span className=\"font-bold\" style={{ color: strokeColors[0] }}>\n {valueFormatter(entry.value as number)}\n </span>\n </div>\n ))}\n </div>\n </div>\n )\n }}\n />\n )}\n \n {categories.map((category, index) => {\n const fillColor = fillColors[index] || fillColors[0]\n const strokeColor = strokeColors[index] || strokeColors[0]\n \n return (\n <Bar\n key={category}\n dataKey={category}\n fill={fillColor}\n stroke={strokeColor}\n strokeWidth={1}\n />\n )\n })}\n </RechartsBarChart>\n </ResponsiveContainer>\n </div>\n )\n }\n)\n\nBarChart.displayName = \"BarChart\"\n\nexport { BarChart, type BarChartProps }", | ||
| "content": "\"use client\"\n\nimport { cn } from \"@/lib/utils\"\nimport React from \"react\"\nimport {\n Bar,\n BarChart as RechartsBarChart,\n CartesianGrid,\n ResponsiveContainer,\n Tooltip,\n XAxis,\n YAxis,\n} from \"recharts\"\n\ninterface BarChartProps extends React.HTMLAttributes<HTMLDivElement> {\n data: Record<string, any>[]\n index: string\n categories: string[]\n strokeColors?: string[]\n fillColors?: string[]\n tooltipBgColor?: string\n tooltipBorderColor?: string\n gridColor?: string\n valueFormatter?: (value: number) => string\n showGrid?: boolean\n showTooltip?: boolean\n stacked?: boolean\n alignment?: \"vertical\" | \"horizontal\"\n className?: string\n}\n\nconst BarChart = React.forwardRef<HTMLDivElement, BarChartProps>(\n (\n {\n data = [],\n index,\n categories = [],\n strokeColors = [\"var(--foreground)\"],\n fillColors = [\"var(--primary)\", \"var(--secondary)\"],\n tooltipBgColor = \"var(--background)\",\n tooltipBorderColor = \"var(--border)\",\n gridColor = \"var(--muted)\",\n valueFormatter = (value: number) => value.toString(),\n showGrid = true,\n showTooltip = true,\n stacked = false,\n alignment = \"vertical\",\n className,\n ...props\n },\n ref\n ) => {\n return (\n <div ref={ref} className={cn(\"h-80 w-full\", className)} {...props}>\n <ResponsiveContainer width=\"100%\" height=\"100%\">\n <RechartsBarChart\n data={data}\n layout={alignment === \"horizontal\" ? \"vertical\" : undefined}\n margin={{ top: 10, right: 30, left: 0, bottom: 0 }}\n >\n {showGrid && (\n <CartesianGrid strokeDasharray=\"3 3\" stroke={gridColor} />\n )}\n\n {alignment === \"horizontal\" ? (\n <>\n <XAxis\n type=\"number\"\n axisLine={false}\n tickLine={false}\n className=\"text-xs fill-muted-foreground\"\n tickFormatter={valueFormatter}\n />\n\n <YAxis\n type=\"category\"\n dataKey={index}\n axisLine={false}\n tickLine={false}\n className=\"text-xs fill-muted-foreground\"\n width={80}\n />\n </>\n ) : (\n <>\n <XAxis\n dataKey={index}\n axisLine={false}\n tickLine={false}\n className=\"text-xs fill-muted-foreground\"\n />\n\n <YAxis\n axisLine={false}\n tickLine={false}\n className=\"text-xs fill-muted-foreground\"\n tickFormatter={valueFormatter}\n />\n </>\n )}\n\n {showTooltip && (\n <Tooltip\n content={({ active, payload, label }) => {\n if (!active || !payload?.length) return null\n\n return (\n <div\n className=\"border p-2 shadow\"\n style={{\n backgroundColor: tooltipBgColor,\n borderColor: tooltipBorderColor\n }}\n >\n <div className=\"grid grid-cols-2 gap-2\">\n <div className=\"flex flex-col\">\n <span className=\"text-[0.70rem] uppercase text-muted-foreground\">\n {index}\n </span>\n <span className=\"font-bold text-muted-foreground\">\n {label}\n </span>\n </div>\n {payload.map((entry, index) => (\n <div key={index} className=\"flex flex-col\">\n <span className=\"text-[0.70rem] uppercase text-muted-foreground\">\n {entry.dataKey}\n </span>\n <span className=\"font-bold\" style={{ color: strokeColors[0] }}>\n {valueFormatter(entry.value as number)}\n </span>\n </div>\n ))}\n </div>\n </div>\n )\n }}\n />\n )}\n\n {categories.map((category, index) => {\n const fillColor = fillColors[index] || fillColors[0]\n const strokeColor = strokeColors[index] || strokeColors[0]\n\n return (\n <Bar\n key={category}\n dataKey={category}\n fill={fillColor}\n stroke={strokeColor}\n strokeWidth={1}\n stackId={stacked ? \"strokeId\" : undefined} />\n )\n })}\n </RechartsBarChart>\n </ResponsiveContainer>\n </div>\n )\n }\n)\n\nBarChart.displayName = \"BarChart\"\n\nexport { BarChart, type BarChartProps }", |
There was a problem hiding this comment.
Fix tooltip color display for multi-category and stacked charts.
The tooltip renders all entry values with strokeColors[0], which displays incorrect colors for charts with multiple categories or stacked bars. Each entry should use its own color.
🔎 Proposed fix
In the tooltip content callback, update the value span to use the entry-specific color:
- <span className="font-bold" style={{ color: strokeColors[0] }}>
+ <span className="font-bold" style={{ color: entry.stroke || entry.fill }}>
{valueFormatter(entry.value as number)}
</span>Recharts populates entry.stroke and entry.fill with the actual colors used for each bar, ensuring the tooltip displays the correct color for each category.
🤖 Prompt for AI Agents
In public/r/bar-chart.json around line 13, the tooltip always uses
strokeColors[0] for every payload entry causing wrong colors for
multi-category/stacked charts; update the tooltip value span to use the actual
color from the payload entry (use entry.stroke or entry.fill provided by
Recharts, with a fallback to strokeColors[index] or fillColors[index]) so each
tooltip item displays its correct category color.
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.