feat(ui): enhance DashboardAlert component#307
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughEnhanced DashboardAlert component to support configurable variants ('info' | 'warning') and optional primary/secondary actions with custom onClick handlers. The component now extends AlertProps, removes default content values, and conditionally renders action buttons through AlertActions container. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
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. 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 |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
…nce Dashboard Alert component
There was a problem hiding this comment.
Pull request overview
This PR enhances the DashboardAlert component by making it more flexible and configurable, removing hardcoded default values and adding support for custom actions.
Changes:
- Removed hardcoded default title and description values, making them fully optional
- Added support for primary and secondary actions with custom labels and onClick handlers
- Introduced configurable
variantprop (info/warning) instead of hardcoded 'info' - Extended the component to accept all
AlertPropsvia spread operator for greater flexibility
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
libs/react/ui/src/components/dashboard/components/dashboard-alert.tsx
Outdated
Show resolved
Hide resolved
libs/react/ui/src/components/dashboard/components/dashboard-alert.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/react/ui/src/components/dashboard/components/dashboard-alert.tsx (1)
49-56: Conditionally renderAlertActionsonly when actions exist.
AlertActionsis always rendered even when bothprimaryActionandsecondaryActionare undefined, resulting in an empty container that may cause unintended spacing or layout issues.♻️ Suggested improvement
- <AlertActions> - {primaryAction && ( - <AlertAction onClick={primaryAction.onClick}>{primaryAction.label}</AlertAction> - )} - {secondaryAction && ( - <AlertAction onClick={secondaryAction.onClick}>{secondaryAction.label}</AlertAction> - )} - </AlertActions> + {(primaryAction || secondaryAction) && ( + <AlertActions> + {primaryAction && ( + <AlertAction onClick={primaryAction.onClick}>{primaryAction.label}</AlertAction> + )} + {secondaryAction && ( + <AlertAction onClick={secondaryAction.onClick}>{secondaryAction.label}</AlertAction> + )} + </AlertActions> + )}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/react/ui/src/components/dashboard/components/dashboard-alert.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
We handle errors at the edge of our applications in most cases. Do not recommend to add error handling around every single function. We prefer them to bubble up and be handled at upper layers.
Files:
libs/react/ui/src/components/dashboard/components/dashboard-alert.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Upload results
- GitHub Check: Continuous integration
🔇 Additional comments (3)
libs/react/ui/src/components/dashboard/components/dashboard-alert.tsx (3)
1-11: LGTM!Imports are well-organized, separating component imports from type imports.
13-27: LGTM!The interface extension and action prop structure are well-designed, allowing flexibility while maintaining type safety.
40-45: LGTM!The Alert configuration correctly passes the variant, handles dismissal via
onOpenChange, and properly forwards remaining props.
…d actions for improved clarity
|
Could we have a story with the variants ? |
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.