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

Actually prevent passive touchmove events default behavior #88

Merged

Conversation

fastory-automation
Copy link

It’s possible to listen to touchmove manually to ensure the listener has the ability to prevent the default event behavior. Otherwise it’s impossible to let users scroll vertically while playing a video with audio enabled on horizontal swipe in Chrome 56+.

@hartzis
Copy link
Collaborator

hartzis commented Oct 16, 2017

@kontest @kl0tl Thanks for the PR, i've been interested in a solution to this for a while. I like where you're going with this and the simple detect-passive-events package.

I'd like to see if we could add tests and possibly move the newProps.ref function into the constructor so it doesn't get rebound on every render.

I'm hoping to take a deeper look at this and compare this solution to react-easy-swipe's solution too 😄

@hartzis hartzis self-assigned this Oct 16, 2017
@kl0tl
Copy link
Contributor

kl0tl commented Oct 23, 2017

@hartzis I’ve moved the definition of the ref handler out of render! I’m not really sure about what to test. Do you think testing an active touchmove listener is manually added on window if passive events are supported and preventDefaultTouchmoveEvent is enabled would be enough ?

Copy link
Collaborator

@hartzis hartzis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You rock @kl0tl ! I'll try to get this merged and published this week. Should be just a minor bump. So 4.2.0.

If you feel exceedingly ambitious, could mimic the tests here ish, if not, i may attempt after the merge.

Cheers!

@hartzis
Copy link
Collaborator

hartzis commented Oct 27, 2017

pulled down the branch, did some testing and some research. Moved the listener setup inside react lifecycle methods to avoid the ref being null.

@hartzis hartzis merged commit 383d372 into FormidableLabs:master Oct 28, 2017
philandstuff added a commit to philandstuff/wind.cards that referenced this pull request Jun 13, 2018
@github-actions github-actions bot mentioned this pull request May 18, 2023
@paulmarsicloud paulmarsicloud mentioned this pull request Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants