First stage of upgrade UX

First stage of upgrade UX

This branch implements the first pass at the upgrade UX. That is,
when changing the value for the unit count, the current unit display
will show a spinning indicator until the two numbers match. It 
also now confirms that the user wants to deploy these new units 
with the current constraint values.

This also includes a driveby fix for the width of the status bar and 
a patch to fix a bug with the databinding throttling (a new test was
added).

https://code.launchpad.net/~hatch/juju-gui/scale-up-ui/+merge/180634

(do not edit description out of merge proposal)

[jeff.pihach, 2013-08-16 18:21:51]

Please take a look.

[benji, 2013-08-16 19:06:04]

LGTM with a few comments.

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

https://codereview.appspot.com/12784046/diff/1/app/templates/serviceOverview....
app/templates/serviceOverview.handlebars:12: <div class="units-starting-spinner
yui3-u hidden"><img src="/juju-ui/assets/images/non-sprites/units-loading.gif"
/></div>
It would be nice if we could break this long line.

https://codereview.appspot.com/12784046/diff/1/app/views/inspector.js
File app/views/inspector.js (right):

https://codereview.appspot.com/12784046/diff/1/app/views/inspector.js#newcode122
app/views/inspector.js:122: @param {Number} requestedUnitCount the number of
units to create.
It's nice to have a "@return {undefined} Nothing" so people don't have to
inspect the code to tell the difference between "returns nothing" and "I forgot
to write down what it returns".

https://codereview.appspot.com/12784046/diff/1/app/views/inspector.js#newcode136
app/views/inspector.js:136: @method _closeUnitConfirm
Did you hear?   We won the lottery!  Now we can afford non-single-character
variable names!

Also, and @param for the parameter.

https://codereview.appspot.com/12784046/diff/1/app/views/viewlets/service-ove...
File app/views/viewlets/service-overview.js (right):

https://codereview.appspot.com/12784046/diff/1/app/views/viewlets/service-ove...
app/views/viewlets/service-overview.js:266: 'update': function(node, value) {
This needs some yuidoc love.  I'm surprised the linter didn't complain.

https://codereview.appspot.com/12784046/diff/1/test/test_conflict_ux.js
File test/test_conflict_ux.js (right):

https://codereview.appspot.com/12784046/diff/1/test/test_conflict_ux.js#newco...
test/test_conflict_ux.js:142: 'missing conflict-pending class');
Wouldn't isTrue be the more common way to spell this?

https://codereview.appspot.com/12784046/diff/1/test/test_inspector_overview.js
File test/test_inspector_overview.js (right):

https://codereview.appspot.com/12784046/diff/1/test/test_inspector_overview.j...
test/test_inspector_overview.js:133: // confirm the constraints
How about "Initiate a change to the number of units."

[bac, 2013-08-16 19:53:39]

LGTM for the code and the QA with your patch was fine.  While doing the QA I did
notice in the unit count the phrase "3 Needs Reboot".  Can that text either be
made conditional on the number of units ("1 Needs Reboot"  and "n Need Reboot"
if n>1) or be set to "Need Reboot" as the most common case?

I did see one instance where the spinner did not go away after the desired unit
number was reached, though I could not reproduce it.

Thanks!

https://codereview.appspot.com/12784046/diff/1/app/views/inspector.js
File app/views/inspector.js (right):

https://codereview.appspot.com/12784046/diff/1/app/views/inspector.js#newcode142
app/views/inspector.js:142: // If this was from the user clicking cancel;
So you're ending your sentences with semi-colons now?  Too much JS for you!

https://codereview.appspot.com/12784046/diff/1/lib/views/juju-inspector.less
File lib/views/juju-inspector.less (right):

https://codereview.appspot.com/12784046/diff/1/lib/views/juju-inspector.less#...
lib/views/juju-inspector.less:22: background: #dd4814;
Please use the symbol @confirm-button-color

[jeff.pihach, 2013-08-16 20:26:23]

Thanks for the reviews!

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

https://codereview.appspot.com/12784046/diff/1/app/templates/serviceOverview....
app/templates/serviceOverview.handlebars:12: <div class="units-starting-spinner
yui3-u hidden"><img src="/juju-ui/assets/images/non-sprites/units-loading.gif"
/></div>
On 2013/08/16 19:06:04, benji wrote:
> It would be nice if we could break this long line.

Done.

https://codereview.appspot.com/12784046/diff/1/app/views/inspector.js
File app/views/inspector.js (right):

https://codereview.appspot.com/12784046/diff/1/app/views/inspector.js#newcode142
app/views/inspector.js:142: // If this was from the user clicking cancel;
Haha, fixed!

https://codereview.appspot.com/12784046/diff/1/app/views/viewlets/service-ove...
File app/views/viewlets/service-overview.js (right):

https://codereview.appspot.com/12784046/diff/1/app/views/viewlets/service-ove...
app/views/viewlets/service-overview.js:266: 'update': function(node, value) {
This isn't a new method being defined here - these are all defined in the
viewlet code.

https://codereview.appspot.com/12784046/diff/1/lib/views/juju-inspector.less
File lib/views/juju-inspector.less (right):

https://codereview.appspot.com/12784046/diff/1/lib/views/juju-inspector.less#...
lib/views/juju-inspector.less:22: background: #dd4814;
On 2013/08/16 19:53:39, bac wrote:
> Please use the symbol @confirm-button-color

Done.

https://codereview.appspot.com/12784046/diff/1/test/test_inspector_overview.js
File test/test_inspector_overview.js (right):

https://codereview.appspot.com/12784046/diff/1/test/test_inspector_overview.j...
test/test_inspector_overview.js:133: // confirm the constraints
Changed

[jeff.pihach, 2013-08-16 20:37:54]

*** Submitted:

First stage of upgrade UX

This branch implements the first pass at the upgrade UX. That is,
when changing the value for the unit count, the current unit display
will show a spinning indicator until the two numbers match. It 
also now confirms that the user wants to deploy these new units 
with the current constraint values.

This also includes a driveby fix for the width of the status bar and 
a patch to fix a bug with the databinding throttling (a new test was
added).

R=benji, bac
CC=
https://codereview.appspot.com/12784046