Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
May 9, 2021
Merged

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

merged 7 commits into from
May 9, 2021

Conversation

IRHM
Copy link
Contributor

@IRHM IRHM commented May 7, 2021

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 added 3 commits May 7, 2021 16:21
- reduce/increase padding on .view-more and .example so it's more even and so the `events order` section can be on one line on desktop views.

- Modify demo slider to include a connection between the two handles.
@IRHM
Copy link
Contributor Author

IRHM commented May 8, 2021

@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
Copy link
Owner

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
Copy link
Contributor Author

IRHM commented May 8, 2021

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

IRHM added 3 commits May 8, 2021 12:11
Also add `connect` to EventData
Now it will always return the first handle in the connection, which is important for people who want to access other handles in their drag event callback.
Created drag.js for an example of how to access handles if the event callback.
@IRHM
Copy link
Contributor Author

IRHM commented May 9, 2021

@leongersen Does that all look good now?

@leongersen leongersen merged commit 172e1ef into leongersen:master May 9, 2021
@IRHM IRHM deleted the drag-event-for-connected-starts branch May 9, 2021 08:05
@leongersen
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

drag event
2 participants