thebuilder / react-intersection-observer

React implementation of the Intersection Observer API to tell you when an element enters or leaves the viewport.
https://react-intersection-observer.vercel.app
MIT License
5.07k stars 185 forks source link

Safari desktop triggers endless loop #308

Closed gabrieltomescu closed 4 years ago

gabrieltomescu commented 4 years ago

Hi!

I'm using inView to detect if a container is in view, then setting this property to a secondary container that should be visible when the first one is out of view.

In Chrome this works fine, but in Safari I'm getting a endless loop of inView being toggled between true and false when my scroll position is at the edge of the intersection point.

navbar

Any help with this?

const [primaryContainerRef, inView] = useInView();

<div className="primary-container" ref={primaryContainerRef}>
       This is the primary container
</div>

<SecondaryContainer isVisible={!inView} />

const SecondaryContainer = ({ isVisible }) => {
   return (
      {isVisible && (
          <div className="secondary-container">This is the secondary container</div>
     )}
   )
};
thebuilder commented 4 years ago

I would try setting rootMargin and/or threshold to modify when inView triggers. The bug you are seeing is most likely related to how safari handles intersection observers natively.

gabrieltomescu commented 4 years ago

Thanks, I tried both of those, but no luck.

thebuilder commented 4 years ago

Can you replicate it in a Code Sandbox?

giannif commented 4 years ago

I'm seeing an infinite loop as well, I tracked it down to the upgrade from 8.25.3 to 8.26.0. I see it on Chrome desktop.

designbyadrian commented 4 years ago

I'm experiencing performance degration on Chrome 80.0.3987.132 with react-intersection-observer 8.26.1

Same with render prop and useInView

designbyadrian commented 4 years ago

@gabrieltomescu I'm currenly using NextJS. I attached the ref to an anchor tag inside a Next/Link component:


import Link from 'next/link';

...

<Link to="/">
<a ref={ref}>Some component</a>
</Link>

Got rid of the issue when applying to a child component to <a>. Will try to mimic in CodeSandbox.


Edit: CodeSandbox example here: https://codesandbox.io/s/nextjs-o0783

Move the ref to the child div to stop the infinite loop.

thebuilder commented 4 years ago

@gabrieltomescu I'm currenly using NextJS. I attached the ref to an anchor tag inside a Next/Link component:

import Link from 'next/link';

...

<Link to="/">
<a ref={ref}>Some component</a>
</Link>

Got rid of the issue when applying to a child component to <a>. Will try to mimic in CodeSandbox.

Edit: CodeSandbox example here: https://codesandbox.io/s/nextjs-o0783

Move the ref to the child div to stop the infinite loop.

The Codesandbox example goes into an infinite loop because of a missing next/router

macrozone commented 4 years ago

I also see a massive performance degeneration with chrome 80.

useInView seems to rerender every frame

thebuilder commented 4 years ago

Is it possible to get a Codesandbox that showcases this issue? I'm not seeing it, so i need to know how you are configuring it to trigger the issue.

Edit react-intersection-observer

macrozone commented 4 years ago

can confirm that 8.26.2 fixes the issue in chrome 80 (and 81)

thebuilder commented 4 years ago

Can you share the code that caused the issue? Would like to know if it's related to switch ref target.

macrozone commented 4 years ago

Can you share the code that caused the issue? Would like to know if it's related to switch ref target.

its nothing special, i did not change the ref target. If it helps: i assigend the ref usually to a component that was created with styled-components

macrozone commented 4 years ago

the error seem to have reappeared in chrome 81. :-(

valerie-roske commented 4 years ago

@thebuilder I'm on version 8.26.2 and I'm experiencing the same issue as @gabrieltomescu in Safari only. The code is set up the same way.

This issue seems to occur for me only when the observing element is sticky to the top (using position:sticky), and overlapping the observed element. (The gif demonstrates this well.) When the observer is sticky at the bottom, this flickering does not occur. The flickering also doesn't occur when using position:fixed.

thebuilder commented 4 years ago

@valerie-roske Can you make a Codesandbox demo of the issue?

valerie-roske commented 4 years ago

@thebuilder here you go: https://codesandbox.io/s/react-intersection-observer-p9ide

So another observation as I was making this demo: the order on the DOM is relevant. In my example, the flickering occurs only when the observer element is inserted above the element it's observing. So for anyone coming across this issue, potential solutions:

thebuilder commented 4 years ago

Thanks for the sandbox @valerie-roske - It seems to be an issue in all browsers, but I think the library is actually acting correctly. When you attach the new element with position sticky it causes a layout shift. This in turn causes the IntersectionObserver to trigger again. In Firefox and Chrome it only seems to have once per scroll event, so it causes flickering while scrolling, with Safari going into an infinite loop.