Disconnect from IS Button #3305
Conversation
There was a problem hiding this comment.
largely looks fine based on dbkr/change_is...dbkr/disco_is
It looks like there's some merge conflicts, but I think they are easily solved here.
| _disconnectIdServer = () => { | ||
| MatrixClientPeg.get().setIdentityServerUrl(null); | ||
| localStorage.removeItem("mx_is_access_token"); | ||
| localStorage.removeItem("mx_is_url"); |
There was a problem hiding this comment.
I kinda wonder if these 3 things should happen in the Lifecycle? It feels a bit dirty to manipulate the settings here.
This is probably fine though.
There was a problem hiding this comment.
yeah, probably, but since it's currently only used here I'd probably argue for moving it into a function in lifecycle if we ever use it anywhere else.
There was a problem hiding this comment.
I think an important step of warning about live bindings is missing here, so if this will be merged as fixing element-hq/element-web#10425, then we should break that extra bit out as a separate issue.
| }); | ||
| }; | ||
|
|
||
| _onDisconnectClicked = () => { |
There was a problem hiding this comment.
I am not sure that this dialog is really adding that much, since it just confirms removing the server, and we're already making the first button appear dangerous... I wonder if @nadonomy has an opinion.
I think we do want some kind of step that warns about disconnecting the IS when there are 3PIDs currrently bound, as mentioned in the issue.
There was a problem hiding this comment.
Trying it with no confirmation felt very trigger-happy - I might even argue that for establishing the convention that any danger button has a confirmation.
Co-Authored-By: Travis Ralston <travpc@gmail.com>
Co-Authored-By: J. Ryan Stinnett <jryans@gmail.com>
Co-Authored-By: J. Ryan Stinnett <jryans@gmail.com>
Co-Authored-By: J. Ryan Stinnett <jryans@gmail.com>
|
binding warning split out to element-hq/element-web#10550 |
Fixes element-hq/element-web#10425
Based on #3300