feat: add Dialog component with various demo stories#75
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new Dialog component set (with theming and optional close button) and accompanying Storybook stories demonstrating five dialog variants. Exports for the Dialog components are added to the UI module and root index. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Story as Storybook (or App)
participant Trigger as DialogTrigger
participant Dialog
participant Portal as DialogPortal
participant Overlay as DialogOverlay
participant Content as DialogContent
User->>Story: click "Open dialog" button
Story->>Trigger: onClick -> open
Trigger->>Dialog: set open state
Dialog->>Portal: render portal
Portal->>Overlay: render backdrop
Portal->>Content: render dialog content (uses theme CSS vars)
User->>Content: interact (scroll, inputs, buttons)
User->>Overlay: click backdrop (optional close)
Content->>Dialog: DialogClose invoked -> set closed
Dialog->>Portal: unmount portal
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (1)
src/components/ui/Dialog.stories.tsx (1)
29-49: Wrap the story form controls withFieldfor consistency.These input groups are currently composed with
Label+Inputdirectly; please switch them to theFieldpattern used in this repo.As per coding guidelines, "Wrap form controls with the
Fieldcomponent to ensure consistent label, description, and error display".Also applies to: 206-233
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/Dialog.stories.tsx` around lines 29 - 49, Replace the direct Label + Input groupings in the Dialog story with the repo's Field pattern: wrap each label/input pair (e.g., the groups using Label and Input with ids "name" and "username" and any similar groups later in the file) inside a Field component so labels, descriptions, and errors render consistently; move the Label into Field's label slot/prop and place the Input as Field's child, preserving ids, defaultValue, and className, and apply the same change to the other form groups flagged (the later blocks around the other form controls).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ui/dialog.tsx`:
- Around line 70-74: The close Button rendered in DialogContent (the Button with
variant="ghost" className="absolute top-4 end-4" size="icon-sm") lacks an
explicit type and can act as a form submit button; update that Button to include
type="button" so it never triggers form submission when DialogContent is used
inside a form.
---
Nitpick comments:
In `@src/components/ui/Dialog.stories.tsx`:
- Around line 29-49: Replace the direct Label + Input groupings in the Dialog
story with the repo's Field pattern: wrap each label/input pair (e.g., the
groups using Label and Input with ids "name" and "username" and any similar
groups later in the file) inside a Field component so labels, descriptions, and
errors render consistently; move the Label into Field's label slot/prop and
place the Input as Field's child, preserving ids, defaultValue, and className,
and apply the same change to the other form groups flagged (the later blocks
around the other form controls).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 496a4056-29ea-4ff7-b449-286ab0a52ce0
📒 Files selected for processing (3)
src/components/ui/Dialog.stories.tsxsrc/components/ui/dialog.tsxsrc/components/ui/index.ts
| <Button | ||
| variant="ghost" | ||
| className="absolute top-4 end-4" | ||
| size="icon-sm" | ||
| /> |
There was a problem hiding this comment.
Set explicit type="button" on the built-in close button.
Without an explicit type, this can act as a submit button when DialogContent is used inside a form, causing unintended submits.
Suggested fix
<Button
variant="ghost"
className="absolute top-4 end-4"
size="icon-sm"
+ type="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 | |
| variant="ghost" | |
| className="absolute top-4 end-4" | |
| size="icon-sm" | |
| /> | |
| <Button | |
| variant="ghost" | |
| className="absolute top-4 end-4" | |
| size="icon-sm" | |
| type="button" | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/dialog.tsx` around lines 70 - 74, The close Button rendered
in DialogContent (the Button with variant="ghost" className="absolute top-4
end-4" size="icon-sm") lacks an explicit type and can act as a form submit
button; update that Button to include type="button" so it never triggers form
submission when DialogContent is used inside a form.
Summary by CodeRabbit
New Features
Documentation