reactstrap: modal-open not being removed from body after closing modal

components: Modal
reactstrap version #6.5.0
import method es
react version #6.6.3
bootstrap version #4.1.3

What is happening?

The modal-open CSS class is not being removed from <body>when I close a modal. Clicking on the header’s “x” button (or any other means of closing the modal) causes the page to be unscrollable.

What should be happening?

The modal-open style should be removed from the page when all modals are closed.

Investigation

Tracing through the Modal code in Chrome devtools, what I saw was two Modal instances being constructed but only one Modal instance is actually being used. The first Modal instance is being constructed by React but is never used after the constructor runs. It’s being thrown away. None of its lifecycle methods are run. The second Modal instance is being used normally and its lifecycle methods fire, but when it closes, Modal.openCount is still nonzero so the modal-open style is not removed from the <body>.

Looking at Modal.js code, I saw a problem which explained the behavior above. The Modal constructor contains a side effect: a call to init() which sets modal-open on the <body>. The constructor (and hence init()) is being called twice, but destroy() is only being called once.

I was able to resolve the problem by forking reactstrap and moving the call to init() from the end of the constructor to the beginning of componentDidMount:

class Modal extends React.Component {
  constructor(props) {
    super(props);

    this._element = null;
    this._originalBodyPadding = null;
    this.getFocusableChildren = this.getFocusableChildren.bind(this);
    this.handleBackdropClick = this.handleBackdropClick.bind(this);
    this.handleBackdropMouseDown = this.handleBackdropMouseDown.bind(this);
    this.handleEscape = this.handleEscape.bind(this);
    this.handleTab = this.handleTab.bind(this);
    this.onOpened = this.onOpened.bind(this);
    this.onClosed = this.onClosed.bind(this);

    this.state = {
      isOpen: props.isOpen,
    };

    // removed
    // if (this.props.isOpen) {
    //   this.init();
    // }
  }

  componentDidMount() {
    // added
    if (this.props.isOpen) {
      this.init();
    }

    if (this.props.onEnter) {
      this.props.onEnter();
    }

    if (this.state.isOpen && this.props.autoFocus) {
      this.setFocus();
    }

    this._isMounted = true;
  }

React advises against side effects in constructors. I suspect the behavior above may be one reason for this recommendation. Apparently, React can construct instances and throw them away without running any lifecycle methods!

I’m happy to prepare a PR to move Modal initialization into componentDidMount(). But there may be other reasons that I’m not aware of to use this non-recommended side effect in the constructor.

Does anyone know the history of why init() is being called in the constructor instead of in componentDidMount where side effects and DOM manipulations are recommended?

Steps to reproduce issue

Repro steps are difficult at the moment because the problem shows up in the middle of a fairly complicated project. To display a modal, my app pushes data into an array that’s stored in React Context (the new 16.x kind, not the deprecated old context). Another part of the app will notice the change to Context and will transform each element in the array into one or more modals. When a modal is closed, the onClosed event handler will remove the data from the array in Context which will cause the modal to be unmounted.

I can pull this logic out of my app to provide a simpler repro, but doing this will take some time so (per discussion above) I wanted to understand more about the reasons for initializing in the constructor before going through the trouble of creating a repro. Hope this is OK.

Error message in console

(none-- it's UI behavior, not an error, that's the problem)

Code

See above-- this behavior is deeply nested inside my app, so it’s non-trivial to pull it out. I can do it if needed, though.

About this issue

Original URL
State: closed
Created 6 years ago
Reactions: 6
Comments: 22 (7 by maintainers)

Commits related to this issue

fix(Modal): fix modal close immediately unmounted #1323 — committed to reactstrap/reactstrap by TheSharpieOne 4 years ago
fix: modal-open class remains on body after modal closed The modal-open class remains on the body tag even after the modal is closed. This is an issue of the reactstrap library (https://github.com/re... — committed to 42BV/ui by deleted user 4 years ago
fix: modal-open class remains on body after modal closed The modal-open class remains on the body tag even after the modal is closed. This is an issue of the reactstrap library (https://github.com/re... — committed to 42BV/ui by deleted user 4 years ago
fix: modal-open class remains on body after modal closed The modal-open class remains on the body tag even after the modal is closed. This is an issue of the reactstrap library (https://github.com/re... — committed to 42BV/ui by deleted user 4 years ago

Most upvoted comments

8.4.1 the issue is still here. I don’t know how to reproduce. It happened after I recently upgraded. The project is big and several months old.

saysmaster on Feb 2, 2020

I’m still having this issue on v. ^8.2.0, does somebody have a workaround? For now I’m going to remove the modal-open class by force using: document.body.classList.remove(‘modal-open’);

lucasfalcaoo on Dec 17, 2019

Fixed my problem for moment…

import React from 'react';
import ReactDOM from 'react-dom';
import { Provider } from 'react-redux';
import { Modal } from "reactstrap";
import 'normalize.css/normalize.css';
import 'alertifyjs/build/css/alertify.min.css';
import 'alertifyjs/build/css/themes/bootstrap.min.css';

import store from '@/redux/store';
import fetchInitialData from '@/redux/fetchInitialData';
import Root from './screens/Root';
import '@/components/PdfViewer/pdfJsPageChange';
/** import  Playground from '@/playground'; */


// store.subscribe(() => console.log(store.getState()));
fetchInitialData();

/** Hotfix css-modal */
Modal.prototype.componentWillUnmount = function() {
  if (this.props.onExit) {
    this.props.onExit();
  }

  if (this._element) {
    this.destroy();
    if (this.props.isOpen || this.state.isOpen) {
      this.close();
    }
  }

  this._isMounted = false;
};
/** Hotfix css-modal */

ReactDOM.render(
  <Provider store={store}>
    <Root />
  </Provider>
  , document.getElementById('ROOT')
);

SteiNNx on Mar 3, 2020

@nibblesnbits,

Yes I came across this issue.

Had to downgrade. The issue is that init() is being called in the life cycle event while the modal is taking itself down.

It might be that its async with the animation so maybe try turning the fade off. Or in your close function just change Modal.openCount = 0; yourself.

Basically the modal count is always 1 (never 0 and thus never closes)

bigorangemachine on May 10, 2019