-
Notifications
You must be signed in to change notification settings - Fork 12
feat(console): add toast notification system for user feedback #66
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
base: main
Are you sure you want to change the base?
Conversation
Add global notification system with: - NotificationContext for type definitions - NotificationProvider component with Snackbar/Alert UI - useNotification hook for accessing notify function - Auto-dismiss notifications after 5 seconds
Wrap app with NotificationProvider to enable global notifications
- Show success notification when build is triggered - Show error notification on build failure - Remove console.error logging
- Show success notification when deployment starts - Show error notification on deployment failure - Remove empty catch block
- Show success notification when agent is created - Show error notification on creation failure - Remove TODO comment and console.error
Required for useNotification hook
Reusable confirmation dialog for destructive actions. Props: title, message, confirmLabel, cancelLabel, confirmColor, isLoading
- Show confirmation dialog before deleting an agent - Display success/error notifications after deletion
WalkthroughThe PR introduces a notification system with queued snackbar display and a reusable confirmation dialog component, then integrates both across multiple pages for user feedback on asynchronous operations and destructive actions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
console/workspaces/libs/shared-component/src/components/index.ts (1)
21-21: ConfirmDialog component is properly exported and integrated.The ConfirmDialog.tsx file exists and is correctly exported in index.ts, aligning with the PR objectives for confirmation dialogs on destructive actions. The export placement is functional and appropriate.
Optional refinement: Consider maintaining alphabetical ordering of exports for easier navigation (currently ConfirmDialog exports out of sequence; alphabetically it should appear after CodeBlock).
console/workspaces/libs/shared-component/src/providers/NotificationProvider.tsx (1)
57-70: Redundant onClose handlers.Both the
Snackbar(line 60) andAlert(line 65) components have identicalonClosehandlers. The Alert'sonCloseis sufficient since clicking the close button on the Alert will trigger it.🔎 Simplify by removing the redundant Snackbar onClose:
<Snackbar open autoHideDuration={AUTO_HIDE_DURATION} - onClose={() => removeNotification(notifications[0].id)} + onClose={() => removeNotification(notifications[0].id)} anchorOrigin={{ vertical: 'bottom', horizontal: 'right' }} >Actually, keep the Snackbar
onClosefor auto-hide and remove it from Alert, or keep both for redundancy. The current implementation works but is slightly redundant.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
console/common/config/rush/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
console/apps/webapp/src/Providers/GlobalProviders/GlobalProviders.tsx(2 hunks)console/workspaces/libs/shared-component/src/components/BuildPanel.tsx(2 hunks)console/workspaces/libs/shared-component/src/components/ConfirmDialog.tsx(1 hunks)console/workspaces/libs/shared-component/src/components/DeploymentConfig.tsx(3 hunks)console/workspaces/libs/shared-component/src/components/index.ts(1 hunks)console/workspaces/libs/shared-component/src/index.ts(1 hunks)console/workspaces/libs/shared-component/src/providers/NotificationContext.ts(1 hunks)console/workspaces/libs/shared-component/src/providers/NotificationProvider.tsx(1 hunks)console/workspaces/libs/shared-component/src/providers/index.ts(1 hunks)console/workspaces/libs/shared-component/src/providers/useNotification.ts(1 hunks)console/workspaces/pages/add-new-agent/package.json(1 hunks)console/workspaces/pages/add-new-agent/src/AddNewAgent.tsx(3 hunks)console/workspaces/pages/overview/src/AgentsList/AgentsList.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
console/workspaces/libs/shared-component/src/providers/NotificationProvider.tsx (1)
console/workspaces/libs/shared-component/src/providers/NotificationContext.ts (2)
NotificationType(22-22)NotificationContext(30-30)
console/workspaces/pages/overview/src/AgentsList/AgentsList.tsx (4)
console/workspaces/libs/shared-component/src/providers/useNotification.ts (1)
useNotification(26-34)console/workspaces/libs/api-client/src/hooks/projects.ts (1)
useGetProject(45-52)console/workspaces/libs/shared-component/src/components/ConfirmationDialog/ConfirmationDialogProvider.tsx (1)
useConfirmationDialog(38-40)console/workspaces/libs/api-client/src/apis/agents.ts (1)
deleteAgent(93-110)
console/workspaces/pages/add-new-agent/src/AddNewAgent.tsx (2)
console/workspaces/libs/shared-component/src/providers/useNotification.ts (1)
useNotification(26-34)console/workspaces/libs/api-client/src/apis/agents.ts (1)
createAgent(57-71)
console/apps/webapp/src/Providers/GlobalProviders/GlobalProviders.tsx (3)
console/workspaces/libs/shared-component/src/providers/NotificationProvider.tsx (1)
NotificationProvider(35-74)console/workspaces/libs/auth/src/asgardio/AuthProvider.tsx (1)
AuthProvider(23-31)console/workspaces/libs/shared-component/src/components/ConfirmationDialog/ConfirmationDialogProvider.tsx (1)
ConfirmationDialogProvider(42-99)
console/workspaces/libs/shared-component/src/components/DeploymentConfig.tsx (2)
console/workspaces/libs/shared-component/src/providers/useNotification.ts (1)
useNotification(26-34)console/workspaces/libs/api-client/src/apis/deployments.ts (1)
deployAgent(40-56)
console/workspaces/libs/shared-component/src/providers/useNotification.ts (1)
console/workspaces/libs/shared-component/src/providers/NotificationContext.ts (1)
NotificationContext(30-30)
console/workspaces/libs/shared-component/src/components/BuildPanel.tsx (2)
console/workspaces/libs/shared-component/src/providers/useNotification.ts (1)
useNotification(26-34)console/workspaces/libs/api-client/src/apis/builds.ts (1)
buildAgent(33-59)
🔇 Additional comments (16)
console/workspaces/libs/shared-component/src/index.ts (1)
20-20: LGTM!The providers export cleanly extends the public API to include the new notification system.
console/workspaces/pages/add-new-agent/package.json (1)
51-51: LGTM!The workspace dependency correctly enables access to the new notification system from the add-new-agent page.
console/workspaces/libs/shared-component/src/components/DeploymentConfig.tsx (3)
27-27: LGTM!The import correctly brings in the useNotification hook from the providers module.
82-111: LGTM!The refactoring to callback-based error handling with notifications is well-implemented:
- Success path closes the drawer and notifies the user.
- Error path keeps the drawer open (allowing retry) and displays the error message.
- Defensive error message extraction handles both Error objects and unknown error types.
53-53: Hook usage is correct and provider is properly configured.The
useNotificationhook at line 53 is correctly used. TheNotificationProvideris properly wrapped inGlobalProviders(atconsole/apps/webapp/src/Providers/GlobalProviders/GlobalProviders.tsxlines 31-37), which wraps the entire application component tree. The hook includes error handling that throws if used outside the provider, ensuring runtime safety.console/workspaces/pages/add-new-agent/src/AddNewAgent.tsx (3)
28-28: LGTM!The import and hook usage correctly integrate the notification system.
Also applies to: 36-36
84-99: LGTM!The notification integration in the onSubmit callbacks provides clear user feedback:
- Success notification before navigation ensures users see the confirmation.
- Error notification with defensive message extraction handles various error types.
101-101: LGTM!Correctly includes
notifyin the dependency array, following React Hook rules and preventing stale closure issues.console/workspaces/libs/shared-component/src/components/BuildPanel.tsx (2)
24-24: LGTM!The import and hook usage correctly integrate the notification system, consistent with other components in this PR.
Also applies to: 45-45
59-80: LGTM!The refactoring follows the same solid pattern used in DeploymentConfig:
- Callback-based error handling with notifications provides clear user feedback.
- Success path closes the panel after notifying.
- Error path keeps the panel open, allowing users to adjust inputs and retry.
console/workspaces/libs/shared-component/src/providers/index.ts (1)
19-21: LGTM!The barrel exports cleanly aggregate the notification-related modules, simplifying imports for consuming components.
console/workspaces/libs/shared-component/src/providers/useNotification.ts (1)
26-34: LGTM! Proper React context implementation.The hook follows the standard React context pattern with correct error handling. The NotificationContext is properly typed with
NotificationContextType | null, and the NotificationProvider implementation is well-structured with memoization to prevent unnecessary re-renders and a queue-based notification system using MUI components.console/apps/webapp/src/Providers/GlobalProviders/GlobalProviders.tsx (1)
21-21: LGTM: Provider nesting is correctly structured.The provider hierarchy is well-organized with
NotificationProviderwrapping the auth and client providers, ensuring the notification context is available throughout the component tree.Also applies to: 31-37
console/workspaces/libs/shared-component/src/providers/NotificationProvider.tsx (1)
56-71: Verify queue behavior with rapid notifications.The notification queue shows only the first notification. When it's dismissed (either manually or via auto-hide), the component will re-render and show the next queued notification. Ensure this behavior works correctly when multiple notifications are triggered in quick succession.
You can test this by triggering multiple notifications rapidly (e.g., clicking a button multiple times) and confirming that:
- Only one notification is visible at a time
- Subsequent notifications appear after the previous one is dismissed
- The 5-second auto-hide works correctly for each notification in the queue
console/workspaces/libs/shared-component/src/providers/NotificationContext.ts (1)
21-30: LGTM: Clean context definition with proper type safety.The
NotificationTypealigns with MUI Alert severities, and the nullable context default ensures theuseNotificationhook can enforce provider usage at runtime.console/workspaces/pages/overview/src/AgentsList/AgentsList.tsx (1)
64-64: LGTM: Well-implemented delete flow with confirmation and notifications.The integration of confirmation dialog and toast notifications provides excellent UX:
- Confirmation dialog prevents accidental deletions
- Success notification includes the agent name for clear feedback
- Error handling with fallback message ensures users are always informed
- Dependencies are correctly specified in the
useCallbackAlso applies to: 126-126, 133-162
| export interface ConfirmDialogProps { | ||
| open: boolean; | ||
| title: string; | ||
| message: string; | ||
| confirmLabel?: string; | ||
| cancelLabel?: string; | ||
| confirmColor?: 'error' | 'primary' | 'secondary'; | ||
| onConfirm: () => void; | ||
| onCancel: () => void; | ||
| isLoading?: boolean; | ||
| } | ||
|
|
||
| /** | ||
| * Reusable confirmation dialog for destructive or important actions. | ||
| * Use for delete confirmations, irreversible operations, etc. | ||
| */ | ||
| export function ConfirmDialog({ | ||
| open, | ||
| title, | ||
| message, | ||
| confirmLabel = 'Confirm', | ||
| cancelLabel = 'Cancel', | ||
| confirmColor = 'error', | ||
| onConfirm, | ||
| onCancel, | ||
| isLoading = false, | ||
| }: ConfirmDialogProps) { | ||
| return ( | ||
| <Dialog open={open} onClose={onCancel} maxWidth="xs" fullWidth> | ||
| <DialogTitle>{title}</DialogTitle> | ||
| <DialogContent> | ||
| <Typography variant="body2" color="text.secondary"> | ||
| {message} | ||
| </Typography> | ||
| </DialogContent> | ||
| <DialogActions> | ||
| <Button | ||
| onClick={onCancel} | ||
| variant="outlined" | ||
| disabled={isLoading} | ||
| > | ||
| {cancelLabel} | ||
| </Button> | ||
| <Button | ||
| onClick={onConfirm} | ||
| variant="contained" | ||
| color={confirmColor} | ||
| disabled={isLoading} | ||
| startIcon={isLoading ? <CircularProgress size={16} color="inherit" /> : undefined} | ||
| > | ||
| {isLoading ? 'Loading...' : confirmLabel} | ||
| </Button> | ||
| </DialogActions> | ||
| </Dialog> | ||
| ); | ||
| } |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if ConfirmDialog is imported or used anywhere in the codebase
rg -n "ConfirmDialog" --type ts --type tsx -C 2Repository: wso2/ai-agent-management-platform
Length of output: 104
🏁 Script executed:
#!/bin/bash
# Search for ConfirmDialog in all files, then filter to .ts and .tsx
rg "ConfirmDialog" -C 2 | head -100Repository: wso2/ai-agent-management-platform
Length of output: 2041
🏁 Script executed:
#!/bin/bash
# Search for ConfirmDialog imports
echo "=== ConfirmDialog imports ==="
rg "import.*ConfirmDialog" -C 2
echo ""
echo "=== ConfirmationDialogProvider usage ==="
rg "ConfirmationDialogProvider|useConfirmationDialog" -C 2 | head -150Repository: wso2/ai-agent-management-platform
Length of output: 4710
🏁 Script executed:
#!/bin/bash
# Look at the ConfirmationDialogProvider implementation
echo "=== ConfirmationDialogProvider location and structure ==="
find . -name "ConfirmationDialogProvider.tsx" -exec wc -l {} \; -exec head -80 {} \;Repository: wso2/ai-agent-management-platform
Length of output: 2315
Remove or document the ConfirmDialog component usage.
The ConfirmDialog component is exported but never imported or used anywhere in the codebase. The application already uses ConfirmationDialogProvider with the useConfirmationDialog hook (as seen in GlobalProviders.tsx, AgentsList.tsx, and ProjectList.tsx), which provides a centralized, queue-based confirmation pattern.
If ConfirmDialog serves a different purpose (e.g., direct prop-controlled confirmations vs. global state-managed ones), add documentation clarifying when to use each. Otherwise, remove it to avoid confusion and reduce bundle size.
| const notify = useCallback((type: NotificationType, message: string) => { | ||
| const id = Date.now().toString(); | ||
| setNotifications((prev) => [...prev, { id, type, message }]); | ||
| }, []); |
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.
Potential ID collision with Date.now().
Using Date.now().toString() for notification IDs may result in collisions if multiple notifications are triggered in rapid succession (within the same millisecond).
🔎 Consider using a more robust ID generation:
- const notify = useCallback((type: NotificationType, message: string) => {
- const id = Date.now().toString();
+ const notify = useCallback((type: NotificationType, message: string) => {
+ const id = `${Date.now()}-${Math.random().toString(36).substr(2, 9)}`;
setNotifications((prev) => [...prev, { id, type, message }]);
}, []);📝 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 notify = useCallback((type: NotificationType, message: string) => { | |
| const id = Date.now().toString(); | |
| setNotifications((prev) => [...prev, { id, type, message }]); | |
| }, []); | |
| const notify = useCallback((type: NotificationType, message: string) => { | |
| const id = `${Date.now()}-${Math.random().toString(36).substr(2, 9)}`; | |
| setNotifications((prev) => [...prev, { id, type, message }]); | |
| }, []); |
🤖 Prompt for AI Agents
In
console/workspaces/libs/shared-component/src/providers/NotificationProvider.tsx
around lines 39-42, the notification ID is generated using Date.now().toString()
which can collide for multiple notifications in the same millisecond; change the
ID generation to a robust method such as crypto.randomUUID() (with a small
fallback to a useRef-based incrementing counter or a lightweight library like
nanoid for environments without crypto.randomUUID), update the function to call
that generator (or fall back) when creating the id, and ensure any TS types
remain compatible.
Purpose
Users have no feedback when actions succeed or fail (e.g., creating an agent, deleting an agent). This makes the UX confusing.
Resolves #62
Goals
Approach
NotificationProvideranduseNotificationhook in shared-componentUser stories
Release note
Added toast notification system for user feedback on actions like agent creation and deletion.
Documentation
N/A - UI enhancement, no new configuration required
Training
N/A
Certification
N/A - UI enhancement only
Marketing
N/A
Automation tests
Security checks
Samples
N/A
Related PRs
N/A
Migrations (if applicable)
N/A
Test environment
Learning
Used MUI Snackbar/Alert components via @wso2/oxygen-ui for toast implementation.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.