Rangeslider: Implement Classes Option #8116
Conversation
add backcompat Fixes jquery-archivegh-7717
add classes Fixes jquery-archivegh-7717
assert classes Fixes jquery-archivegh-7717
assert classes fix Fixes jquery-archivegh-7717
| @@ -58,15 +55,17 @@ | |||
| _sliderFirst = _sliderWidgetFirst.slider, | |||
| _sliderLast = _sliderWidgetLast.slider, | |||
| firstHandle = _sliderWidgetFirst.handle, | |||
| _sliders = $( "<div class='ui-rangeslider-sliders' />" ).appendTo( $el ); | |||
| _sliders = $( "<div />" ); | |||
changes from review Fixes jquery-archivegh-7717
| _inputFirst = $el.find( "input" ).first(), | ||
| _inputLast = $el.find( "input" ).last(), | ||
| _label = $el.find( "label" ).first(), | ||
| var rangeslider = this.element, |
There was a problem hiding this comment.
lets drop the extra var here rangeslider is only 1 char shorter then this.element and gzip likes repeated strings :-)
|
Overall looking good just a couple smallish things. In addition to the comments already there.
|
alter backcompat test file to include core Fixes jquery-archivegh-7717
|
time for another round @arschmitz |
|
|
||
| if ( this.options.mini !== undefined ) { | ||
| this.element.toggleClass( "ui-mini", !!this.options.mini ); | ||
| } |
There was a problem hiding this comment.
Isn't mini already handled by the backcompat?
There was a problem hiding this comment.
Discussed with @cgack on irc there is a bigger issue here to be looked into
move mini to backcompat Fixes jquery-archivegh-7717
|
@arschmitz I moved ui-mini to backcompat but could not fully remove it to get the classes applied correctly for the demo to look correct. Lemme know if you see something i'm missing when you review again. |
|
The 2 remaining changes are minor you can go ahead make them rebase and land i don't need to look again. 👍 |
|
landed with b6e619e |
Fixes gh-7717