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

[zoom] make wheel event active by default. fixes #455 #456

Merged
merged 1 commit into from
May 9, 2019

Conversation

hshoff
Copy link
Member

@hshoff hshoff commented May 3, 2019

The Problem

Chrome 73 made wheel event listeners passive by default. A passive event listener is an event listener that does not call preventDefault() on the event. This is great because it makes scrolling faster. https://developers.google.com/web/updates/2019/02/scrolling-intervention.

Google found 98% of wheel and mousewheel event listeners do not call preventDefault(). So the trade off here of faster scrolling on the web for the relatively small cost of 2% of listeners need to opt-in to making the wheel listener active. Unfortunately, <Zoom /> falls in the 2% here.

The recommended fix

Convert event listeners that rely on preventDefault() to active.

- node.addEventListener('wheel', this.handleWheel);
+ node.addEventListener('wheel', this.handleWheel, { passive: false });

But what about React?

Folks in react-land probably aren't used to writing addEventListener() these days. The more usual pattern is to add onWheel={this.handleWheel} prop. So how do we make an event listener active in React? My observed behavior of React is that it registers event listeners without a default value for passive (I could be wrong about this, I was running in circles in the React codebase). Ideally React would support the ability to set the passive value. Until then, we'll have to fall back to rolling our own addEventListener + removeEventListener.

The solution

Keeping the default behavior

The default behavior for <Zoom /> will be to add an active event listener in componentDidMount and remove it componentWillUnmount() and call preventDefault() on wheel events. This matches the previous default behavior, but requires the removal of a prop to get rid of the console warning from Chrome.

The breaking change

If you update to this version of @vx/zoom, you'll just have to remove onWheel={zoom.handleWheel} and the warning will go away and everything should work as before. The diff is small:

<MyComponent
- onWheel={zoom.handleWheel}
/>

Opt-in to passive on wheel event listener

The upside here is you now have a way to opt-in to passive on wheel event listener. This will not call preventDefault()

<Zoom
+ passive={true}
>
  {zoom => {
    return (
      <MyComponent
+      onWheel={zoom.handleWheel}
      /> 
    );
  }}
</Zoom>

Further reading

Changelog

💥 Breaking Changes

  • [zoom] make wheel event active by default. fixes Chrome 73 scroll intervention warning.
    • To keep the default behavior before Chrome 73 and remove console warnings in Chrome 73, remove:
      <MyComponent
      - onWheel={zoom.handleWheel}
      />
    • To make the onWheel events passive, add:
      <Zoom
      + passive={true}
      >
        {zoom => {
          return (
            <MyComponent
      +      onWheel={zoom.handleWheel}
            /> 
          );
        }}
      </Zoom>

📝 Documentation

  • [zoom] update docs

Thanks to @j1mie for bringing this to my attention.

@hshoff hshoff added this to the v0.0.189 milestone May 3, 2019
@hshoff hshoff merged commit 903b3ab into master May 9, 2019
@hshoff hshoff deleted the harry-fix-zoom-wheel branch May 9, 2019 15:04
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.

1 participant