-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Add PWI opt-out toggle #2122
Add PWI opt-out toggle #2122
Conversation
} | ||
updateProfile.mutate({ | ||
profile, | ||
updates: existing => { |
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.
I realize this is not new, but are we mutating something that's also used for rendering? Generally in React we shouldn't mutate stuff we render — can this be a new object?
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.
It's not being used for rendering. I agree with the general approach as a safe default approach, but this is a narrow non-react api that handles this okay. Not against changing it, but not an issue
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.
ah if we don't render it then cool
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.
lg but mutation seems shady
This just adds the opt-out toggle. The behaviors for handling the opt-out can come in a follow-up PR (not a blocker until we enable the PWI).
Current copy
Previous iterations (no longer current)