Code review comment for lp:~jcsackett/juju-gui/charm-slider

Revision history for this message
j.c.sackett (jcsackett) wrote :

Agree with most comments, address the remainder. Code is coming up
shortly.

https://codereview.appspot.com/7641043/diff/1/app/widgets/charm-slider.js
File app/widgets/charm-slider.js (right):

https://codereview.appspot.com/7641043/diff/1/app/widgets/charm-slider.js#newcode21
app/widgets/charm-slider.js:21: *
On 2013/03/08 15:12:02, rharding wrote:
> In the doc string can you put a sample of min. html needed to use the
widget?
> Want to include enough to reuse without too much hunting.

Per discussion, this doesn't actual require any setup; just pass in
parentNode to render, as with default Y.Widget.

https://codereview.appspot.com/7641043/diff/1/app/widgets/charm-slider.js#newcode128
app/widgets/charm-slider.js:128: index = parseInt(index, 10);
On 2013/03/08 15:12:02, rharding wrote:
> why does index need to be parsed? How is it coming in as a string?

We get index out of a data-* attribute on the element, but I have moved
the parseInt to where we fetch it, so it's an int when it hits this
method.

https://codereview.appspot.com/7641043/

« Back to merge proposal