This repository has been archived by the owner on Sep 11, 2024. It is now read-only.
Bring back lost functionality on login/register/password-reset screens #200
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… in @kegsay's refactor. specifically: 1) custom HS/IS urls are now persisted in HTML5 local storage. As a result, all the login components now distinguish between default HS/IS URLs and custom specified ones again. ( 2) custom HS/IS urls are synchronised between the instances of ServerConfig found in the Login, Registration and Forgot Password screens. 3) username are persisted over changing homeserver (but not password, to stop accidentally leaking passwords to the wrong server) 4) correctly interpret a blank URL field as meaning the placeholder text 5) when toggling custom URLs on and off, remember what the custom values were, and use the default URLs if custom mode is not engaged also, guest access now upholds custom HS/IS URLs found in local storage rather than being limited to the server config () also adds assorted comments and improved console debug and a few minor cosmetic changes to the login components. this commit sponsored by VS27...
| @@ -115,7 +118,7 @@ class MatrixClient { | |||
| } | |||
| } else { | |||
| console.warn("No local storage available: can't persist HS/IS URLs!"); | |||
| } | |||
| } | |||
|
@kegsay ptal... |
| } | ||
| }, | ||
|
|
||
| onHomeserverChanged: function(ev) { | ||
| this.setState({hs_url: ev.target.value}, function() { | ||
| this._hsTimeoutId = this._waitThenInvoke(this._hsTimeoutId, function() { | ||
| this.props.onHsUrlChanged(this.state.hs_url.replace(/\/$/, "")); | ||
| var hsUrl = this.state.hs_url.trim().replace(/\/$/, ""); |
There was a problem hiding this comment.
If customHsUrl is null, then this.state.hs_url will be null, meaning this will NPE. We should have default props for the custom URLs, or mark isRequired so when the inevitable happens we have an idea why.
|
LGTM aside from NPE risk. |
ara4n
added a commit
that referenced
this pull request
Mar 15, 2016
Bring back lost functionality on login/register/password-reset screens
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
specifically:
also, guest access now upholds custom HS/IS URLs found in local storage rather than being limited to the server config ()
also adds assorted comments and improved console debug and a few minor cosmetic changes to the login components.