Body class not removed under certain circumstances

[Plygon]

Summary:

If React Modal is mounted and almost immediately unmounted, the body's class is not removed when the modal disappears. If this class is used to prevent scrolling, the body will remain unscrollable forever.

Steps to reproduce:

1. Create a project using React Modal.
2. Add the following component somewhere on the page:

   
   import React, { useEffect, useState } from 'react';
   import Modal from 'react-modal';

export const Flash = () => { const [hide, setHide] = useState(0);

useEffect(() => {
    if (!hide) {
        setHide(true);
    }
}, [hide])

return <>
    {!hide && <Modal isOpen={true}>
        <div>
            Modal text.
        </div>
    </Modal>}
    Other text
</>

}

 3. Load the page and inspect the body element. Note that it still has the class `ReactModal__Body--open` even though no modal is open.

### Expected behavior:
The body element should not have the class `ReactModal__Body--open`.

### Link to example of issue:
https://codesandbox.io/s/react-modal-unmount-bug-duvib?file=/src/App.js

### Additional notes:
This is happens because the example component is unmounting the modal immediately in a useEffect. This causes it to unmount after `componentDidMount` is called, but before ModalPortal's `setState` takes effect. So when `componentWillUnmount` is called on ModalPortal, it does not yet have the open state, so it does not decrement the counter of the className.

[diasbruno]

Hi @Plygon

It's not recommended to use conditional rendering on modal !hide && <Modal ... />.

[Plygon]

Hi @diasbruno,

I completely agree that it doesn't make much sense to conditionally render a modal. The example only does so for simplicity's sake.

A more complete example would have the modal nested in a hierarchy of components. If any of its parent components are quickly mounted and unmounted in a similar way, the same effect will happen.

Of course rendering something and then immediately unmounting it is something that everyone should try to avoid, but it should not permanently affect the page in the event that it accidentally happens somewhere.

[diasbruno]

If the component that contains the modal has a conditional rendering, it's the same problem, unfortunately. (and it's had to debug too).

It would be nice to have the component where it is used (cohesion), but because of this problem, we can't have that (because of the animation). 😞

[diasbruno]

Maybe, we can try something that at least guarantee that those classes are removed.

[Plygon]

Maybe the actions in beforeOpen can be called after the isOpen state is set to true, instead of before. That way it won't add the class until the state is actually set. It at least does the trick at first glance.

I have no clue if that would break something else, but I wanted to offer something instead of just reporting this and dipping!

[diasbruno]

This class is added to remove the scroll and fix the position...not sure if it will help in this case.

[GravityStart]

Hello! this issue has been affecting the site for my company and was wondering if any progress has been made for a fix yet before we try to figure out a workaround.

We have our modal buried in components and are having difficulty trying to figure out the cause of the dismount/mounting. In our use case, we lock the scrolling for the page when the modal is up but under specific use cases, exiting the modal does not remove "overflow-hidden" from the body. We figured the problem is similar to if not what Plygon reported here.

[diasbruno]

@GravityStart This is related to conditional rendering and createPortal. It's not recommended using in this case.

[vadymshymko]

Hi there) I ran into the same issue that @Plygon and @GravityStart wrote about. I use className ReactModal__Body--open as a sign that the modal is open. in some cases where the modal is not closed by changing the isOpen property, this className remains. To fix this I used the following approach:

1. I created a wrapper component

   
   import ReactModal from 'react-modal';

ReactModal.setAppElement('my-app-element-id');

function Modal(props) { return <ReactModal {...props} /> }

2. In this component, I added useEffect to the cleanUp function of which I added the following code:
```javascript
  document.body.classList.remove('ReactModal__Body--open');
  document.getElementById('my-app-element-id')?.removeAttribute('aria-hidden');

Finaly, my code looks like this:

import ReactModal from 'react-modal';

ReactModal.setAppElement('my-app-element-id');

function Modal(props) {
  useEffect(() => {
    return () => {
      // this runs before the component is removed from the UI
      document.body.classList.remove('ReactModal__Body--open');
      document.getElementById('my-app-element-id')?.removeAttribute('aria-hidden');
    }
  }, [])

  return <ReactModal {...props} />
}

export default Modal;

// somewhere in some app page:

import Modal from 'src/components/Modal'

function SomeComponent() {
//...
  <Modal {...someReactModalProps}>
  // ...

And everything seems to be working great. But I'm not sure if this solution is correct, so I'm asking for your opinion)

[vadymshymko]

@diasbruno What do you think of my solution to the problem? Is there anything that I didn't consider or missed (regarding the specifics of the react-modal component)? THX

[diasbruno]

@vadymshymko it's ok as a workaround for the problem, but it should not be handled by the user.

We need to investigate this further. Although, I don't have time to work on this, I can assist if you guys want to give it a try. Just @ me an I'll answer.

[Jolsty]

Hi, is there a fix for this?

[AndreasHaaversen]

Here is some more info on this issue for those who stumble upon this thread: The issue seems related to the new mount-unmount-mount cycle React 18 uses for component mounting in StrictMode. I don't have a solution, but the library maintainers probably need to update the mounting and clean-up code to account for this.

[diasbruno]

Unfortunately, this is not an easy fix. There are other threads where I explain that this behavior comes from react 16.3+, where they changed how the lifecycle works, mostly for portals.

Any abruptly destruction of the Modal may cause issues where things are not properly removed, or transitions not working...this scenario is most common when you setup the modal using a "conditional rendering" or even when you have a modal inside of an component that is conditionally rendered.

---

Today, I'd not recommend to use any kind of components that uses portals in react, and, use a parallel component to render modals (modals, popovers, alerts, dialog boxes...) that can be triggered using simple javascript events (you can even embed another "react app" where the modal is rendered).

Taking these components out of the react tree gives you better control over the transitions and positioning.

[AndreasHaaversen]

I see! I thought the info about StrictMode in React 18 could be helpful since we didn't have any problems with React 17, though I now understand that it's more complex than that.

Thanks for the advice on portals and parallel components! 🙌

[atomanyih]

We're finding this can happen due to automated browser tests being super fast, even though we are properly using isOpen

[diasbruno]

@atomanyih Are your tests running in concurrently?

[ADTC]

This also happens when there are modals embedded inside modals, or there are multiple modals on the page, opened by different buttons for example. Sometimes react-modal fails to remove the class ReactModal__Body--open when all modals are closed. Same as #764.

The useEffect hack above won't work because the class should remain if at least one modal is open. We'd need a separate wrapper without the effect for any inner modals, then it would work I suppose.

Is the use of a Context a possible solution by always rendering a single modal persistently in the app (opening and closing it but never removing it from DOM) and just replacing its contents? The problem is, it might be strange if you try to create a modal within another modal. Maybe the inner modal would just replace the outer one completely, losing it entirely?

Maybe the maintainers can make use of an useEffect somewhere that tracks the total count of open modals and removes the class when the count is <= 0. Or IDK.

[diasbruno]

@ADTC Working with a single modal is pretty easy. Once you start stacking and/or nesting modals + react new lifecycle the problems gets really big.

It's impossible to know all the use cases people are doing.

Also, without a proper reproducible example it gets hard to debug.

[ADTC]

The best solution I can come up with is to do your HTML body structure like this:

<body>
  <main className='container'>
    {children}
  </main>
</body>

And have global CSS:

body {
  width: 100vw !important; // not sure if this is necessary
  height: 100vh !important;
  overflow: clip !important;

  > main.container {
    padding: 0 !important; // not sure if this is necessary
    height: calc(100vh - Hpx) !important; // Hpx is any sticky header height. If none, just height: 100vh will do.
    overflow-x: clip !important;
    overflow-y: auto !important;
    overscroll-behavior: contain !important;
  }
}

By permanently clipping body and using an inner container to scroll content, the modal will not scroll the content in the background, and we don't have to rely on react-modal to add/remove overflow dynamically. It will all be elegantly handled by static CSS.

[ADTC]

@diasbruno Sad to say, I could not reproduce in a simple example of nested modals. Hopefully others can use this setup as a base and try to reproduce it: https://codesandbox.io/p/sandbox/react-modal-unmount-bug-forked-gzndqq?file=/src/App.js

I will try again another time. There might be more to this than what a simple example could reproduce.

That being said, I'm very happy with the HTML/CSS-only solution in my last comment. Works excellently well.

[diasbruno]

That's fine. Let me know if you find something, so we can further investigate this.