asyarb / use-intersection-observer

Small React hook wrapper around the IntersectionObserver API.
MIT License
6 stars 3 forks source link

Hook callback now receives the entire entries array #7

Closed asyarb closed 4 years ago

asyarb commented 4 years ago

Changes the user-land hook callback to now receive the entire entries array as an argument instead. This allows the hook to support multiple thresholds and ultimately be more flexible since we now provide all the intersection data to the user.

asyarb commented 4 years ago

While working on this though, separating into the two different cases for triggerOnce still doesn't seem to resolve cases where really fast scrolling causes callbacks to not be called.

Even with handleIntersect being:

const handleIntersect = () => {
  console.log('intersect')
}

I can still reliably reproduce a loss of about half of the callbacks that IntersectionObserver should be firing.

fabb commented 4 years ago

While working on this though, separating into the two different cases for triggerOnce still doesn't seem to resolve cases where really fast scrolling causes callbacks to not be called.

Do you mean for triggerOnce being true or false? In the first case, did adding tge return now resolved this issue?

asyarb commented 4 years ago

For both true and false. Having the return didn't resolve it either. Even completely stubbing the implementation of the IntersectionObserver callback doesn't seem to resolve it either.

The example/ directory currently renders 100 observed items, and I can reliably trigger missed callbacks with a callback that is just:

const handleIntersect = console.count

With really fast scrolling, I can get about ~50% of callbacks to not fire 😬.

asyarb commented 4 years ago

That being said, I think that's a larger issue outside of the scope of just this PR (providing all entries to the callback), so I can iterate/research more about a solution for that later and get this merged and released.

fabb commented 4 years ago

With really fast scrolling, I can get about ~50% of callbacks to not fire 😬.

How does your example look? Is it a list with rows where you scroll over and expect all rows to be firing the callback, even when the list just quickly scrolls over them?

In that case I‘d say it is expected that the callback does not fire for every element in the list, as when scrolling fast, elements might be below the scroller viewport in one render frame, and immediately above the scroller viewport in the next render frame. In that case the element just goes from non-intersecting to non-intersecting. I have the same issue here: https://github.com/GoogleChrome/samples/pull/678#issuecomment-599027933