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.

Sign up for GitHub

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

Jump to bottom

feat(spinner): add spinner to action buttons #640

Merged
merged 14 commits into from
Nov 17, 2022
Merged

feat(spinner): add spinner to action buttons #640

merged 14 commits into from
Nov 17, 2022

Conversation

tthvo
Copy link
Member

@tthvo tthvo commented Nov 10, 2022 •
edited
Loading

Fixes #597

The following action buttons are configured to show progress indicator:

  • Target
    • Create
    • Delete
  • Automatic Rule
    • Create
    • Upload (also added client-side content check before sending requests).
  • Recording
    • Create Active/Snapshot in form (also corrected type hints, refactored props, rename modules)
    • Delete
    • Stop
    • Archive
    • Bulk-edit labels
    • Upload Archives
  • Security
    • Save credentials (also added error handler and block Enter key if form is empty)
    • Upload SSL Certificates (also wrapped callbacks in React.useCallback)
  • Events
    • Upload Event Templates (also refactored modal into separate component)
    • Upload Agent Templates (also refactored modal into separate component)

Except for ArchiveModal, all modals will hide close button in loading state to avoid closing. This will be reverted after abort support is added (Related to #651).

@tthvo tthvo added the feat New feature or request label Nov 10, 2022
@tthvo tthvo force-pushed the progress-indicators branch 6 times, most recently from b0624d3 to 9b46fb6 Compare November 17, 2022 01:32
@andrewazores
Copy link
Member

Anything else left todo @tthvo ?

@tthvo
Copy link
Member Author

tthvo commented Nov 17, 2022 •
edited
Loading

Oh just going through each one again to check if error handler is added or else it will be stuck in loading state :D Any other places to add spinner than above you want in mind too?

@tthvo tthvo force-pushed the progress-indicators branch from 39d4406 to 709719e Compare November 17, 2022 18:07
@tthvo tthvo marked this pull request as ready for review November 17, 2022 18:09
@tthvo
Copy link
Member Author

tthvo commented Nov 17, 2022

Ready for review now. Please let me know if any other places need adding.

@tthvo tthvo force-pushed the progress-indicators branch from 709719e to 3cd1118 Compare November 17, 2022 18:39
@andrewazores
Copy link
Member

Haven't reviewed the code yet but manual test looks great.

@tthvo
Copy link
Member Author

tthvo commented Nov 17, 2022 •
edited
Loading

Also just a note on this: buttons now are a little wider. I think its the effect of adding loading props (maybe for smoother animations).

@tthvo tthvo mentioned this pull request Nov 17, 2022
Merged
maxcao13
maxcao13 approved these changes Nov 17, 2022
View reviewed changes
Copy link
Member

@maxcao13 maxcao13 left a comment

Choose a reason for hiding this comment

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

LGTM!

@andrewazores andrewazores merged commit 31e82e7 into cryostatio:main Nov 17, 2022
@tthvo tthvo deleted the progress-indicators branch November 17, 2022 22:37
This was referenced Nov 18, 2022
Merged
Merged
Closed
@tthvo tthvo mentioned this pull request Dec 2, 2022
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxcao13 maxcao13 maxcao13 approved these changes

@andrewazores andrewazores andrewazores approved these changes

Assignees
No one assigned
Labels
feat New feature or request
Projects
No open projects
2.3.0 release
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Task] Add progress spinner to action buttons
3 participants
@tthvo @andrewazores @maxcao13