react-image-lightbox icon indicating copy to clipboard operation
react-image-lightbox copied to clipboard

fix: conditional rendering causes issues in React 16.3+

Open swquinn opened this issue 2 years ago • 2 comments

Removes conditional rendering in favor of passing down the isOpen property to the React <Modal> component.

In more recent versions of React, conditional rendering through the use of portals creates an issue that appears (from my anecdotal experience) related to #589 ("Loading icon gets stuck..."). Tracking this down in the React Modal issue tracker, led me to: https://github.com/reactjs/react-modal/issues/808

... in which the following statement was made:

It's recommended to not use modal with conditional rendering. There is a problem with createPortal when using this way (need to check if it still happening. It starts happening on version +16.3 of react).

Are you all using conditional rendering?

Since react-modal recommends against using conditional rendering, I'm submitting a patch that should hopefully complement other fixes for #589 and also bring the usage of the <Modal /> in this project closer to what is recommended by the upstream project maintainers.

swquinn avatar Jun 18 '22 15:06 swquinn

I wouldn't keep my hopes too high for this PR to get merged. I submitted a trivial PR for the issue you mentioned above almost 3 months ago, but it still didn't get any attention from the maintainers. So I ended up releasing yet-another-react-lightbox.

igordanchenko avatar Jun 20 '22 05:06 igordanchenko

@igordanchenko I'm happy to make this same PR against your yet-another-react-lightbox project as well.

I found it easier just to pull the code for this into the current Next.js blog project I have rather than rely on additional dependencies so this not getting merged in isn't a blocker for me--but I thought I would contribute back to the community. Hopefully someone sees the value in this and approves the CI workflow and merges it.

swquinn avatar Jun 29 '22 18:06 swquinn