Slider: bugfix and improving implements #4678

flytreeleft · 2016-10-22T10:11:34Z

There are following changes:

Code cleaning and format;
Add slider container padding when locating labels;
Use css function calc() to locate thumb, track fill and labels, so no need to listen resize event or observe element changes;
Extra parameter thumbVal, secondThumbVal for listener onChange() and onMove();
Exactly updating global variables value, module.thumbVal, module.secondThumbVal, position and secondPos;
Set or update value to min or max when mouse is moving out slider;

…dating when opening Chrome DevTools

…) and settings.onMove()

bdaunton · 2016-10-22T16:44:12Z

Found a bit of an issue when I set the min to something other then 0

bdaunton · 2016-10-22T16:50:14Z

src/definitions/modules/slider.js

-                    :
-                    module.is.reversed() ? 'right' : 'left'
+                  $label = $('<li class="label">' + module.get.label(i) + '</li>'),
+                  ratio  = i / (len + 1)


this isn't quite right. If I have a max of 10 and a min of 0, I am going to have 8 labels. So this would be 1/9 not 1/10 as it should be.

With the change to module.get.numLables(): i / (len + 1) -> i / len

bdaunton · 2016-10-22T16:53:30Z

src/definitions/modules/slider.js

                $labels = $module.append('<ul class="auto labels"></ul>').find('.labels');
-              for(var i = 0; i <= module.get.numLabels(); i++) {
+              }
+              for(var i = 1, len = module.get.numLabels(); i <= len; i++) {


i <= len -> i < len to go along with the change in module.get.numLables():

var value = Math.round((module.get.max() - module.get.min()) / module.get.step()) - 2;

should now be

var value = Math.round((module.get.max() - module.get.min()) / module.get.step());

bdaunton · 2016-10-22T17:01:03Z

src/definitions/modules/slider.js

            ;
            $children.each(function(index) {
              var
-                $child = $(this),
+                $child    = $(this),


nit: no need for more whitespace

flytreeleft · 2016-10-23T09:44:09Z

@gdaunton Thanks, all known issues are already fixed. :)

bdaunton · 2016-11-02T01:32:50Z

Found an issue with the double handle:

It works fine after I grab the second handle.

This is not an issue on the vertical variant for some reason.

bdaunton · 2016-11-02T01:39:14Z

Really love the calc solution on this 👍 .

I think it is a tiny bit slower, but it does get rid of the positioning issues when the slider width changes.

Thanks for the help!

bdaunton

A few things that need to be removed thanks to the awesome calc solution. Other then that LGTM! 🎉

bdaunton · 2016-11-02T01:42:29Z

src/definitions/modules/slider.js

@@ -122,7 +122,7 @@ $.fn.slider = function(parameters) {
          module.unbind.events();
          module.unbind.slidingEvents();
          $module.removeData(moduleNamespace);
-          module.disconnect.sliderObserver();
+          // module.disconnect.sliderObserver();


Go ahead and remove this line and function if it is no longer used

Also you can probably remove the resize listener

bdaunton · 2016-11-02T01:50:34Z

src/definitions/modules/slider.js

@@ -244,7 +243,7 @@ $.fn.slider = function(parameters) {
        bind: {
          events: function() {
            module.bind.globalKeyboardEvents();
-            module.bind.resizeListener();
+            // module.bind.resizeListener();


bdaunton · 2016-11-02T01:50:45Z

src/definitions/modules/slider.js

@@ -309,7 +308,7 @@ $.fn.slider = function(parameters) {
            $module.off('touchstart' + eventNamespace);
            $module.off('keydown' + eventNamespace);
            $module.off('focusout' + eventNamespace);
-            $(window).off('resize' + eventNamespace);
+            // $(window).off('resize' + eventNamespace);


bdaunton · 2016-11-02T01:53:02Z

src/definitions/modules/slider.js

@@ -103,9 +103,9 @@ $.fn.slider = function(parameters) {
          module.read.metadata();
          module.read.settings();

-          module.observeChanges();
+          // module.observeChanges();


bdaunton · 2016-11-02T01:54:23Z

src/definitions/modules/slider.js

@@ -138,6 +138,9 @@ $.fn.slider = function(parameters) {
          slider: function() {
            sliderObserver.observe($module[0], {


Do we still need to have an observer? I can't think of anything in the class or style that would change where we would need to re-sync everything as all the positioning is percentage based.

bdaunton · 2016-11-02T01:57:07Z

src/definitions/modules/slider.less

  transform: translateX(-50%);
 }

 .ui.labeled.vertical.slider > .labels .label {
-  transform: translate(-100%, -@labelWidth);
+  transform: translate(-100%, -50%); // translate(-100%, -@labelWidth);


you can 🔪 the comment

flytreeleft · 2016-11-06T10:11:57Z

@gdaunton The commented codes are all removed.

And, I create a demo which can be used to test new changes. The issue you mentioned should already be fixed in previous submits :)

flytreeleft added 9 commits October 22, 2016 16:12

'.ui.vertical.slider.inner' -> '.ui.vertical.slider .inner'

7699b6e

Fixed missing comma, semicolon and global variable

6f1ef35

Only observing 'class' and 'style' changes to avoid endless slider up…

c751726

…dating when opening Chrome DevTools

'$.fn.range.settings' -> '$.fn.slider.settings'

d97af7c

Single line if/else clause should wrap with '{}'

ec8fc98

The label position should plus the padding of the slider container

a82f746

Using css function 'calc()' to locate thumb and track fill

249dfc9

Add extra parameters 'thumbVal, secondThumbVal' to settings.onChange(…

b12481d

…) and settings.onMove()

Format code

8a35efb

flytreeleft mentioned this pull request Oct 22, 2016

Range slider module #4212

Closed

bdaunton suggested changes Oct 22, 2016

View reviewed changes

flytreeleft added 5 commits October 23, 2016 15:45

Thumb and track position should minus 'settings.min'

c0a4fc2

Fixed that the auto generated labels number is less than expected

486d8d0

Fixed wrong offset of vertical labels

27c2eb9

Bring back smooth moving

b4fd88c

Support to enable smooth moving using css name 'smooth'

ce39dab

bdaunton approved these changes Nov 2, 2016

View reviewed changes

Remove useless code

73d699d

jlukic merged commit 5ab148c into Semantic-Org:slider Jan 12, 2017

noinkling mentioned this pull request May 18, 2018

[UI] Input Slider / Range Component #1336

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Slider: bugfix and improving implements #4678

Slider: bugfix and improving implements #4678

flytreeleft commented Oct 22, 2016 •

edited by jlukic

Loading

bdaunton commented Oct 22, 2016 •

edited

Loading

bdaunton Oct 22, 2016

bdaunton Oct 22, 2016

bdaunton Oct 22, 2016

flytreeleft commented Oct 23, 2016

bdaunton commented Nov 2, 2016 •

edited

Loading

bdaunton commented Nov 2, 2016

bdaunton left a comment

bdaunton Nov 2, 2016

bdaunton Nov 2, 2016

bdaunton Nov 2, 2016

bdaunton Nov 2, 2016

bdaunton Nov 2, 2016

bdaunton Nov 2, 2016

bdaunton Nov 2, 2016

flytreeleft commented Nov 6, 2016

		@@ -138,6 +138,9 @@ $.fn.slider = function(parameters) {
		slider: function() {
		sliderObserver.observe($module[0], {

Slider: bugfix and improving implements #4678

Slider: bugfix and improving implements #4678

Conversation

flytreeleft commented Oct 22, 2016 • edited by jlukic Loading

bdaunton commented Oct 22, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flytreeleft commented Oct 23, 2016

bdaunton commented Nov 2, 2016 • edited Loading

bdaunton commented Nov 2, 2016

bdaunton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flytreeleft commented Nov 6, 2016

flytreeleft commented Oct 22, 2016 •

edited by jlukic

Loading

bdaunton commented Oct 22, 2016 •

edited

Loading

bdaunton commented Nov 2, 2016 •

edited

Loading