leongersen / noUiSlider

noUiSlider is a lightweight, ARIA-accessible JavaScript range slider with multi-touch and keyboard support. It is fully GPU animated: no reflows, so it is fast; even on older devices. It also fits wonderfully in responsive designs and has no dependencies.
https://refreshless.com/nouislider/
MIT License
5.66k stars 659 forks source link

Create `drag` event for when a connect is being dragged #1135

Closed IRHM closed 3 years ago

IRHM commented 3 years ago

There is no event for when a user drags a connect between two starts, this PR creates one named drag.

It adds these lines:

// If target is a connect, then fire drag event
if (data.target != undefined && hasClass(data.target, options.cssClasses.connect)) {
    fireEvent("drag", data.handleNumbers[0]);
}

to the eventMove() function. I'm not very familiar with this code base, so I have no idea if this is a good spot to put this (could be a little less performant, since it is checking if the target is a connect on all move events. Don't know how big of an impact this would be though).

Currently it just returns the first handle number (data.handleNumbers[0]) and to access both values of the starts connected together you'd have to do something like this:

slider.noUiSlider.on('drag', function (values, handle) {
    // (handle) for 1st value
    // (handle +1) for second value
    console.log(values[handle], values[handle + 1]);
});

I have added this event to the documentation. The demo slider now features a connection between the two handles that were already there and the behaviour is set to 'drag-tap', so that people can test the drag event. I have also modified the padding a little on the .example and .view-more classes so that the events fire order list can be on one line for desktop users.

Closes #887

IRHM commented 3 years ago

@leongersen What do you think about where the drag event is being fired? I feel like there is a much better place it could go that I am unaware of.

leongersen commented 3 years ago

Thank you for this PR. I like these changes, and I think this additional event makes sense. Regarding where it is fired: I'd move it into the moveHandles function that is called from the eventMove function where it is placed now. This would allow for a check on state there, so the event only fires if anything actually changed.

Regarding the check on whether the element is a connect, I'd like to see this added as a property to EventData and passed from the start here.

The documentation should also mention which handleNumber is given as the argument to the drag event.

IRHM commented 3 years ago

@leongersen I'll work on that, thanks for the feedback!

IRHM commented 3 years ago

@leongersen Does that all look good now?

leongersen commented 3 years ago

Yes, looks great! I've just merged this and release noUiSlider 15.1.0. Thanks for contributing!