onyx.Slider sliding past limits

Just playing around with 2.5.1 and noticed that the onyx.Slider allows for sliding past its limits in both directions when it is bound to an input. Is there some way to stop this behavior as it was not present in 2.4?

http://jsfiddle.net/6swc7qf4/9/

Comments

  • I'm also curious about this. I played around with it a little yesterday and saw that the sampler doesn't exhibit this behavior, but it is using onChange and onChanging in conjunction with getValue / setValue. I couldn't figure out a way to bind to those builtin methods for the slider.
    Should we be able to bind directly to the slider's .value as in jmich's fiddle?
    That would certainly reduce the code enormously.
  • I have no idea if this is an 'enyo way' to do it, but I came up with a workaround for oneway binding by adding a public bindableValue and updating that with the onChange, onChanging handlers so that the built-in clamping is performed and then this 'sanitized' value is bound to the input:
    jsfiddle.net/buckbito/6swc7qf4/13/
    I'd love to hear if there is a better way to do this, since I'm just starting to wrap my head around it.
  • I think a fix may be called for in onyx.Slider.js .
    Problem with onyx.Slider: Value was not being clamped during drag.
    This allowed values outside specified range, including negative values.
    Perhaps this 'fix' should be inplemented in onyx.Slider.js
    TODO: Tap on bar shows values that don't respect set increment during animation.

    Here's my fiddle with a bindableSlider that overrides drag:
    jsfiddle.net/buckbito/6swc7qf4/16/
  • I've started to look at moonstone's slider and it looks like a few bits should be borrowed from there for the onyx.Slider. moon.Slider calls clampValue in it's drag handler as well as a number of other places where it is missing from onyx.Slider, it also has an override of the default getValue that checks if the slider is animating and returns the target value rather than the current value if that is the case which would address the "TODO" mentioned above...
  • bbito, thanks for your input and findings. I unfortunately have had little time in the last week or so to dig into this but I will hopefully be able to soon.
  • I hope someone with deeper understanding of the framework will comment on the possibility of bringing moon Slider refinements into onyx Slider, because I think that may be the best course. I think at this point, a safe route would be to use a "bindableSlider" with the drag override as in the last fiddle above. Then if enyo Slider is updated, it should be easy to swap in the new vanilla Sliders for bindableSliders.
  • Pull requests are always appreciated!
  • err, @RoySutton I had a feeling that comment was coming...
    It may take a while, I've never made a contribution before, but I did setup a github account last week! I found the Style Guide, but is there a pull request guide for complete newbs?
  • Try this page: http://enyojs.com/community/contribute/

    Looking forward to seeing it!
  • Thanks for the report @jmich and investigation @bbito! I've submitted a PR to get this fixed. Should land in the next couple days.

    https://github.com/enyojs/onyx/pull/216
  • @theryanjduffy - that's awesome! I actually got mired in the onyx.Slider and started trying to merge moon.Slider into it to get what I assume to be a more current code style - e.g. (inValue) instead of (value) arguments and also wanted to bring in the bgProgress feature.
  • Actually, the inValue is the old style. value is the new style.
  • @RoySutton - Great to know, I was assuming moon would have the newer style - doh!
  • We did a huge code cleanup when we did the documentation updates. Not all of Moonstone was updated to match the new style, though.
  • If you've put in work on new features (e.g. bgProgress), please open a PR and we'll take a look!
  • @theryanjduffy - I just started looking at bgProgress last weekend and realized it's a big one (for me), iirc, touching on ProgressBar.js, and one or two .less files as well. I hope I can get something working soon, but I'm just a weekend coder.
  • No worries. Thanks for your willingness to get involved and contribute!
  • edited March 2015
    Hey @RoySutton or @theryanjduffy two related questions:
    1) Is the Changed(was, is) the current style - vs Changed()?
    -EDIT: I ask because it is used in moon.Slider even when the arguments are not used in the private method e.g.: minChanged: function (was, is) and maxChanged: function (was, is).
    2) I can't figure out how to get @theryanjduffy's current Slider.js out of git - is there an easy way in the git bash?
  • Hi @bbito, I happened to be driving by so I'll take a stab at answering these questions in the meantime.

    1. These two styles are not mutually-exclusive. Each Changed method is passed two parameters: the previous value (was), and the current value (is) of the property. So if you need to use one or both of these values, just specify the parameters so you have access to them in your Changed function; if you don't need to use them, you don't necessarily need to explicitly declare these parameters in the function definition.
    2. For @theryanjduffy's Slider.js, did you try going to your Onyx repo and doing a
    git checkout ENYO-1141-ryanjduffy
    to checkout the branch with his changes?
  • Thanks @aarontam!
    If it isn't a performance or maintenance hit to declare the arguments, I like that better - I think it helps in making the source more understandable especially if one is looking to override one of these methods even if the stock method isn't using the arguments.
    Also thanks for the git line - I think thats exactly what I was looking for!
  • A tiny note on change methods and arguments vs. no arguments. Somtimes a situation arises where a Changed() method needs to be called at instance creation time. In that scenario there is no 'was' value, but an 'is' value might be available.

    If your Changed() feels like it could fit this case, it's often better to only declare an optional 'was' argument - and only if there are use-cases where the 'was' value is important - and resolve the 'is' value with a getter rather than an argument.
Sign In or Register to comment.