-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
fix: alert & reports active toggle optimistic update #20402
fix: alert & reports active toggle optimistic update #20402
Conversation
Codecov Report
@@ Coverage Diff @@
## master #20402 +/- ##
=======================================
Coverage 66.52% 66.53%
=======================================
Files 1739 1739
Lines 65141 65142 +1
Branches 6900 6900
=======================================
+ Hits 43336 43343 +7
+ Misses 20052 20046 -6
Partials 1753 1753
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
||
updateResource(update_id, { active: checked }, false, false) | ||
.then() | ||
.catch(() => setResourceCollection(original)); |
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.
Shouldn't we display a toast for the user like "An error occurred while changing the active state"?
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.
We are. The updateResource
comes from the useSingleViewResource
hook, which accepts a function to handle errors and we're passing addDangerToast
to it.
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.
LGTM
(cherry picked from commit 4dc3044)
(cherry picked from commit 4dc3044)
SUMMARY
For the alert and reports, the active toggle could lag and give the user the impression nothing is happening.
This PR just uses an optimistic update instead, and revert the change if the API fails.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
Screen.Recording.2022-06-15.at.16.50.44.mov
After:
new.mov
TESTING INSTRUCTIONS
The expected result is that the user sees the toggle pretty quickly.
ADDITIONAL INFORMATION