-
Notifications
You must be signed in to change notification settings - Fork 891
feat(site): dismiss health section warnings #11059
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
Conversation
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.
Looks good! Just had a question about how we had the enabled/dismiss mutations set up
const queryClient = useQueryClient(); | ||
const healthSettingsQuery = useQuery(healthSettings()); | ||
// They call the same mutation but are used in diff contexts so we don't want | ||
// to merge their states |
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.
What edge cases were you running into by having just one mutation? I trust you, but I'm just curious, since the output looks so similar across each of the if branches
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.
Updated the comments to include a use case:
They call the same mutation but are used in diff contexts so we don't want
to merge their states. Eg. You dismiss a warning and when it is done it
will show the enable button but since the mutation is still invalidating
other queries it will be in the loading state when it should be idle.
Screen.Recording.2023-12-06.at.10.05.05.mov