yellow team mailing list archive

[Matthew Scott <matthew.scott@xxxxxxxxxxxxx>]

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/

-- 
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.