Skip to content

Adds support for pointer events to useDrag1D #1069

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

Closed
wants to merge 1 commit into from

Conversation

itaylor
Copy link
Member

@itaylor itaylor commented Sep 13, 2020

Closes #1063

This adds support with feature detection for PointerEvents to the existing useDrag1D, without altering its declared API.
In reality, several places were making use of items passed back in the output of useDrag1D in a way that was probably not intended in the API, by limiting the returned properties in order to do things like event chaining. I fixed all the places that were doing this to be compatible with my changes, and added tests that cover all existing uses of useDrag1D to use pointerEvents as well as mouseEvents. I also added the touch-events: none CSS declaration to draggable elements so that they can be dragged using PointerEvents without triggering things like scrolling.

Worth noting:
JSDOM doesn't support PointerEvents currently, as it is missing the event type for PointerEvent on the window object. I added my own (rather poor) polyfill in order to support testing of PointerEvents via JSDOM/jest. It should be removed if/when JSDOM adds support for PointerEvents (see: jsdom/jsdom#2666).

Also, I had to work around a testing-library bug with PointerEvents as well:
The workaround is pretty harmless (see line 18 in SplitView.test.js. That workaround can be removed when testing-library/react-testing-library#783 is fixed and we've upgraded to that version of testing-library.

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Manual testing steps:
Modern desktop browser:
Visit the storybook for Slider and Split view.
The Slider and Split view should work the same as they did before this change, you are able to drag the draggable elements.

Modern mobile browser:
Visit the storybook for Slider and Split view. It should be possible to drag the draggable elements, before this change it was not.

Legacy browser that doesn't support PointerEvents (Safari 12 or IE 10):
Visit the storybook for Slider and Split view. It should work as it did before this change, as it still uses mouse events.

🧢 Your Project:

Not for any particular project, just because I was interested.

@mischnic
Copy link
Contributor

Thank you for working on this.

We are currently looking into expanding/replacing the useDrag1D hook into useMove ("2D") and don't know yet how many of your changes can be reused. We will get back to you once we do.

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.

useDrag1D should use pointer events instead of (or in addition to) mouse events
2 participants