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

fix(courses): fix switches behavior #482

Closed
wants to merge 1 commit into from

Conversation

emacoricciati
Copy link
Contributor

Closes #477

@emacoricciati emacoricciati requested a review from a team as a code owner May 7, 2024 08:20
@github-actions github-actions bot added the cc-fix PR includes fix commit label May 7, 2024
@QcFe
Copy link
Member

QcFe commented May 7, 2024

I'm not extremely sure this is the most appropriate way to fix this issue... If I understood correctly, this would imply an optimistic approach but the final state of the switches could still not be the correct one. I would suggest a slightly different approach, such as disabling the switch until the change has been correctly propagated...

You can likely use properties from useMutation to understand if there's something going on and disable the input/show a loading spinner accordingly.

@emacoricciati
Copy link
Contributor Author

@QcFe Right, in some cases, this version doesn't consider the updated version from the server and could lead to some inconsistencies between the server and the phone. Trying to leverage the isLoading return value of useMutation still shows some issues in propagating notifications and updating switches. I was wondering if it would be better, after a button is pressed, to use a timeout (e.g., 500/1000 ms) during which the buttons remain disabled to propagate the change.

@QcFe QcFe marked this pull request as draft May 7, 2024 17:29
@Bri74
Copy link
Contributor

Bri74 commented May 9, 2024

@emacoricciati I never find it a good idea to use a timeout in these situations. @QcFe Any chance of finding a more elegant solution that works?

@Bri74 Bri74 closed this May 14, 2024
@emacoricciati emacoricciati deleted the hotfix/477-switches-not-move-properly branch May 30, 2024 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cc-fix PR includes fix commit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switches appear to not move properly if notification settings for a course are toggled too quickly
3 participants