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

e.preventDefault() inside passive event listener in mobile devices #581

Closed
fsubal opened this issue Jan 28, 2019 · 4 comments
Closed

e.preventDefault() inside passive event listener in mobile devices #581

fsubal opened this issue Jan 28, 2019 · 4 comments

Comments

@fsubal
Copy link

fsubal commented Jan 28, 2019

With "react-color": "^2.17.0"

In Chrome 71.0.3578.98, with devtools responsive mode, the following warning is emitted.

2019-01-28 18 01 56

It look like React implicitly addEventListeners touchmove event as { passive: true }.

Since you should not fire e.preventDefault() in touchmove handler, which Saturation component is violating it.

handleChange = (e) => {
typeof this.props.onChange === 'function' && this.throttle(
this.props.onChange,
saturation.calculateChange(e, this.props.hsl, this.container),
e,
)
}

Fundamentally it is caused by saturation.calculateChange(), so it seems this could be solved by

  1. making .calculateChange not requiring Event object
  2. not using Saturation.this._handleChange for onTouchmove, replacing to another event handler
@fsubal
Copy link
Author

fsubal commented Jan 28, 2019

Also reproduced in https://casesandberg.github.io/react-color/

2019-01-28 18 14 52

@casesandberg
Copy link
Owner

Hmm, this is an interesting case. I am going to look back through to see why preventDefault was added in the first place. We might be able to just remove it.

@glauger-petal
Copy link

Looks like this is a known issue with Chrome and React:
facebook/react#8968

@casesandberg
Copy link
Owner

I am going to go ahead and close this as it has to do with a specific version of React and not this package itself.

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

Successfully merging a pull request may close this issue.

3 participants