facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
229.1k stars 46.87k forks source link

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

Open YuriGor opened 2 years ago

YuriGor commented 2 years ago

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.

bvaughn commented 2 years ago

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 commented 2 years ago

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,

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 commented 2 years ago

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 commented 2 years ago

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 :cry: @gaearon what do you think?

gaearon commented 2 years ago

What log spam are you seeing?

gaearon commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

No problem!

evgenyneu commented 1 year ago

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).

mi-na-bot commented 1 year ago

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 commented 1 year ago

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.

mi-na-bot commented 1 year ago

@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 commented 11 months ago

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 commented 6 months ago

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