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

React 18 not passive wheel / touch event listeners support #22794

Open
YuriGor opened this issue Nov 19, 2021 · 14 comments
Open

React 18 not passive wheel / touch event listeners support #22794

YuriGor opened this issue Nov 19, 2021 · 14 comments
Labels
React 18 Bug reports, questions, and general feedback about React 18 Type: Discussion

Comments

@YuriGor
Copy link

YuriGor commented Nov 19, 2021

Hi all, is there any chances React 18 will support not passive wheel / touch event listeners?
In React 17 they are passive so no way to preventDefault and I had to add active listeners manually by ref.
So maybe in v18 there will be some option to make them not passive in react?

I just tested my code with React 18 beta and found some internal order of useEffect calls was changed, so my approach is failing because of desync of changes made in active listener vs other props changes.

@YuriGor YuriGor added React 18 Bug reports, questions, and general feedback about React 18 Type: Discussion labels Nov 19, 2021
@bvaughn
Copy link
Contributor

bvaughn commented Nov 22, 2021

The decision to change touch/wheel events to passive is explained with a bit of background in #19651 and #19654. I'm not aware of any plans to change this for 18.

so my approach is failing because of desync of changes made in active listener vs other props changes.

Can you clarify what this means?

@YuriGor
Copy link
Author

YuriGor commented Nov 22, 2021

I have zoom/pan board element in my app.
Zoom I implemented in mouse wheel event listener using css transform/scale and pan I implemented utilizing native browser scrollTop \ scrollLeft assigned via ref.
So when user scrolls mouse wheel I prevent default to block native scrolling and do scaling instead.
But to make scale properly I need also to adjust scrolling offset synchronously, to keep screen centered on mouse pointer.
So in react v17 scrollTop \ scrollLeft assigned in native mousewheel event listener was in same timeframe as rendering canvas component with new scale value and all worked fine.

In v18 render I have two renders instead one,

  • first scrolling is applied so the canvas jerks with same scale
  • second scale prop change comes to render method and canvas jerks back to correct position because now scroll and scale are matching as expected.

here is a piece of my active native mousewheel event listener:

//...
          e.preventDefault();
          let newScale =
            scale + Math.sign(e.deltaY || e.deltaX || e.deltaZ) * -diagramDefaults.wheelStep;
          if (newScale < minScale) newScale = minScale;
          else if (newScale > diagramDefaults.maxScale) newScale = diagramDefaults.maxScale;
          // below scroll changes happen immediately
          containerRef.current.scrollTop =
            ((containerRef.current.scrollTop + e.clientY) * newScale) / scale - e.clientY;

          containerRef.current.scrollLeft =
            ((containerRef.current.scrollLeft + e.clientX) * newScale) / scale - e.clientX;

          // ref.current.style.transform = `scale(${newScale})`; // here I tried to dirty fix this issue but with low effect
          // triggered scale prop update comes to render with delay in v18, while in v17 it was in same js event loop cycle.
          onScale(newScale);
//...

@bvaughn
Copy link
Contributor

bvaughn commented Nov 22, 2021

Gotcha. Thanks for explaining your use case.

React DevTools (written with React DOM v18) has some "wheel" events code but it is just added in an event so that it can preventDefault.

I'm not really up to date with the latest thinking around this event decision. @gaearon may know more (or it may be that his most recent update– linked above– still reflects our current thinking).

@YuriGor
Copy link
Author

YuriGor commented Nov 22, 2021

Yes, I remember that discussion, I implemented my current solution inspired by comments there.
It looks bad and will not work in the future.
Having something like onWheelActive in v18 would be very helpful.
or maybe like this:

function handleWheel(e){ /*...*/ }
handleWheel.active = true;
//..
<div onWheel={handleWheel}`/>

..so react will just check active prop on handler before making listener passive or not.
We could consistently support this for all events, so setting it to boolean false (to distinguish from undefined) will make passive listeners even if they are active by default currently.

Browser gives this choice, why react takes it away from me, it does not help at all 😢
@gaearon what do you think?

@gaearon
Copy link
Collaborator

gaearon commented Apr 8, 2022

What log spam are you seeing?

@gaearon
Copy link
Collaborator

gaearon commented Apr 8, 2022

I think there is a misunderstanding.

This issue is about support for marking events as not passive because they are passive by default.

The warning you're showing is the exact opposite. Can you please show which callsite the warning highlights? I have a suspicion that it's either not React, or it's some older version.

@cha0s
Copy link

cha0s commented Apr 8, 2022

You're right, it's not React. It's in fact Kefir (inside useEffect).

Really sorry for wasting your time! Thanks for your work. :)

@gaearon
Copy link
Collaborator

gaearon commented Apr 8, 2022

No problem!

@evgenyneu
Copy link

evgenyneu commented Dec 29, 2022

Same issue, I need to call preventDefault() inside onWheel, onTouchMove and onTouchStart event handlers to prevent the page from scrolling when user interacts with my component. This does not work because these even handler are passive. Is there an API in React to add a non-passive event handler? (currently I have to attach event handlers manually with myElement.addEventListener inside useEffect).

@minervabot
Copy link

I am working on a carousel component that sometimes needs to prevent vertical scrolling of the window, and this design decision is apparently going to force me to add the listener manually as well.

This is fine, but I do wonder how the React team thinks nobody would ever want to use preventDefault on a pointer event.

@callumlocke
Copy link

Any workaround for this? If I want to handle mousewheels in a special way within a component (e.g. in a game or UI with special requirements), then I need to be able to do event.preventDefault() or it scrolls the page at the same time.

Would a safe workaround be to use useRef to get a ref for the element, and attach my listener via ref.current.addEventListener within a useEffect callback? That seems to work but I'm concerned about the possibilty that React may replace the div dynamically at some point and my effect won't be re-run so the event listener won't be attached to the new div.

@minervabot
Copy link

@callumlocke You probably want to use a ref callback on the JSX element instead passing an actual ref. https://react.dev/reference/react-dom/components/common#ref-callback

Also, make sure to clean up the listeners when they are no longer needed. In newer versions of React I think you can return a cleanup function like useEffect, but in older versions, you might need to do something else.

@vincerubinetti
Copy link

stopPropagation is also not allowed on passive event listeners, it seems. So we have to manually create a ref, attach listeners, etc. any time we want to prevent default or stop propagation. Nice.

@pham-tuan-binh
Copy link

Same problem with me, need to have to use an active Wheel event in a div component. Only way I've found right now is using react-event-injector or using refs to attach event manually

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
React 18 Bug reports, questions, and general feedback about React 18 Type: Discussion
Projects
None yet
Development

No branches or pull requests

9 participants