feat: replace alert() dialogs with Sonner toast notifications#253
feat: replace alert() dialogs with Sonner toast notifications#253gautamsidhwani29 wants to merge 2 commits into
Conversation
Signed-off-by: gautamsidhwani29 <gautamsidhwani405@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Code Review
This pull request replaces native browser alert() calls with the sonner library to provide modern toast notifications across the frontend. The changes involve adding the sonner dependency, integrating the Toaster component in App.jsx, and updating multiple components to use the new notification system. Review feedback recommends restoring context-aware error messages in the Jobs component and using semantic methods like toast.info for unimplemented features to ensure consistency with the rest of the application.
| // ignore error | ||
| } | ||
| alert("Error creating job: " + errorMsg); | ||
| toast.error(errorMsg); |
There was a problem hiding this comment.
The error message prefix "Error creating job: " was removed during the transition from alert() to toast.error(). This makes the notification less context-aware compared to the previous implementation and other similar components in the codebase (e.g., Pods.jsx).
| toast.error(errorMsg); | |
| toast.error("Error creating job: " + errorMsg); |
| // For now, no creation dialog | ||
| const handleCreate = () => { | ||
| alert("Create PodGroup not implemented yet"); | ||
| toast("Create PodGroup not implemented yet"); |
There was a problem hiding this comment.
For consistency with the rest of the application and to leverage the richColors configuration in App.jsx, it is recommended to use a semantic toast method like toast.info() instead of the generic toast() call for "not implemented" messages.
| toast("Create PodGroup not implemented yet"); | |
| toast.info("Create PodGroup not implemented yet"); |
Signed-off-by: gautamsidhwani29 <gautamsidhwani405@gmail.com>
Problem
The dashboard used native browser
alert()for success and error messages across multiple pages. These block the UI thread, look outdated, and provide poor UX.Solution
Replaced all
alert()calls with Sonner toast notifications.Fixes #248
Changes
sonnerpackage<Toaster />toApp.jsxalert()calls withtoast.success()andtoast.error()in:Jobs.jsxQueues.jsxPods.jsxPodGroups.jsxJobEditDialog.jsxEditQueueDialog.jsxScreenshots :