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

onScroll event propagation #175

Closed
O4epegb opened this issue Jul 20, 2020 · 5 comments
Closed

onScroll event propagation #175

O4epegb opened this issue Jul 20, 2020 · 5 comments

Comments

@O4epegb
Copy link

O4epegb commented Jul 20, 2020

So, since React bubbles all its events, even the ones that the browser doesn't bubble by default, there is some weird behaviour with onScroll event.

Example: https://codesandbox.io/s/httpsgithubcomreactjsrfcsissues175-viizc

onScroll callback on Parent element is fired when Children element is scrolled.

Why this is weird and might lead to unexpected bugs? Imagine following situation:

  1. You have some scrollable component A where you are doing something when onScroll fires
  2. Then somebody introduces another scrollable component B as a children of your component
  3. When B is scrolled your onScroll handler on component A is also fired.
  4. To mitigate this you need to add some checks like this one: e.target === e.currentTarget

Basically it means that you need to add this check every time you add onScroll to any element because you can not be sure that nothing scrollable will be introduced down the tree later.

I think there are more events like this, for example onSubmit, but at least you can't put <form> inside another <form>.

It might be worth to reconsider event bubbling in the future.

Issue discussion facebook/react#15723

@gaearon
Copy link
Member

gaearon commented Jul 20, 2020

If you'd like to file an RFC, please send a pull request.

@necolas
Copy link

necolas commented Jul 20, 2020

@O4epegb Yes this is indeed weird behavior. React also bubbles blur and focus, which requires the same mitigation strategy. These are old decisions in the early versions of ReactDOM, which makes it difficult to change the behavior of those event props going forward (as there is no straight-forward way to deprecate the existing behavior for a period of time). It's also difficult to automatically analyze code for dependencies on the non-standard behavior, and most apps will not have automated test coverage to catch UX/a11y regressions if the behavior changes.

Something a few of us (not the core team) have been working on is a new ReactDOM event API that's independent of the existing event props. This allows us to have a fresh start and undo some of these decisions. In the meantime, I think your mitigation strategy (or calling stopPropagation) is a suitable work-around.

@gaearon
Copy link
Member

gaearon commented Jul 20, 2020

There's also media events that all have the same problem.

@theKashey
Copy link

I would not call it a problem.
You are getting an ability to observe more events, having an ability to understand should you react or not to those events as well.
The only real problem with even bubbling are portals, where user ability to understand a location of the event (one could traverse from target to currentTarget) is broken - facebook/react#14540

@O4epegb
Copy link
Author

O4epegb commented Aug 9, 2020

It seems like that behaviour gonna be fixed in v 17.
facebook/react#19464
Should we close this issue now?

@gaearon gaearon closed this as completed Aug 10, 2020
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

No branches or pull requests

4 participants