Merge lp://staging/~gary/juju-gui/login-ui into lp://staging/juju-gui/experimental

Reviewers: mp+142599_code.launchpad.net,

Message:
Please take a look.

Description:
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)

Please review this at https://codereview.appspot.com/7060066/

Affected files:
   A [revision details]
   M app/app.js
   M app/config-debug.js
   M app/config-prod.js
   M app/index.html
   M app/store/env.js
   A app/templates/login.handlebars
   M app/views/login.js
   M lib/views/stylesheet.less
   M test/index.html
   M test/test_app.js
   M test/test_login.js

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#newcode8
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#newcode1406
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?

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

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#newcode8
> 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#newcode1406
> 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?

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

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.

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

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#newcode8
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#newcode1406
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.

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