-
-
Notifications
You must be signed in to change notification settings - Fork 834
Get confirmation before enabling encryption #2728
Conversation
21fc480
to
9402e81
Compare
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.
Wording looks plausible, modulo a typo. I can't comment on the React bits, so will allow someone else to review that.
src/components/views/settings/tabs/room/SecurityRoomSettingsTab.js
Outdated
Show resolved
Hide resolved
), | ||
onFinished: (confirm) => { | ||
if (!confirm) { | ||
this.setState({encrypted: false}); |
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.
All looks good but I'm getting hung up on this, the code later tries to revert to the original state yet this just blindly says false, I don't see why we need a setState here at all though?
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.
Because the toggle is partially controlled and animates as enabling. It's a feature of the toggle, so we have to counteract it. It also gives a visual cue that the toggle is in fact disabled.
Fixes element-hq/element-web#8843
Well paired with #2724