From ab068cc3722db81b644dc4733ee043be53bd9759 Mon Sep 17 00:00:00 2001
From: Matthew Hodgson <matthew@matrix.org>
Date: Mon, 31 Aug 2015 19:30:24 +0100
Subject: [PATCH] improve login, including checkbox fix for advanced options,
 rechecking when you change server, avoiding flickering when you change HS,
 better error/spinner layout, and trimming whitespace

---
 skins/base/css/templates/Login.css     | 10 ++++++-
 skins/base/views/templates/Login.js    | 41 +++++++++++++-------------
 skins/base/views/templates/Register.js |  8 ++---
 src/controllers/templates/Login.js     | 14 ++++++---
 4 files changed, 43 insertions(+), 30 deletions(-)

diff --git a/skins/base/css/templates/Login.css b/skins/base/css/templates/Login.css
index fe5646f9..49feffbe 100644
--- a/skins/base/css/templates/Login.css
+++ b/skins/base/css/templates/Login.css
@@ -82,11 +82,19 @@ limitations under the License.
     opacity: 0.8;
 }
 
+.mx_Login_loader {
+    position: absolute;
+    left: 50%;
+    margin-top: 12px;
+}
+
 .mx_Login_error {
     color: #ff2020;
     font-weight: bold;
     text-align: center;
-    margin-bottom: 24px;
+    height: 24px;
+    margin-top: 12px;
+    margin-bottom: 12px;
 }
 
 .mx_Login_create:link {
diff --git a/skins/base/views/templates/Login.js b/skins/base/views/templates/Login.js
index a783296d..887c7ca7 100644
--- a/skins/base/views/templates/Login.js
+++ b/skins/base/views/templates/Login.js
@@ -74,14 +74,14 @@ module.exports = React.createClass({
      */
     getFormVals: function() {
         return {
-            'username': this.refs.user.getDOMNode().value,
-            'password': this.refs.pass.getDOMNode().value
+            'username': this.refs.user.getDOMNode().value.trim(),
+            'password': this.refs.pass.getDOMNode().value.trim()
         };
     },
 
     onHsUrlChanged: function() {
-        this.customHsUrl = this.refs.serverConfig.getHsUrl();
-        this.customIsUrl = this.refs.serverConfig.getIsUrl();
+        this.customHsUrl = this.refs.serverConfig.getHsUrl().trim();
+        this.customIsUrl = this.refs.serverConfig.getIsUrl().trim();
         MatrixClientPeg.replaceUsingUrls(
             this.getHsUrl(),
             this.getIsUrl()
@@ -93,23 +93,24 @@ module.exports = React.createClass({
         // XXX: HSes do not have to offer password auth, so we
         // need to update and maybe show a different component
         // when a new HS is entered.
-        /*if (this.updateHsTimeout) {
+        if (this.updateHsTimeout) {
             clearTimeout(this.updateHsTimeout);
         }
         var self = this;
         this.updateHsTimeout = setTimeout(function() {
             self.onHSChosen();
-        }, 500);*/
+        }, 500);
     },
 
     componentForStep: function(step) {
         switch (step) {
             case 'choose_hs':
+            case 'fetch_stages':
                 var serverConfigStyle = {};
                 serverConfigStyle.display = this.state.serverConfigVisible ? 'block' : 'none';
                 return (
                     <div>
-                        <input className="mx_Login_checkbox" id="advanced" type="checkbox" value={this.state.serverConfigVisible} onChange={this.onServerConfigVisibleChange} />
+                        <input className="mx_Login_checkbox" id="advanced" type="checkbox" checked={this.state.serverConfigVisible} onChange={this.onServerConfigVisibleChange} />
                         <label className="mx_Login_label" htmlFor="advanced">Use custom server options (advanced)</label>
                         <div style={serverConfigStyle}>
                             <ServerConfig ref="serverConfig"
@@ -126,7 +127,7 @@ module.exports = React.createClass({
                         <form onSubmit={this.onUserPassEntered}>
                         <input className="mx_Login_field" ref="user" type="text" value={this.state.username} onChange={this.onUsernameChanged} placeholder="Email or user name" /><br />
                         <input className="mx_Login_field" ref="pass" type="password" value={this.state.password} onChange={this.onPasswordChanged} placeholder="Password" /><br />
-                        {this.componentForStep('choose_hs')}
+                        { this.componentForStep('choose_hs') }
                         <input className="mx_Login_submit" type="submit" value="Log in" />
                         </form>
                     </div>
@@ -143,20 +144,18 @@ module.exports = React.createClass({
     },
 
     loginContent: function() {
-        if (this.state.busy) {
-            return (
-                <Loader />
-            );
-        } else {
-            return (
-                <div>
-                    <h2>Sign in</h2>
-                    {this.componentForStep(this.state.step)}
-                    <div className="mx_Login_error">{this.state.errorText}</div>
-                    <a className="mx_Login_create" onClick={this.showRegister} href="#">Create a new account</a>
+        var loader = this.state.busy ? <div className="mx_Login_loader"><Loader /></div> : null;
+        return (
+            <div>
+                <h2>Sign in</h2>
+                {this.componentForStep(this.state.step)}
+                <div className="mx_Login_error">
+                        { loader }
+                        {this.state.errorText}
                 </div>
-            );
-        }
+                <a className="mx_Login_create" onClick={this.showRegister} href="#">Create a new account</a>
+            </div>
+        );
     },
 
     render: function() {
diff --git a/skins/base/views/templates/Register.js b/skins/base/views/templates/Register.js
index 4bfb8727..10d04c83 100644
--- a/skins/base/views/templates/Register.js
+++ b/skins/base/views/templates/Register.js
@@ -46,10 +46,10 @@ module.exports = React.createClass({
 
     getRegFormVals: function() {
         return {
-            email: this.refs.email.getDOMNode().value,
-            username: this.refs.username.getDOMNode().value,
-            password: this.refs.password.getDOMNode().value,
-            confirmPassword: this.refs.confirmPassword.getDOMNode().value
+            email: this.refs.email.getDOMNode().value.trim(),
+            username: this.refs.username.getDOMNode().value.trim(),
+            password: this.refs.password.getDOMNode().value.trim(),
+            confirmPassword: this.refs.confirmPassword.getDOMNode().value.trim()
         };
     },
 
diff --git a/src/controllers/templates/Login.js b/src/controllers/templates/Login.js
index 7a79dbb2..f3528e33 100644
--- a/src/controllers/templates/Login.js
+++ b/src/controllers/templates/Login.js
@@ -35,7 +35,7 @@ module.exports = {
     },
 
     setStep: function(step) {
-        this.setState({ step: step, errorText: '', busy: false });
+        this.setState({ step: step, busy: false });
     },
 
     onHSChosen: function() {
@@ -45,11 +45,14 @@ module.exports = {
         );
         this.setState({
             hs_url: this.getHsUrl(),
-            is_url: this.getIsUrl()
+            is_url: this.getIsUrl(),
         });
         this.setStep("fetch_stages");
         var cli = MatrixClientPeg.get();
-        this.setState({busy: true});
+        this.setState({
+            busy: true,
+            errorText: "",
+        });
         var self = this;
         cli.loginFlows().done(function(result) {
             self.setState({
@@ -66,7 +69,10 @@ module.exports = {
 
     onUserPassEntered: function(ev) {
         ev.preventDefault();
-        this.setState({busy: true});
+        this.setState({
+            busy: true,
+            errorText: "",
+        });
         var self = this;
 
         var formVals = this.getFormVals();