-
Notifications
You must be signed in to change notification settings - Fork 46.5k
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
Ensure touch events are properly handled for pageX and pageY #15587
Conversation
Details of bundled changes.Comparing: c7398f3...497e937 react-events
Generated by 🚫 dangerJS |
packages/react-events/src/Press.js
Outdated
if (touchEvents.length === 0) { | ||
throw new Error('TouchEvent does not have any changed touches'); | ||
} | ||
// TODO: we need to support multi-touch in the future, this is a temp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does multi-touch related to Press?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we discussed in our meeting, we need to consider the user pressing multiple Press event responders at the same time. I'll address this TODO tomorrow in a follow up.
}); | ||
}); | ||
|
||
describe('press with movement (touch events fallback)', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a little difficult to follow what happened in this part of the diff. Is this duplicating the entire pointer events test block? If so, I don't think we need to do that (and we don't for mouse events)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't duplicate it all, a lot of it, but I changed a few tests too and remove a few there were mouse related.
Whilst working with an iOS device, I noticed that we were incorrectly trying to access the co-ords for
pageX
andpageY
on thenativeEvent
object. They don't exist in this place, rather this change makes it so we access them on thechangedTouches
array (if it exists). We only take them from the first event, we need to add support multiple touches in the future.Ref #15257