Improve login UX

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://code.launchpad.net/~gary/juju-gui/login-ui/+merge/142599

(do not edit description out of merge proposal)

[gary.poster, 2013-01-09 21:38:39]

Please take a look.

[matthew.scott, 2013-01-10 00:32:12]

This looks good to me, land with a few minors (or correct me if I'm wrong!).

Works well for me.

https://codereview.appspot.com/7060066/diff/1/app/templates/login.handlebars
File app/templates/login.handlebars (right):

https://codereview.appspot.com/7060066/diff/1/app/templates/login.handlebars#...
app/templates/login.handlebars:8: <input type="text" disabled="disabled"
value="admin"></input>
In general, <a> and <script> are the only empty tags.  input (along with hr, b,
etc) is usually a void tag, meaning that it self-closes like <input />.  I
belieeeeve that's what's done elsewhere?  I'll need to check, but I suppose
matching the current standard we use is best.

https://codereview.appspot.com/7060066/diff/1/lib/views/stylesheet.less
File lib/views/stylesheet.less (right):

https://codereview.appspot.com/7060066/diff/1/lib/views/stylesheet.less#newco...
lib/views/stylesheet.less:1406: 
Indenting is a mix of 2 and 4 here (which, to be fair, is the case for the whole
file.  Maybe this is something to look into in the future for the rest of the
file, but could be solidified for this bit below.

https://codereview.appspot.com/7060066/diff/1/test/test_login.js
File test/test_login.js (right):

https://codereview.appspot.com/7060066/diff/1/test/test_login.js#newcode119
test/test_login.js:119: test('the view login method calls the environment login
one', function() {
The 'one' at the end of the test description is ambiguous; thought it was
referring to view, which may be due to our dual use of 'environment' for the
view and the WS layer.  'the view login method logs in through the environment'
maybe?

[matthew.scott, 2013-01-10 00:40:06]

On 2013/01/10 00:32:12, matthew.scott wrote:
> This looks good to me, land with a few minors (or correct me if I'm wrong!).
> 
> Works well for me.
> 
> https://codereview.appspot.com/7060066/diff/1/app/templates/login.handlebars
> File app/templates/login.handlebars (right):
> 
>
https://codereview.appspot.com/7060066/diff/1/app/templates/login.handlebars#...
> app/templates/login.handlebars:8: <input type="text" disabled="disabled"
> value="admin"></input>
> In general, <a> and <script> are the only empty tags.  input (along with hr,
b,

Should be br, sorry!

> etc) is usually a void tag, meaning that it self-closes like <input />.  I
> belieeeeve that's what's done elsewhere?  I'll need to check, but I suppose
> matching the current standard we use is best.
> 
> https://codereview.appspot.com/7060066/diff/1/lib/views/stylesheet.less
> File lib/views/stylesheet.less (right):
> 
>
https://codereview.appspot.com/7060066/diff/1/lib/views/stylesheet.less#newco...
> lib/views/stylesheet.less:1406: 
> Indenting is a mix of 2 and 4 here (which, to be fair, is the case for the
whole
> file.  Maybe this is something to look into in the future for the rest of the
> file, but could be solidified for this bit below.
> 
> https://codereview.appspot.com/7060066/diff/1/test/test_login.js
> File test/test_login.js (right):
> 
> https://codereview.appspot.com/7060066/diff/1/test/test_login.js#newcode119
> test/test_login.js:119: test('the view login method calls the environment
login
> one', function() {
> The 'one' at the end of the test description is ambiguous; thought it was
> referring to view, which may be due to our dual use of 'environment' for the
> view and the WS layer.  'the view login method logs in through the
environment'
> maybe?

[frankban, 2013-01-10 10:13:02]

Land with changes suggested by Matthew.

Thanks Gary for your fixes to the test problems we were encountering yesterday:
this branch is really cool, and you did most of the work on it anyway! :-)

As agreed, we (Nicola and I) will fix the remaining issues and land this branch
ourselves.

[frankban, 2013-01-10 10:57:53]

Thanks for the review Matthew.
This branch has been fixed and landed.
See https://codereview.appspot.com/7060068

https://codereview.appspot.com/7060066/diff/1/app/templates/login.handlebars
File app/templates/login.handlebars (right):

https://codereview.appspot.com/7060066/diff/1/app/templates/login.handlebars#...
app/templates/login.handlebars:8: <input type="text" disabled="disabled"
value="admin"></input>
On 2013/01/10 00:32:12, matthew.scott wrote:
> In general, <a> and <script> are the only empty tags.  input (along with hr,
b,
> etc) is usually a void tag, meaning that it self-closes like <input />.  I
> belieeeeve that's what's done elsewhere?  I'll need to check, but I suppose
> matching the current standard we use is best.

Done.

https://codereview.appspot.com/7060066/diff/1/lib/views/stylesheet.less
File lib/views/stylesheet.less (right):

https://codereview.appspot.com/7060066/diff/1/lib/views/stylesheet.less#newco...
lib/views/stylesheet.less:1406: 
On 2013/01/10 00:32:12, matthew.scott wrote:
> Indenting is a mix of 2 and 4 here (which, to be fair, is the case for the
whole
> file.  Maybe this is something to look into in the future for the rest of the
> file, but could be solidified for this bit below.

Done. Indented 4 spaces.

https://codereview.appspot.com/7060066/diff/1/test/test_login.js
File test/test_login.js (right):

https://codereview.appspot.com/7060066/diff/1/test/test_login.js#newcode119
test/test_login.js:119: test('the view login method calls the environment login
one', function() {
On 2013/01/10 00:32:12, matthew.scott wrote:
> The 'one' at the end of the test description is ambiguous; thought it was
> referring to view, which may be due to our dual use of 'environment' for the
> view and the WS layer.  'the view login method logs in through the
environment'
> maybe?

Done.