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

Thanks for the code! See my comments (as we discussed) below.

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

https://codereview.appspot.com/7641043/diff/8001/app/widgets/charm-slider.js#newcode72
app/widgets/charm-slider.js:72: if (Y.Lang.isValue(index)) {
We decided to no longer use isValue() as all it's really doing is a
glorified falsy check

https://codereview.appspot.com/7641043/diff/8001/app/widgets/charm-slider.js#newcode102
app/widgets/charm-slider.js:102: sub(that.itemTemplate, {width: width,
index: index}));
There is no need to do the that=this trick here because (like most
methods of this type in yui) it has a final param which is the context.

http://yuilibrary.com/yui/docs/api/classes/Array.html#method_map

https://codereview.appspot.com/7641043/diff/8001/app/widgets/charm-slider.js#newcode103
app/widgets/charm-slider.js:103: tmpNode.setContent(item);
setContent() is depricated use setHTML()

https://codereview.appspot.com/7641043/diff/8001/app/widgets/charm-slider.js#newcode116
app/widgets/charm-slider.js:116: Y.log('_generateSliderControls',
'info', this.name);
I think we have standardized on using console.log()

https://codereview.appspot.com/7641043/diff/8001/app/widgets/charm-slider.js#newcode119
app/widgets/charm-slider.js:119: Y.Array.each(this.get('items'),
function(item, index) {
Like I mentioned above, YUI's iterators have a context param as the
final param so the that=this can be removed in favour of that.

https://codereview.appspot.com/7641043/diff/8001/app/widgets/charm-slider.js#newcode195
app/widgets/charm-slider.js:195: var index =
this.getAttribute('data-index');
event callbacks are passed an event object which hold reference to the
target.

so this can be changed to e.currentTarget.getAttribute('data-index');

https://codereview.appspot.com/7641043/diff/8001/app/widgets/charm-slider.js#newcode198
app/widgets/charm-slider.js:198: }, 'li'));
delegate() also has a context property

https://codereview.appspot.com/7641043/diff/8001/lib/views/browser/charm-slider.less
File lib/views/browser/charm-slider.less (right):

https://codereview.appspot.com/7641043/diff/8001/lib/views/browser/charm-slider.less#newcode1
lib/views/browser/charm-slider.less:1:
.yui3-browser-charm-slider-content {
Because we are using less you can actually nest most of the rules in
this commit - let me know if you need help doing this.

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