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

Add a confirmation modal before account deactivation #6107

Closed
wants to merge 1 commit into from

Conversation

jaiwanth-v
Copy link
Contributor

@jaiwanth-v jaiwanth-v commented May 27, 2021

Closes element-hq/element-web#17422

image

Signed-off-by: Jaiwanth jaiwanth2011@gmail.com

@t3chguy
Copy link
Member

t3chguy commented May 27, 2021

So the difficulty here is that there is already a modal, but we need to ask the server whether it accepts password or SSO for confirmation and the only way to do that is to hit the /deactivate endpoint, which sometimes the server will just do implicitly if you performed UIA/logged in recently.

image

So with this change, you may end up with two warnings. The one added here and then also the one in my screenshot, depending on the state of the server.

@jaiwanth-v
Copy link
Contributor Author

So the difficulty here is that there is already a modal

Yes, I knew about this. I tested this with one of my test accounts, the modal disappeared immediately and it was deactivated in no time with the settings panel still being active.

So with this change, you may end up with two warnings. The one added here and then also the one in my screenshot, depending on the state of the server.

Yeah, I think it would be better if we have two warnings as this is related to account deletion.

@t3chguy t3chguy requested a review from a team May 27, 2021 07:19
@aaronraimist
Copy link
Contributor

aaronraimist commented Jun 16, 2021

Regardless of whether there should be one warning or two warnings, it should be consistent. There should always be the same number of warnings. Some users should not get one warning while others get two.

(though as a temporarily workaround this is certainly better than nothing)

@jaiwanth-v
Copy link
Contributor Author

I agree, but as mentioned in the issue, this is just a temporary workaround to prevent accidental deletions. We would probably remove this warning in the future.

@MadLittleMods
Copy link
Contributor

Is this still relevant now that matrix-org/synapse#10184 is merged which will not skip UI auth for account deletion? The other modal will now show always AFAICT.

@jaiwanth-v
Copy link
Contributor Author

No, it isn't. Closing this.

@jaiwanth-v jaiwanth-v closed this Sep 15, 2021
@jaiwanth-v jaiwanth-v deleted the deactivate-dialog branch September 15, 2021 01:35
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.

Add temporary workaround to synapse UIA auto-advancing on deactivation
4 participants