Skip to content
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

js/opnsense_ui: Add checkmark to SimpleActionButton on success #8310

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Monviech
Copy link
Member

The Apply button does not give any feedback on success.

This PR makes a checkmark appear on success. It is consistent with how the dashboard handles its Save button, where the spinner and checkmark also appear on success.

The width of the Apply button stays consistent when the spinner and checkmark are added or removed.

@Monviech Monviech added the cleanup Low impact changes label Feb 11, 2025
@Monviech Monviech self-assigned this Feb 11, 2025
@fichtner
Copy link
Member

Frankly I'm not a fan of this behaviour: the spinner should spin while work is in progress. On abort the spinner should stop and show an API error or other message if relevant. The default outcome is success and indicating this further seems spurious. Just my 2 cents here.

@Monviech Monviech requested a review from swhite2 February 12, 2025 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Low impact changes
Development

Successfully merging this pull request may close these issues.

2 participants