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

Crash on add/remove slide #435

Open
BenjaminSYD opened this issue Mar 26, 2024 · 4 comments
Open

Crash on add/remove slide #435

BenjaminSYD opened this issue Mar 26, 2024 · 4 comments
Labels
bug Something isn't working fixed on dev

Comments

@BenjaminSYD
Copy link

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 alvarotrigo added the bug Something isn't working label Mar 26, 2024
@alvarotrigo
Copy link
Owner

Good catch!

I've fixed it on dev!

@BenjaminSYD
Copy link
Author

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
Copy link
Owner

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed on dev
Projects
None yet
Development

No branches or pull requests

2 participants