Slider: Add touch event support for better mobile interaction #2362
Conversation
|
This is a change that we'd really like to incorporate into the WordPress admin. If there's anything you need to help move it forward, happy to help. |
|
Thanks for the PR. I'll look into it once we get jQuery 4.0.0-rc.1 out as that's our main focus at the moment. |
|
In the meantime, can you fix the linting failures that you now should see reported? |
|
@mgol Thanks! I've resolved the reported linting errors. Let me know if there's anything else you'd like me to adjust in the meantime. |
There was a problem hiding this comment.
I played with the PR a little. I'm a little torn where to go from there, so I'll present my thinking.
On one hand, this seems to work fine (though I'd have to spend a bit more time testing) and in so little code it's hard to believe.
On the other hand, jQuery UI has abstracted mouse interactions between many widgets into a separate https://github.com/jquery/jquery-ui/blob/1.14.1/ui/widgets/mouse.js file used by the slider, draggable, resizable, selectable & sortable widgets. All of them could use better touch support as well.
I've been thinking that it'd be good to make use of Pointer Events here and create a pointer.js version of the mouse.js file that just does everything in pointer events, with similar extension methods, without handling mouse & touch events separately.
For backwards compatibility with all the plugins out there who may expect touch events to not be handled, we'd probably have to keep mouse.js as an optional module, but we could switch our widgets to the new pointer.js one. It'd be good to preserve as much of the interface as possible unless it totally doesn't make sense in pointer events. This could then be shipped as part of 1.15.0.
The mouse.js module is bigger than this patch but it's still not huge. I think I'd prefer to at least try using Pointer Events for this and avoid forking the events - also because it will be easier to maintain that way, especially given our limited mobile test coverage.
We only support modern browsers in the latest versions of jQuery UI, so we don't need any fallbacks or polyfills for Pointer Events, we should just be able to use them.
What do you think? Would you like to attempt such an implementation?
P.S. If it turns out that this migration has some issues that would have to be ironed out for non-slider components currently using mouse.js, we could just migrate slider.js to use pointer.js initially and leave the other migrations for separate PRs or future versions, depending on the effort required.
@fnagel I'd also love your thoughts here.
|
BTW, the test failures are a result of a config incompatibility between the older Of course, some unit tests could be useful here as well, but I see even our mouse coverage is not huge here due to difficulties testing such interactions in unit tests, so I didn't focus on that part; we'd manage to think about something, that's not the biggest issue to address. |
…er-mobile-interaction
Bumps the github-actions group with 1 update: [github/codeql-action](https://github.com/github/codeql-action). Closes jquerygh-2357 Updates `github/codeql-action` from 3.28.18 to 3.29.2 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@ff0a06e...181d5ee) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 3.29.2 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-actions ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
When a stable 4.0.0 gets released, we'll remove the RC in favor of it. Closes jquerygh-2363
Changes: * Checkboxradio: Change `.attr( "checked", true )` to `.attr( "checked", "checked" ) * Selectmenu: Disable the `boolean-attributes` patch for one assertion where it's impossible to avoid Closes jquerygh-2364
There was a typo in the test command for Edge on Windows - a double `--`. This worked with npm included with Node.js 22.17.1, but started failing with 22.18.0. Closes jquerygh-2367
…ion' of https://github.com/nikunj8866/jquery-ui into 2361-add-touch-event-support-for-better-mobile-interaction
|
@mgol I agree that re-using (as in copy and modify) the The PR itself looks straight forward, but I have no time to test really test it thoroughly. Some, even basic unit test, would be very nice though. FYI: there was once a plan to use PEP (Pointer Events Polyfill) everywhere, see the (very) old roadmap: https://jqueryui.pbworks.com/w/page/12138038/Roadmap Idea was to rewrite the |
|
There also is this project that translates touch events to mouse: https://github.com/RWAP/jquery-ui-touch-punch/tree/master |
Issues: #2361