yellow team mailing list archive

[Gary Poster <gary.poster@xxxxxxxxxxxxx>]

Gary Poster has proposed merging lp:~gary/juju-gui/login-ui into lp:juju-gui.

Requested reviews:
  Juju GUI Hackers (juju-gui)
Related bugs:
  Bug #1097272 in juju-gui: "Implement username/password login UI"
  https://bugs.launchpad.net/juju-gui/+bug/1097272

For more details, see:
https://code.launchpad.net/~gary/juju-gui/login-ui/+merge/142599

Improve login UX

Use the login UX provided by our designers.  This also means that help text can advise what password to use, and that we can retry authentication after a failed password.

https://codereview.appspot.com/7060066/

-- 
https://code.launchpad.net/~gary/juju-gui/login-ui/+merge/142599
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~gary/juju-gui/login-ui into lp:juju-gui.

=== modified file 'app/app.js'
--- app/app.js	2013-01-09 15:55:25 +0000
+++ app/app.js	2013-01-09 21:39:19 +0000
@@ -244,7 +244,7 @@
       this.env.after('providerTypeChange', this.onProviderTypeChange);
 
       // Once the user logs in we need to redraw.
-      this.env.after('login', this.dispatch, this);
+      this.env.after('login', this.onLogin, this);
 
       // Feed environment changes directly into the database.
       this.env.on('delta', this.db.on_delta, this.db);
@@ -534,36 +534,51 @@
      *
      */
     check_user_credentials: function(req, res, next) {
-      var viewInfo = this.getViewInfo('login');
-      if (!viewInfo.instance) {
-        viewInfo.instance = new views.LoginView({
-          app: this,
-          env: this.env
-        });
+      if (!this.get('container').getDOMNode()) {
+        // We are probably after a test tear down, in a handler for a
+        // setTimeout in the YUI router.  Ugh, setTimeout is the source of
+        // so many fun race conditions and fragilities. :-/  Let's just bail.
+        return;
       }
-      var view = viewInfo.instance;
-      // If there are no stored credentials the user is prompted for some.
+      // If there are no stored credentials, the user is prompted for some.
       var user = this.env.get('user');
       var password = this.env.get('password');
       if (!Y.Lang.isValue(user) || !Y.Lang.isValue(password)) {
-        view.promptUser();
+        this.showView('login', {
+          env: this.env,
+          help_text: this.get('login_help')
+        });
       }
-      // If there are credentials available and there has not been a successful
-      // login attempt and we are not waiting on a login attempt, try to log in.
+      // If there are credentials available and there has not been
+      // a successful login attempt, try to log in.
       if (Y.Lang.isValue(user) && Y.Lang.isValue(password) &&
-          !this.env.waiting && !this.env.userIsAuthenticated) {
+          !this.env.userIsAuthenticated) {
         this.env.login();
         return;
       }
-      // If there has not been a successful login attempt, do not let the route
-      // dispatch proceed.
-      if (this.env.waiting || !this.env.userIsAuthenticated) {
+      // If there has not been a successful login attempt,
+      // do not let the route dispatch proceed.
+      if (!this.env.userIsAuthenticated) {
         return;
       }
       next();
     },
 
     /**
+     * Hide the login mask and redispatch the router.
+     *
+     * When the environment gets a response from a login attempt,
+     * it fires a login event, to which this responds.
+     *
+     * @method onLogin
+     * @private
+     */
+    onLogin: function() {
+      Y.one('body > #login-mask').hide();
+      this.dispatch();
+    },
+
+    /**
      * Display the provider type.
      *
      * The provider type arrives asynchronously.  Instead of updating the
@@ -573,11 +588,8 @@
      * @method onProviderTypeChange
      */
     onProviderTypeChange: function(evt) {
-      var providerType = evt.newVal,
-          providerNode = Y.one('#provider-type');
-      if (Y.Lang.isValue(providerType)) {
-        providerNode.set('text', 'on ' + providerType);
-      }
+      var providerType = evt.newVal;
+      Y.all('.provider-type').set('text', 'on ' + providerType);
     },
 
     /**

=== modified file 'app/config-debug.js'
--- app/config-debug.js	2012-12-05 12:33:09 +0000
+++ app/config-debug.js	2013-01-09 21:39:19 +0000
@@ -7,5 +7,6 @@
   // FIXME: turn off transitions until they are fixed.
   transitions: false,
   charm_store_url: 'http://jujucharms.com/',
-  socket_url: 'ws://localhost:8081/ws'
+  socket_url: 'ws://localhost:8081/ws',
+  login_help: 'For this demonstration, use the "admin" password to connect.'
 };

=== modified file 'app/config-prod.js'
--- app/config-prod.js	2012-12-13 02:43:15 +0000
+++ app/config-prod.js	2013-01-09 21:39:19 +0000
@@ -7,5 +7,8 @@
   // FIXME: turn off transitions until they are fixed.
   transitions: false,
   charm_store_url: 'http://jujucharms.com/',
-  socket_url: 'ws://localhost:8081/ws'
+  socket_url: 'ws://localhost:8081/ws',
+  login_help: (
+      'The password is the admin-secret from the Juju environment.  This can ' +
+      'often be found by looking in ~/.juju/environments.yaml.')
 };

=== modified file 'app/index.html'
--- app/index.html	2013-01-04 09:11:46 +0000
+++ app/index.html	2013-01-09 21:39:19 +0000
@@ -18,7 +18,7 @@
   </head>
 
   <body>
-
+      <div id="login-mask" class="crosshatch-background"></div>
       <div id="notifier-box"></div>
       <div id="viewport-wrapper">
         <div id="vp-left-border"></div>

=== modified file 'app/store/env.js'
--- app/store/env.js	2013-01-09 14:57:34 +0000
+++ app/store/env.js	2013-01-09 21:39:19 +0000
@@ -37,6 +37,7 @@
       this._txn_callbacks = {};
       // Consider the user unauthenticated until proven otherwise.
       this.userIsAuthenticated = false;
+      this.failedAuthentication = false;
       // When the server tells us the outcome of a login attempt we record
       // the result.
       this.on('login', this.handleLoginEvent, this);
@@ -100,11 +101,11 @@
     handleLoginEvent: function(evt) {
       // We are only interested in the responses to login events.
       this.userIsAuthenticated = !!evt.data.result;
-      this.waiting = false;
       // If the credentials were rejected remove them.
       if (!this.userIsAuthenticated) {
         this.set('user', undefined);
         this.set('password', undefined);
+        this.failedAuthentication = true;
       }
     },
 

=== added file 'app/templates/login.handlebars'
--- app/templates/login.handlebars	1970-01-01 00:00:00 +0000
+++ app/templates/login.handlebars	2013-01-09 21:39:19 +0000
@@ -0,0 +1,16 @@
+<div id="login-column">
+  <i class="sprite juju_logo" title="Juju GUI"></i>
+  <div id="login-form">
+    <div id="login-header">
+      {{environment_name}} <span class="provider-type">{{provider_type}}</span>
+    </div>
+    <form>
+      <input type="text" disabled="disabled" value="admin"></input>
+      <input type="password" value="" placeholder="Enter password"></input>
+      <input type="submit" value="Login"></input>
+    </form>
+  </div>
+  <div class="error">{{error_text}}</div>
+  <div>{{help_text}}</div>
+  <div><a href="https://juju.ubuntu.com/"; target="_blank">juju.ubuntu.com</a></div>
+</div>

=== modified file 'app/views/login.js'
--- app/views/login.js	2013-01-09 14:16:34 +0000
+++ app/views/login.js	2013-01-09 21:39:19 +0000
@@ -8,44 +8,75 @@
   var views = Y.namespace('juju.views');
 
   var LoginView = Y.Base.create('LoginView', Y.View, [views.JujuBaseView], {
-    // This is so tests can easily determine if the user was prompted.
-    _prompted: false,
-
-    /**
-     * Prompt the user for input.
-     *
-     * Does nothing if a special global flag has been set.  This is used so
-     * tests do not generate prompts.
-     *
-     * @method _prompt
-     * @param {String} message The message to display to the user.
-     * @return {String} The string the user typed.
-     */
-    _prompt: function(message) {
-      // noLogin is a global.
-      if (noLogin) {
-        return null;
-      }
-      return window.prompt(message);
-    },
-
-    /**
-     * Prompt the user for their user name and password.
-     *
-     * @method promptUser
-     * @return {undefined} Nothing.
-     */
-    promptUser: function() {
-      this._prompted = true;
-      var env = this.get('env');
-      // The user's name is always "admin".
-      env.set('user', 'admin');
-      env.set('password', this._prompt('Password'));
+    template: views.Templates.login,
+    events: {
+      '#login-form input[type=submit]': {click: 'login'}
+    },
+
+    /**
+     * Login event handler. When clicking the login form submit button,
+     * disable the form, take username and password from the input fields
+     * and put them in the environment, and call the environment login method.
+     *
+     * @method login
+     * @param {Object} ev An event object (with a "currentTarget" attribute).
+     * @return {undefined} Mutates only.
+    **/
+    login: function(ev) {
+      ev.preventDefault();
+      if (ev.currentTarget.get('disabled')) {
+        return;
+      }
+      var env = this.get('env');
+      var container = this.get('container');
+      container.all('input').set('disabled', true);
+      env.set('user', container.one('input[type=text]').get('value'));
+      env.set('password', container.one('input[type=password]').get('value'));
+      env.login();
+    },
+
+    /**
+     * Render the page.
+     *
+     * Reveal the mask element, and show the login form.
+     *
+     * @method render
+     * @return {undefined} Mutates only.
+    **/
+    render: function() {
+      // In order to have the mask cover everything, it needs to be an
+      // immediate child of the body.  In order for it to render immediately
+      // when the app loads, it needs to be in index.html.
+      var loginMask = Y.one('body > #login-mask');
+      if (!loginMask) {
+        // No login mask in the DOM, as is the case in tests.
+        return this;
+      }
+      loginMask.show();
+      var env = this.get('env');
+      var environment_name_node = Y.one('#environment-name');
+      var provider_type_node = Y.one('#provider-type');
+      var environment_name = (
+          environment_name_node ?
+          environment_name_node.get('text') : 'Environment');
+      var provider_type = (
+          provider_type_node ? provider_type_node.get('text') : '');
+      // In order to have events work and the view cleanly be replaced by
+      // other views, we need to put the contents in the usual "container"
+      // node, even though it is not a child of the mask node.
+      this.get('container').setHTML(this.template({
+        environment_name: environment_name,
+        provider_type: provider_type,
+        error_text: (
+            env.failedAuthentication ? 'Unknown user or password.' : ''),
+        help_text: this.get('help_text')
+      }));
+      return this;
     }
 
   });
 
-  views.LoginView = LoginView;
+  views.login = LoginView;
 
 }, '0.1.0', {
   requires: [
@@ -54,4 +85,3 @@
     'node'
   ]
 });
-

=== modified file 'lib/views/stylesheet.less'
--- lib/views/stylesheet.less	2013-01-08 14:02:06 +0000
+++ lib/views/stylesheet.less	2013-01-09 21:39:19 +0000
@@ -27,6 +27,7 @@
     margin-top: 87px;
     background-color: #302b28;
     font-family: @font-family;
+    height: 100%;
 }
 
 input, button, select, textarea {
@@ -1402,3 +1403,95 @@
     margin-top: 5px;
   }
 }
+
+#login-mask {
+  display: block;
+  position: absolute;
+  top: 0;
+  left: 0;
+  width: 100%;
+  height: 100%;
+  z-index: 10000;
+}
+#login-column {
+  display: block;
+  z-index: 10001;
+  position: absolute;
+  top: 80px;
+  left: 50%;
+  text-align: center;
+  width: 300px;
+  margin-left: -150px;
+
+  div {
+    width: 220px;
+    text-align: left;
+    margin: 0 auto;
+    margin-bottom: 1ex;
+  }
+  a {
+      color: @charm-panel-orange;
+      &:link, &:visited, &:active, &:hover {
+         text-decoration: none
+      }
+  }
+  @login-panel-color: #f5f5f5;
+  @login-panel-gradient: (@login-panel-color - #111);
+  @login-panel-shadow: (@login-panel-color - #444);
+  @curve-radius: ~"24px 28px";
+  div#login-form {
+    input {
+      margin-top: 8px;
+      margin-bottom: 8px;
+    }
+    input[type=submit] {
+      width: 220px;
+      height: 26px;
+      text-transform: uppercase;
+      text-shadow: none;
+      background-position: 0 0;     /* Needed to defeat bootstrap btn:hover 0 -15px*/
+      .create-button(@charm-panel-deploy-button-color,
+                     @charm-panel-deploy-button-bottom-gradient,
+                     @charm-panel-deploy-button-shadow, 0px);
+      &:active {
+          .create-button(@charm-panel-deploy-button-color,
+                         @charm-panel-deploy-button-color,
+                         @charm-panel-deploy-button-shadow, 1px);
+      }
+      color: white;
+    }
+    padding: 5px 25px 5px;
+    margin-top: 3ex;
+    margin-bottom: 4ex;
+    .create-button(@login-panel-color,
+                   @login-panel-gradient,
+                   @login-panel-shadow,
+                   -6px);
+    .create-border-radius(~"24px / 28px");
+    div#login-header {
+      padding: 15px 25px 10px 25px;
+      margin-left: -25px;
+      margin-top: -5px;
+      -webkit-border-top-left-radius: @curve-radius;
+      -webkit-border-top-right-radius: @curve-radius;
+      -moz-border-radius-topleft: @curve-radius;
+      -moz-border-radius-topright: @curve-radius;
+      border-top-left-radius: @curve-radius;
+      border-top-right-radius: @curve-radius;
+      @gradient-start: @login-panel-gradient;
+      @gradient-end: @login-panel-shadow;
+      background-image: -ms-linear-gradient(top, @gradient-start, @gradient-end);
+      background-image: -webkit-gradient(linear, 0 0, 0 100%, from(@gradient-start), to(@gradient-end));
+      background-image: -webkit-linear-gradient(top, @gradient-start, @gradient-end);
+      background-image: -o-linear-gradient(top, @gradient-start, @gradient-end);
+      background-image: -moz-linear-gradient(top, @gradient-start, @gradient-end);
+      background-image: linear-gradient(top, @gradient-start, @gradient-end);
+      background-repeat: repeat-x;
+
+    }
+  }
+  .error {
+      color: red;
+      font-weight: bold;
+  }
+}
\ No newline at end of file

=== modified file 'test/index.html'
--- test/index.html	2013-01-09 15:55:25 +0000
+++ test/index.html	2013-01-09 21:39:19 +0000
@@ -38,7 +38,7 @@
   <script src="test_charm_panel.js"></script>
   <script src="test_charm_store.js"></script>
   <script src="test_charm_view.js"></script>
-  <script src="test_console.js"></script> 
+  <script src="test_console.js"></script>
   <script src="test_d3_components.js"></script>
   <script src="test_environment_view.js"></script>
   <script src="test_env.js"></script>
@@ -72,8 +72,7 @@
 </head>
 
 <body>
-  <div id="main" class="container">
-  </div>
+  <div id="main" class="container"></div>
   <div id="mocha"></div>
 </body>
 </html>

=== modified file 'test/test_app.js'
--- test/test_app.js	2013-01-09 15:55:25 +0000
+++ test/test_app.js	2013-01-09 21:39:19 +0000
@@ -52,7 +52,7 @@
           .append(Y.Node.create('<span/>')
             .set('id', 'environment-name'))
           .append(Y.Node.create('<span/>')
-            .set('id', 'provider-type'))
+            .addClass('provider-type'))
           .hide();
       app = new Y.juju.App(
           { container: container,
@@ -115,12 +115,12 @@
     it('should show the provider type, when available', function() {
       var providerType = 'excellent provider';
       // Since no provider type has been set yet, none is displayed.
-      assert.equal('', container.one('#provider-type').get('text'));
+      assert.equal('', container.one('.provider-type').get('text'));
       app.env.set('providerType', providerType);
       // The provider type has been displayed.
       assert.equal(
           'on ' + providerType,
-          container.one('#provider-type').get('text')
+          container.one('.provider-type').get('text')
       );
     });
 

=== modified file 'test/test_login.js'
--- test/test_login.js	2013-01-09 15:55:25 +0000
+++ test/test_login.js	2013-01-09 21:39:19 +0000
@@ -4,17 +4,18 @@
 
   describe('environment login support', function() {
     var requires = ['node', 'juju-gui', 'juju-views', 'juju-tests-utils'];
-    var Y, conn, env, utils, juju, makeLoginView, views, app;
+    var Y, conn, env, utils, juju;
     var test = it; // We aren't really doing BDD so let's be more direct.
 
-    before(function() {
-      Y = YUI(GlobalConfig).use(requires);
-      utils = Y.namespace('juju-tests.utils');
-      juju = Y.namespace('juju');
+    before(function(done) {
+      Y = YUI(GlobalConfig).use(requires, function(Y) {
+        utils = Y.namespace('juju-tests').utils;
+        juju = Y.namespace('juju');
+        done();
+      });
     });
 
     beforeEach(function() {
-      var container = Y.Node.create('<div/>');
       conn = new utils.SocketStub();
       env = new juju.Environment({conn: conn});
       env.connect();
@@ -82,4 +83,59 @@
 
   });
 
+
+  describe('login view', function() {
+    var requires = ['node', 'juju-gui', 'juju-views', 'juju-tests-utils'];
+    var Y, conn, env, utils, juju, views, loginView, container, loginMask;
+    var test = it; // We aren't really doing BDD so let's be more direct.
+
+    before(function(done) {
+      Y = YUI(GlobalConfig).use(requires, function(Y) {
+        utils = Y.namespace('juju-tests').utils;
+        juju = Y.namespace('juju');
+        views = Y.namespace('juju.views');
+        done();
+      });
+    });
+
+    beforeEach(function() {
+      conn = new utils.SocketStub();
+      env = new juju.Environment({conn: conn});
+      env.connect();
+      conn.open();
+      container = Y.one('body').appendChild('<div/>');
+      // Needed by the render method.
+      loginMask = Y.one('body').appendChild('<div/>').set('id', 'login-mask');
+      loginView = new views.login(
+          {container: container, env: env, help_text: 'Help text'});
+    });
+
+    afterEach(function() {
+      env.destroy();
+      container.remove(true);
+      loginMask.remove(true);
+    });
+
+    test('the view login method calls the environment login one', function() {
+      var noop = function() {};
+      var ev = {preventDefault: noop, currentTarget: {get: noop}};
+      container.appendChild('<input/>').set('type', 'text').set(
+          'value', 'user');
+      container.appendChild('<input/>').set('type', 'password').set(
+          'value', 'password');
+      loginView.login(ev);
+      assert.equal(conn.last_message().op, 'login');
+      assert.equal(conn.last_message().user, 'user');
+      assert.equal(conn.last_message().password, 'password');
+    });
+
+    test('the view render method adds the login form', function() {
+      loginView.render();
+      assert.equal(
+          container.one('#login-form input[type=submit]').get('value'),
+          'Login');
+    });
+
+  });
+
 })();