Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Get confirmation before enabling encryption #2728

Merged
merged 4 commits into from
Mar 5, 2019

Conversation

turt2live
Copy link
Member

Fixes element-hq/element-web#8843
Well paired with #2724

image

@turt2live turt2live requested a review from a team March 1, 2019 03:39
Copy link
Member

@uhoreg uhoreg left a 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.

),
onFinished: (confirm) => {
if (!confirm) {
this.setState({encrypted: false});
Copy link
Member

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?

Copy link
Member Author

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.

@turt2live turt2live requested review from t3chguy and a team and removed request for t3chguy March 1, 2019 16:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants