Add range slider component #154
Conversation
| * Increment value for range input changes | ||
| * | ||
| * @property {step} | ||
| * @type {Number?} |
There was a problem hiding this comment.
I actually did that deliberately to try to indicate that step can also be null... happy to remove if you prefer?
| rangeInputWrapperStyle: computed('min', 'max', 'value', 'sliderProgress', function() { | ||
| let { min, max, value: current, sliderProgress } = this.getProperties('min', 'max', 'value', 'sliderProgress'); | ||
| let styleProps = assign({ min, max, current }, { | ||
| current, |
There was a problem hiding this comment.
is this current needed here, since it's also in the object in which it's merged?
There was a problem hiding this comment.
Eagle-eyes @vladucu 😅 Thanks, this was left over from a previous iteration 👍
| actions: { | ||
| handleChange(event) { | ||
| let onChange = this.get('onChange'); | ||
| if (isNone(onChange)) { |
There was a problem hiding this comment.
since we make this a no-op for us, we can probably remove this?
There was a problem hiding this comment.
Can do, was just following the React implementation which has onChange as a required argument but still makes this check 🤷♂️
There was a problem hiding this comment.
yeah, fine with me to leave it too....nothing bad here
| step={{step}} | ||
| value={{value}} | ||
| disabled={{disabled}} | ||
| oninput={{action "handleChange"}} |
There was a problem hiding this comment.
onchange only fired once at the end of dragging the slider handle, which meant that the fill on the slider, output value in the tooltip etc. didn't update during dragging. oninput seemed to fix that.
…r-polaris into add-range-slider-component
Adds a full implementation of the Polaris range slider component: