-
-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Storybook to store components #1012
base: main
Are you sure you want to change the base?
Conversation
…nother package since it breaks Storybook
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
📝 WalkthroughWalkthroughThis pull request introduces Storybook to the UI workspace, setting up a comprehensive component library documentation system. The changes include configuring Storybook, adding configuration files, creating story files for various UI components, and updating package scripts to support Storybook development and building. Changes
Assessment against linked issues
✨ Finishing Touches
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
🧹 Nitpick comments (31)
ui/src/logos/spinner-icon.tsx (1)
14-14
: Consider making animation duration configurable.The animation speed is hardcoded via Tailwind's
animate-spin
. Consider making it configurable through props for different use cases.-className={twMerge('h-5 w-5 animate-spin', className)} +className={twMerge('h-5 w-5', animationSpeed === 'fast' ? 'animate-spin-fast' : 'animate-spin', className)}ui/src/stories/alert.stories.tsx (1)
14-86
: Consider enhancing stories with additional features.The stories provide good coverage of component variants. Consider these enhancements:
- Add args configuration for interactive controls
- Include accessibility examples (ARIA labels, keyboard navigation)
- Add responsive design examples
- Demonstrate error handling scenarios
Example args configuration:
export const Primary: Story = { args: { variant: 'primary', title: 'Primary Alert', description: 'This is a primary alert with an icon.', showIcon: true, }, render: (args) => ( <Alert variant={args.variant}> {args.showIcon && <InformationCircleIcon className="h-4 w-4" />} <AlertTitle>{args.title}</AlertTitle> <AlertDescription>{args.description}</AlertDescription> </Alert> ), };ui/.storybook/preview.js (1)
4-13
: Consider enhancing preview configuration with additional parameters.Add viewport and theme configurations to improve story development experience.
const preview = { parameters: { + viewport: { + viewports: { + mobile: { name: 'Mobile', styles: { width: '360px', height: '640px' } }, + tablet: { name: 'Tablet', styles: { width: '768px', height: '1024px' } }, + desktop: { name: 'Desktop', styles: { width: '1280px', height: '800px' } }, + }, + }, + themes: { + default: 'light', + list: [ + { name: 'light', class: 'light-theme', color: '#ffffff' }, + { name: 'dark', class: 'dark-theme', color: '#000000' }, + ], + }, controls: {ui/.storybook/main.js (1)
14-18
: Add useful Storybook addons for enhanced development.Consider adding a11y and viewport addons for better component testing.
addons: [ getAbsolutePath('@storybook/addon-onboarding'), getAbsolutePath('@storybook/addon-essentials'), getAbsolutePath('@storybook/addon-interactions'), + getAbsolutePath('@storybook/addon-a11y'), + getAbsolutePath('@storybook/addon-viewport'), ],ui/src/stories/avatar.stories.tsx (1)
4-8
: Add component description and usage guidelines.Enhance documentation with component description and usage examples.
const meta = { title: 'Components/Avatar', component: Avatar, tags: ['autodocs'], + parameters: { + docs: { + description: { + component: 'Avatar component for displaying user profile images with fallback support.', + }, + }, + }, } satisfies Meta<typeof Avatar>;ui/src/stories/accordion.stories.tsx (1)
20-20
: Make accordion width responsive.Replace hardcoded width with responsive classes.
- className: 'w-[400px]', + className: 'w-full max-w-md', - <Accordion type="single" collapsible className="w-[400px]"> + <Accordion type="single" collapsible className="w-full max-w-md"> - className: 'w-[400px]', + className: 'w-full max-w-md', - <Accordion type="multiple" className="w-[400px]"> + <Accordion type="multiple" className="w-full max-w-md">Also applies to: 23-23, 47-47, 50-50
ui/src/stories/badge.stories.tsx (1)
64-74
: Consider using CSS variables for colors.Using direct Tailwind color classes like
bg-blue-500
makes theme customization harder. Consider using CSS variables for better maintainability.-<Badge className="bg-blue-500 hover:bg-blue-600">Custom Blue</Badge> +<Badge className="bg-[var(--badge-color)] hover:bg-[var(--badge-hover-color)]">Custom Blue</Badge>ui/src/stories/progress.stories.tsx (1)
46-67
: Consider making animation speed configurable.The animation interval is hardcoded to 1000ms. Consider making it configurable through story controls.
+export const Animated: Story = { + args: { + interval: 1000, + }, render: ({ interval }) => { const [progress, setProgress] = React.useState(0); React.useEffect(() => { - const timer = setInterval(() => { + const timer = setInterval(() => { setProgress((prevProgress) => (prevProgress >= 100 ? 0 : prevProgress + 10)); - }, 1000); + }, interval);ui/src/stories/checkbox.stories.tsx (1)
91-102
: Simplify the checkbox state handling.The Boolean type casting and callback can be simplified.
-<Checkbox checked={checked} onCheckedChange={(checkedState) => setChecked(Boolean(checkedState))} /> +<Checkbox checked={checked} onCheckedChange={setChecked} />ui/src/stories/switch.stories.tsx (2)
54-57
: Add form submission handler implementation.The form submission handler is currently empty. Consider implementing the handler or adding a comment explaining that it's intentionally left empty for demonstration purposes.
onSubmit={(e) => { e.preventDefault(); - // Handle form submission + // This is a demo form submission handler + console.log('Form submitted'); }}
63-69
: Add aria-label to notification settings section.The notification settings section should be labeled for better accessibility.
-<div className="space-y-2"> +<div className="space-y-2" role="group" aria-label="Notification Settings">ui/src/stories/popover.stories.tsx (2)
48-58
: Add input validation and error handling to form fields.The form fields lack validation and error handling. Consider adding required fields and validation messages.
<input id="email" defaultValue="[email protected]" + required + pattern="[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,}$" className="border-input col-span-2 h-8 rounded-md border px-3" + aria-describedby="email-error" /> +<span id="email-error" className="text-destructive text-sm" />
69-70
: Consider adding loading states for delayed popovers.The custom delay implementation could benefit from a loading indicator during the delay period.
ui/src/stories/dialog.stories.tsx (1)
72-90
: Add form validation and aria-required attributes.The profile form fields should include proper validation attributes and aria labels.
<input id="name" + required + aria-required="true" className="col-span-3 h-10 rounded-md border px-3" placeholder="Enter your name" />ui/src/stories/carousel.stories.tsx (1)
108-116
: Add pause on hover for autoplay carousel.Consider adding a pause feature when users hover over the carousel for better user experience.
options={{ autoPlay: { enabled: true, delay: 3000, + pauseOnHover: true }, }}
ui/src/stories/radio-group.stories.tsx (1)
119-132
: Use design system tokens for custom styling.Instead of using direct color classes like
border-blue-500
, consider using design system tokens for consistent theming.-<RadioGroupItem value="option-1" id="custom-1" className="border-blue-500 text-blue-500" /> +<RadioGroupItem value="option-1" id="custom-1" className="border-primary text-primary" /> -<RadioGroupItem value="option-2" id="custom-2" className="border-green-500 text-green-500" /> +<RadioGroupItem value="option-2" id="custom-2" className="border-secondary text-secondary" />ui/src/stories/toggle.stories.tsx (1)
17-31
: Extract SVG icons into separate components.The SVG icons are duplicated across multiple stories. Consider extracting them into reusable components.
Create an icons directory and move the SVGs there:
// src/components/icons/italic-icon.tsx export const ItalicIcon = () => ( <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round" > <line x1="19" y1="4" x2="10" y2="4" /> <line x1="14" y1="20" x2="5" y2="20" /> <line x1="15" y1="4" x2="9" y2="20" /> </svg> );Also applies to: 40-54, 110-123, 126-140, 143-156
ui/src/stories/typography.stories.tsx (1)
6-23
: Import color tokens from design system.Instead of maintaining a separate FONT_COLORS array, consider importing these values from your design system tokens.
import { colorTokens } from '../theme/tokens'; const FONT_COLORS: FontColor[] = Object.keys(colorTokens) as FontColor[];ui/src/stories/tooltip.stories.tsx (2)
22-139
: Add ARIA labels for better accessibility.Consider adding aria-label attributes to buttons in the stories, especially for icon-only buttons.
-<button className="flex h-8 w-8 items-center justify-center rounded-full border"> +<button + className="flex h-8 w-8 items-center justify-center rounded-full border" + aria-label="Information">
140-187
: Document delay duration constraints.Consider adding a comment about minimum/maximum recommended delay durations for optimal user experience.
ui/src/stories/collapsible.stories.tsx (2)
44-50
: Optimize state management for multiple sections.Consider using a Set instead of an array for better performance with includes/filter operations.
-const [openSections, setOpenSections] = React.useState<number[]>([]); +const [openSections, setOpenSections] = React.useState<Set<number>>(new Set()); const toggleSection = (index: number) => { - setOpenSections((current) => - current.includes(index) ? current.filter((i) => i !== index) : [...current, index], - ); + setOpenSections((current) => { + const newSet = new Set(current); + current.has(index) ? newSet.delete(index) : newSet.add(index); + return newSet; + }); };
111-138
: Make animation duration configurable.Consider making the animation duration customizable through props for more flexibility.
ui/src/stories/menubar.stories.tsx (1)
1-171
: Document keyboard navigation patterns.Add comments or stories demonstrating keyboard navigation patterns (e.g., arrow keys, Enter, Escape) for better accessibility documentation.
ui/src/stories/hover-card.stories.tsx (1)
1-161
: Add hover delay configuration.Consider adding a delay prop to prevent accidental triggering, especially for interactive content.
-<HoverCard> +<HoverCard openDelay={200} closeDelay={150}>ui/src/stories/tabs.stories.tsx (2)
16-24
: Add aria-label to Basic tabs example.Enhance accessibility by adding descriptive aria-labels to the tabs component.
-<Tabs defaultValue="account"> +<Tabs defaultValue="account" aria-label="Account settings">
85-97
: Extract SVG icons and enhance accessibility.Consider extracting SVG icons into separate components and adding proper accessibility attributes.
-<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round"> +<svg aria-hidden="true" xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round">Also applies to: 101-114
ui/src/stories/dropdown-menu.stories.tsx (1)
58-69
: Add cross-platform keyboard shortcuts.Consider showing different shortcuts for Windows/Linux users.
-New Tab <DropdownMenuShortcut>⌘T</DropdownMenuShortcut> +New Tab <DropdownMenuShortcut>{isMac ? '⌘T' : 'Ctrl+T'}</DropdownMenuShortcut>ui/src/stories/form.stories.tsx (1)
161-215
: Add error boundary to form examples.Wrap form examples with error boundaries to handle runtime errors gracefully.
+import { ErrorBoundary } from 'react-error-boundary'; export const DifferentStates: Story = { render: () => ( + <ErrorBoundary fallback={<div>Something went wrong</div>}> <div className="space-y-8"> // ... existing code ... </div> + </ErrorBoundary> ), };ui/src/stories/navigation-menu.stories.tsx (3)
23-44
: Add accessibility improvements to ListItem componentConsider enhancing the component with aria-label and prop types validation.
const ListItem = React.forwardRef<React.ElementRef<'a'>, React.ComponentPropsWithoutRef<'a'>>( ({ className, title, children, ...props }, ref) => { return ( <li> <NavigationMenuLink asChild> <a ref={ref} + aria-label={title} className={cn( 'hover:bg-accent hover:text-accent-foreground focus:bg-accent focus:text-accent-foreground block select-none space-y-1 rounded-md p-3 leading-none no-underline outline-none transition-colors', className, )} {...props} >
46-70
: Add aria-label to navigation menuEnhance accessibility by adding an aria-label to the navigation menu.
- <NavigationMenu> + <NavigationMenu aria-label="Main navigation">
160-180
: Add documentation for custom styling approachConsider adding comments to explain the custom styling methodology for better maintainability.
// Custom Styled +// This story demonstrates how to override default styles using Tailwind classes +// primary and primary-foreground colors are defined in the theme configuration export const CustomStyled: Story = {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (35)
package.json
(1 hunks)ui/.gitignore
(1 hunks)ui/.storybook/main.js
(1 hunks)ui/.storybook/preview.js
(1 hunks)ui/package.json
(2 hunks)ui/src/components/button.tsx
(1 hunks)ui/src/logos/spinner-icon.tsx
(1 hunks)ui/src/stories/accordion.stories.tsx
(1 hunks)ui/src/stories/alert.stories.tsx
(1 hunks)ui/src/stories/avatar.stories.tsx
(1 hunks)ui/src/stories/badge.stories.tsx
(1 hunks)ui/src/stories/button.stories.tsx
(1 hunks)ui/src/stories/card.stories.tsx
(1 hunks)ui/src/stories/carousel.stories.tsx
(1 hunks)ui/src/stories/checkbox.stories.tsx
(1 hunks)ui/src/stories/collapsible.stories.tsx
(1 hunks)ui/src/stories/dialog.stories.tsx
(1 hunks)ui/src/stories/dropdown-menu.stories.tsx
(1 hunks)ui/src/stories/form.stories.tsx
(1 hunks)ui/src/stories/hover-card.stories.tsx
(1 hunks)ui/src/stories/input.stories.tsx
(1 hunks)ui/src/stories/label.stories.tsx
(1 hunks)ui/src/stories/menubar.stories.tsx
(1 hunks)ui/src/stories/navigation-menu.stories.tsx
(1 hunks)ui/src/stories/popover.stories.tsx
(1 hunks)ui/src/stories/progress.stories.tsx
(1 hunks)ui/src/stories/radio-group.stories.tsx
(1 hunks)ui/src/stories/select.stories.tsx
(1 hunks)ui/src/stories/separator.stories.tsx
(1 hunks)ui/src/stories/switch.stories.tsx
(1 hunks)ui/src/stories/table.stories.tsx
(1 hunks)ui/src/stories/tabs.stories.tsx
(1 hunks)ui/src/stories/toggle.stories.tsx
(1 hunks)ui/src/stories/tooltip.stories.tsx
(1 hunks)ui/src/stories/typography.stories.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- ui/.gitignore
- ui/src/components/button.tsx
🔇 Additional comments (14)
ui/src/logos/spinner-icon.tsx (3)
1-2
: LGTM! Imports are clean and necessary.
4-4
: LGTM! Component signature is well-typed.
17-22
: Verify licensing requirements for copied SVG.Since this SVG was copied from another site, please verify and document any licensing or attribution requirements.
ui/src/stories/alert.stories.tsx (2)
1-13
: LGTM! Well-structured Storybook configuration.The meta configuration and imports are properly set up following Storybook best practices.
3-3
: Verify Alert component exports.Let's ensure the Alert component and its subcomponents are properly exported.
✅ Verification successful
Alert component imports are correct
The Alert components are properly exported from
ui/src/components/alert.tsx
and match the imports in the stories file.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if Alert components are properly exported ast-grep --pattern 'export { Alert, AlertDescription, AlertTitle }' ui/src/components/alert/index.tsLength of output: 182
Script:
#!/bin/bash # Find alert-related files fd alert.ts -t f fd alert/index.ts -t fLength of output: 71
Script:
#!/bin/bash # Check exports in alert.tsx ast-grep --pattern 'export function Alert' ui/src/components/alert.tsx ast-grep --pattern 'export function AlertDescription' ui/src/components/alert.tsx ast-grep --pattern 'export function AlertTitle' ui/src/components/alert.tsxLength of output: 234
Script:
#!/bin/bash # View the content of alert.tsx to understand export pattern cat ui/src/components/alert.tsxLength of output: 1709
ui/src/stories/badge.stories.tsx (1)
1-12
: LGTM! Proper Storybook configuration.The metadata setup and type definitions follow best practices.
ui/src/stories/label.stories.tsx (1)
1-116
: Well-structured stories with good accessibility practices!The Label stories effectively demonstrate various use cases while maintaining proper accessibility patterns.
ui/src/stories/radio-group.stories.tsx (1)
85-88
: Implement form submission handler.The form submission handler is currently empty.
Do you want me to generate an implementation for the form submission handler?
ui/src/stories/select.stories.tsx (1)
89-92
: Implement form submission handler.The form submission handler is currently empty.
Do you want me to generate an implementation for the form submission handler?
ui/src/stories/tooltip.stories.tsx (1)
1-21
: LGTM! Well-structured imports and meta configuration.ui/src/stories/navigation-menu.stories.tsx (2)
1-20
: Well-structured Storybook configuration!The meta configuration and imports are properly set up, with autodocs enabled for automatic documentation generation.
72-158
: Well-implemented navigation menu variations!The MultipleSections and WithActiveStates stories effectively demonstrate different use cases and states.
ui/package.json (1)
46-52
: Verify Storybook version compatibilityThe Storybook version 8.4.7 is quite recent. Please verify compatibility with the current React version (^18.2.0).
Also applies to: 64-64, 69-70
✅ Verification successful
Storybook 8.4.7 is compatible with React ^18.2.0
The peer dependencies explicitly support React versions ^18.0.0, which includes React ^18.2.0.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any compatibility issues between Storybook and React versions npm info @storybook/[email protected] peerDependenciesLength of output: 254
package.json (1)
46-47
: Well-structured workspace scripts!The ui:serve and ui:build scripts are properly configured using workspace references.
<svg | ||
version="1.1" | ||
xmlns="http://www.w3.org/2000/svg" | ||
xmlSpace="preserve" | ||
fill="none" | ||
aria-hidden={true} | ||
aria-labelledby="si-logo-title" | ||
viewBox="0 0 24 24" | ||
className={twMerge('h-5 w-5 animate-spin', className)} | ||
{...props} | ||
> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add corresponding title element for aria-labelledby.
The SVG references "si-logo-title" but no title element exists.
Add this before the circle element:
<svg ...>
+ <title id="si-logo-title">Loading spinner</title>
<circle ...
📝 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.
<svg | |
version="1.1" | |
xmlns="http://www.w3.org/2000/svg" | |
xmlSpace="preserve" | |
fill="none" | |
aria-hidden={true} | |
aria-labelledby="si-logo-title" | |
viewBox="0 0 24 24" | |
className={twMerge('h-5 w-5 animate-spin', className)} | |
{...props} | |
> | |
<svg | |
version="1.1" | |
xmlns="http://www.w3.org/2000/svg" | |
xmlSpace="preserve" | |
fill="none" | |
aria-hidden={true} | |
aria-labelledby="si-logo-title" | |
viewBox="0 0 24 24" | |
className={twMerge('h-5 w-5 animate-spin', className)} | |
{...props} | |
> | |
<title id="si-logo-title">Loading spinner</title> |
export const WithImage: Story = { | ||
render: () => ( | ||
<Avatar> | ||
<AvatarImage src="https://github.com/shadcn.png" alt="@shadcn" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Replace external image URL with local asset.
Using external URLs can make stories unreliable if the resource becomes unavailable.
- <AvatarImage src="https://github.com/shadcn.png" alt="@shadcn" />
+ <AvatarImage src="/assets/avatar-placeholder.png" alt="Avatar" />
Committable suggestion skipped: line range outside the PR's diff.
// In Card Layout | ||
export const InCardLayout: Story = { | ||
render: () => ( | ||
<div className="space-y-4 rounded-lg border p-4"> | ||
<div className="flex items-center justify-between"> | ||
<h3 className="font-semibold">Account Settings</h3> | ||
<button className="text-sm text-blue-500">Edit</button> | ||
</div> | ||
<Separator /> | ||
<div className="space-y-4"> | ||
<div> | ||
<h4 className="text-sm font-medium">Email</h4> | ||
<p className="text-muted-foreground text-sm">[email protected]</p> | ||
</div> | ||
<Separator /> | ||
<div> | ||
<h4 className="text-sm font-medium">Password</h4> | ||
<p className="text-muted-foreground text-sm">Last changed 3 months ago</p> | ||
</div> | ||
</div> | ||
</div> | ||
), | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add aria-label to the edit button.
The edit button needs an aria-label for better accessibility.
-<button className="text-sm text-blue-500">Edit</button>
+<button className="text-sm text-blue-500" aria-label="Edit account settings">Edit</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.
// In Card Layout | |
export const InCardLayout: Story = { | |
render: () => ( | |
<div className="space-y-4 rounded-lg border p-4"> | |
<div className="flex items-center justify-between"> | |
<h3 className="font-semibold">Account Settings</h3> | |
<button className="text-sm text-blue-500">Edit</button> | |
</div> | |
<Separator /> | |
<div className="space-y-4"> | |
<div> | |
<h4 className="text-sm font-medium">Email</h4> | |
<p className="text-muted-foreground text-sm">[email protected]</p> | |
</div> | |
<Separator /> | |
<div> | |
<h4 className="text-sm font-medium">Password</h4> | |
<p className="text-muted-foreground text-sm">Last changed 3 months ago</p> | |
</div> | |
</div> | |
</div> | |
), | |
}; | |
// In Card Layout | |
export const InCardLayout: Story = { | |
render: () => ( | |
<div className="space-y-4 rounded-lg border p-4"> | |
<div className="flex items-center justify-between"> | |
<h3 className="font-semibold">Account Settings</h3> | |
<button className="text-sm text-blue-500" aria-label="Edit account settings">Edit</button> | |
</div> | |
<Separator /> | |
<div className="space-y-4"> | |
<div> | |
<h4 className="text-sm font-medium">Email</h4> | |
<p className="text-muted-foreground text-sm">[email protected]</p> | |
</div> | |
<Separator /> | |
<div> | |
<h4 className="text-sm font-medium">Password</h4> | |
<p className="text-muted-foreground text-sm">Last changed 3 months ago</p> | |
</div> | |
</div> | |
</div> | |
), | |
}; |
<Button size="icon" variant="default"> | ||
<PlusIcon className="h-4 w-4" /> | ||
</Button> | ||
<Button size="icon" variant="secondary"> | ||
<TrashIcon className="h-4 w-4" /> | ||
</Button> | ||
<Button size="icon" variant="outline"> | ||
<ArrowRightIcon className="h-4 w-4" /> | ||
</Button> | ||
<Button size="icon" variant="ghost"> | ||
<ChevronRightIcon className="h-4 w-4" /> | ||
</Button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add aria-labels to icon-only buttons for accessibility.
Icon-only buttons need aria-labels to be accessible to screen readers.
<Button size="icon" variant="default">
+ <span className="sr-only">Add item</span>
<PlusIcon className="h-4 w-4" />
</Button>
<Button size="icon" variant="secondary">
+ <span className="sr-only">Delete item</span>
<TrashIcon className="h-4 w-4" />
</Button>
<Button size="icon" variant="outline">
+ <span className="sr-only">Next item</span>
<ArrowRightIcon className="h-4 w-4" />
</Button>
<Button size="icon" variant="ghost">
+ <span className="sr-only">Show more</span>
<ChevronRightIcon className="h-4 w-4" />
</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.
<Button size="icon" variant="default"> | |
<PlusIcon className="h-4 w-4" /> | |
</Button> | |
<Button size="icon" variant="secondary"> | |
<TrashIcon className="h-4 w-4" /> | |
</Button> | |
<Button size="icon" variant="outline"> | |
<ArrowRightIcon className="h-4 w-4" /> | |
</Button> | |
<Button size="icon" variant="ghost"> | |
<ChevronRightIcon className="h-4 w-4" /> | |
</Button> | |
<Button size="icon" variant="default"> | |
<span className="sr-only">Add item</span> | |
<PlusIcon className="h-4 w-4" /> | |
</Button> | |
<Button size="icon" variant="secondary"> | |
<span className="sr-only">Delete item</span> | |
<TrashIcon className="h-4 w-4" /> | |
</Button> | |
<Button size="icon" variant="outline"> | |
<span className="sr-only">Next item</span> | |
<ArrowRightIcon className="h-4 w-4" /> | |
</Button> | |
<Button size="icon" variant="ghost"> | |
<span className="sr-only">Show more</span> | |
<ChevronRightIcon className="h-4 w-4" /> | |
</Button> |
<label htmlFor="email" className="text-sm font-medium"> | ||
</label> | ||
<Input id="email" type="email" placeholder="Enter email" className="border-destructive" /> | ||
<p className="text-destructive text-sm">Please enter a valid email address</p> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance error state accessibility.
Add ARIA attributes to properly communicate the error state to screen readers.
<label htmlFor="email" className="text-sm font-medium">
Email
</label>
-<Input id="email" type="email" placeholder="Enter email" className="border-destructive" />
-<p className="text-destructive text-sm">Please enter a valid email address</p>
+<Input
+ id="email"
+ type="email"
+ placeholder="Enter email"
+ className="border-destructive"
+ aria-invalid="true"
+ aria-describedby="email-error"
+/>
+<p id="email-error" className="text-destructive text-sm">Please enter a valid email address</p>
📝 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.
<label htmlFor="email" className="text-sm font-medium"> | |
</label> | |
<Input id="email" type="email" placeholder="Enter email" className="border-destructive" /> | |
<p className="text-destructive text-sm">Please enter a valid email address</p> | |
</div> | |
<label htmlFor="email" className="text-sm font-medium"> | |
</label> | |
<Input | |
id="email" | |
type="email" | |
placeholder="Enter email" | |
className="border-destructive" | |
aria-invalid="true" | |
aria-describedby="email-error" | |
/> | |
<p id="email-error" className="text-destructive text-sm">Please enter a valid email address</p> | |
</div> |
export const Interactive: Story = { | ||
render: () => ( | ||
<Card className="w-[350px] cursor-pointer transition-shadow hover:shadow-lg"> | ||
<CardHeader> | ||
<CardTitle>Interactive Card</CardTitle> | ||
<CardDescription>This card has hover and click effects</CardDescription> | ||
</CardHeader> | ||
<CardContent> | ||
<p>Click or hover over this card to see the effects.</p> | ||
</CardContent> | ||
</Card> | ||
), | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add keyboard interaction support to Interactive card.
The card has hover effects but should also support keyboard interactions for accessibility.
-<Card className="w-[350px] cursor-pointer transition-shadow hover:shadow-lg">
+<Card
+ className="w-[350px] cursor-pointer transition-shadow hover:shadow-lg focus-visible:shadow-lg focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring"
+ tabIndex={0}
+ role="button"
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ // Handle interaction
+ e.preventDefault();
+ }
+ }}
+>
📝 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 Interactive: Story = { | |
render: () => ( | |
<Card className="w-[350px] cursor-pointer transition-shadow hover:shadow-lg"> | |
<CardHeader> | |
<CardTitle>Interactive Card</CardTitle> | |
<CardDescription>This card has hover and click effects</CardDescription> | |
</CardHeader> | |
<CardContent> | |
<p>Click or hover over this card to see the effects.</p> | |
</CardContent> | |
</Card> | |
), | |
}; | |
export const Interactive: Story = { | |
render: () => ( | |
<Card | |
className="w-[350px] cursor-pointer transition-shadow hover:shadow-lg focus-visible:shadow-lg focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring" | |
tabIndex={0} | |
role="button" | |
onKeyDown={(e) => { | |
if (e.key === 'Enter' || e.key === ' ') { | |
// Handle interaction | |
e.preventDefault(); | |
} | |
}} | |
> | |
<CardHeader> | |
<CardTitle>Interactive Card</CardTitle> | |
<CardDescription>This card has hover and click effects</CardDescription> | |
</CardHeader> | |
<CardContent> | |
<p>Click or hover over this card to see the effects.</p> | |
</CardContent> | |
</Card> | |
), | |
}; |
<DialogFooter className="gap-2"> | ||
<Button variant="outline">Cancel</Button> | ||
<Button variant="destructive">Yes, delete my account</Button> | ||
</DialogFooter> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add confirmation step for destructive actions.
The delete account action should require a confirmation step, such as typing "delete" to confirm.
const DemoSlide = ({ children }: { children: React.ReactNode }) => ( | ||
<Card className="m-1"> | ||
<CardContent className="flex aspect-square items-center justify-center p-6"> | ||
<span className="text-4xl font-semibold">{children}</span> | ||
</CardContent> | ||
</Card> | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add keyboard navigation support to DemoSlide component.
The DemoSlide component should support keyboard navigation for better accessibility.
const DemoSlide = ({ children }: { children: React.ReactNode }) => (
<Card className="m-1">
<CardContent className="flex aspect-square items-center justify-center p-6">
- <span className="text-4xl font-semibold">{children}</span>
+ <span className="text-4xl font-semibold" tabIndex={0} role="region">
+ {children}
+ </span>
</CardContent>
</Card>
);
📝 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 DemoSlide = ({ children }: { children: React.ReactNode }) => ( | |
<Card className="m-1"> | |
<CardContent className="flex aspect-square items-center justify-center p-6"> | |
<span className="text-4xl font-semibold">{children}</span> | |
</CardContent> | |
</Card> | |
); | |
const DemoSlide = ({ children }: { children: React.ReactNode }) => ( | |
<Card className="m-1"> | |
<CardContent className="flex aspect-square items-center justify-center p-6"> | |
<span className="text-4xl font-semibold" tabIndex={0} role="region"> | |
{children} | |
</span> | |
</CardContent> | |
</Card> | |
); |
<span className="inline-flex items-center rounded-full bg-green-100 px-2.5 py-0.5 text-xs font-medium text-green-800"> | ||
Completed | ||
</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance status indicators for accessibility.
Add aria-labels and icons to status indicators for better accessibility.
-<span className="inline-flex items-center rounded-full bg-green-100 px-2.5 py-0.5 text-xs font-medium text-green-800">
+<span role="status" aria-label="Order completed" className="inline-flex items-center rounded-full bg-green-100 px-2.5 py-0.5 text-xs font-medium text-green-800">
Completed
</span>
Also applies to: 169-171, 179-181
function onSubmit(values: z.infer<typeof formSchema>) { | ||
console.log(values); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Replace console.log with proper form submission handling.
Remove debug console.log statements and implement proper form submission handling.
-function onSubmit(values: z.infer<typeof formSchema>) {
- console.log(values);
-}
+async function onSubmit(values: z.infer<typeof formSchema>) {
+ try {
+ // Add your form submission logic here
+ await submitForm(values);
+ } catch (error) {
+ // Handle submission errors
+ }
+}
Also applies to: 89-91
Closes #969
This PR introduces the initial integration of Storybook and some AI-generated stories for components. You can run it locally using the command
npm run ui:serve
.To simplify the setup, I copied the spinner icon directly from the site. This was necessary because Storybook could not handle imports from another workspace, and copying the asset was the simplest solution for now🙂
Let's discuss the next steps to determine what should be included in this PR:
@socialincome/ui
, with the main site importing them from there?Summary by CodeRabbit
Release Notes: UI Component Library Update
New Features
Improvements
Development Tools
This update provides a robust set of interactive component stories, improving developer experience and component documentation.