alvarotrigo / react-fullpage

Official React.js wrapper for fullPage.js https://alvarotrigo.com/react-fullpage/
GNU General Public License v3.0
1.29k stars 178 forks source link

Crash on add/remove slide #435

Open BenjaminSYD opened 8 months ago

BenjaminSYD commented 8 months ago

Description

Hello,

I have encountered a bug while using the ReactFullPage with slides. Specifically:

  1. When attempting to add a slide to a section, the slide fails to render, and the FullPage arrow buttons behave erratically.
  2. Additionally, when attempting to remove a slide from a section, React crashes with the error message "Node.removeChild: The node to be removed is not a child of this node".

Link to Isolated Reproduction with No External CSS/JS

Link to Codesandbox for Reproduction

Steps to Reproduce Crash on Removing a Slide

  1. Open the Codesandbox link provided.
  2. Click the "remove slide" button.
  3. Observe the application crash.

Steps to Reproduce Bug on Adding a Slide

  1. Reload the Codesandbox link.
  2. Click the "add slide" button.
  3. Notice that the "Slide3" slide fails to render, and the arrow buttons exhibit unexpected behavior.

Note: If you add a slide and then remove it, the crash does not occur, and the issue with the arrow buttons resolves itself.

Workaround

I noticed that this rebuilding log is not called when adding a slide so I guess isReRenderNecessary() is returning false. I managed to fix it by sending the following prop sectionsColor to ReactFullpage to force a re-render:

const sectionsColors = useMemo(() => {
  // Second color is never used because I only have 1 section but it triggers a re-render.
  return ["#6495ed", "#00000" + (slides.length % 9)];
}, [slides.length]);

And the crash when a slide is removed can be fixed with a call to the destroy() method of ReactFullPage ref before removing the slide.

const fullPageRef = useRef<any>(null);

const handleRemoveSlide = useCallback(() => {
  fullPageRef.current.destroy();
  setSlides((prevSlides) => prevSlides.slice(0, -1));
}, [setSlides]);

<ReactFullpage
    ref={fullPageRef}
    // ...
/>

Versions

Versions can be found within the Codesandbox link provided.

alvarotrigo commented 8 months ago

Good catch!

I've fixed it on dev!

BenjaminSYD commented 8 months ago

Thanks for the quick response!

The bug is resolved when I add a slide. However, the crash persists when I attempt to remove a slide. And my workaround is bad since it bring me back to slide 1.

alvarotrigo commented 8 months ago

However, the crash persists when I attempt to remove a slide. And my workaround is bad since it bring me back to slide 1.

It seems the function componentWillUnmount gets triggered when removing a Slide. Which shouldn't be the case. But I'm not quite sure what's the issue here.

alvarotrigo commented 2 months ago

The adding part was fixed in version 0.1.43 🥳 I'll have to take a second look at the remove part.