Skip to content

feat: replace alert() dialogs with Sonner toast notifications#253

Open
gautamsidhwani29 wants to merge 2 commits into
volcano-sh:mainfrom
gautamsidhwani29:feat/replace-alert-with-sonner-toast
Open

feat: replace alert() dialogs with Sonner toast notifications#253
gautamsidhwani29 wants to merge 2 commits into
volcano-sh:mainfrom
gautamsidhwani29:feat/replace-alert-with-sonner-toast

Conversation

@gautamsidhwani29

Copy link
Copy Markdown

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

  • Installed sonner package
  • Added <Toaster /> to App.jsx
  • Replaced all alert() calls with toast.success() and toast.error() in:
    • Jobs.jsx
    • Queues.jsx
    • Pods.jsx
    • PodGroups.jsx
    • JobEditDialog.jsx
    • EditQueueDialog.jsx
Screenshot from 2026-05-16 21-48-06 Screenshot from 2026-05-16 21-48-21 Screenshot from 2026-05-16 21-48-40

Screenshots :

Signed-off-by: gautamsidhwani29 <gautamsidhwani405@gmail.com>
@volcano-sh-bot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign monokaix for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread frontend/src/components/jobs/Jobs.jsx Outdated
// ignore error
}
alert("Error creating job: " + errorMsg);
toast.error(errorMsg);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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).

Suggested change
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");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
toast("Create PodGroup not implemented yet");
toast.info("Create PodGroup not implemented yet");

Signed-off-by: gautamsidhwani29 <gautamsidhwani405@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: replace alert() dialogs with sonner notifications

2 participants