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

Closing a nested modal renderes the parent unscrollable (if body.classList is used) #1189

Closed
kraenhansen opened this issue Aug 28, 2018 · 0 comments · Fixed by #1190 or #1247
Closed

Comments

@kraenhansen
Copy link
Contributor

kraenhansen commented Aug 28, 2018

  • components: Modal
  • reactstrap version #6.4.0
  • import method es
  • react version #16.4.0
  • bootstrap version #4.1.3

What is happening?

  1. Open a modal with a lot of text, which produce a scrollbar.
  2. Open another modal within the first modal.
  3. Perform any unrelated operation on the document.body.classList.
  4. Close the second modal again.
  5. Observer how the first modal is no longer scrollable.

I am using Reactstrap in an app that also uses react-draggable. The latter uses the document.body.classList.remove as it's trying to remove an unrelated class from body when I click close. A side-effect of this is that duplicate classes are removed from the body element and this breaks the Modal component. I don't think the Modal component can assume that other components won't be modifying the body classes using the APIs available.

What should be happening?

I would expect to be able to scroll the initial modal after closing the second modal, even if another module manipulated the document.body.classList.

I suggest the Modal component class keeps a static numeric member (or perhaps a number scoped to the module) of how many modals are currently open and only adds a single modal-open class and removes this when the last modal closes.

Steps to reproduce issue

See "What is happening?".

Code

https://stackblitz.com/edit/reactstrap-pv5c4i?file=Example.js

import React, { Component } from 'react';
import { Button, Modal, ModalBody } from 'reactstrap';

const texts = (new Array(100)).fill("A lot of text ...", 0, 99);

class Example extends Component {
  state = { isSecondOpen: true };

  render() {
    return (
      <Modal isOpen={true}>
        <ModalBody>
          {texts.map((text, i) => <p key={i}>{text}</p>)}
          <Modal isOpen={this.state.isSecondOpen}>
            <ModalBody>
              <Button onClick={this.onAddUnrelatedClass}>
                Add an unrelated class using document.body.classList
              </Button>
              <hr />
              <Button onClick={this.onCloseSecond}>
                Close second Modal
              </Button>
            </ModalBody>
          </Modal>
        </ModalBody>
      </Modal>
    )
  }

  onCloseSecond = () => {
    this.setState({ isSecondOpen: false });
  };

  onAddUnrelatedClass = () => {
    // Let's add an unrelated class just like "react-draggable" removes a class
    document.body.classList.add("an-unrelated-class");
  };
}

export default Example;
@kraenhansen kraenhansen changed the title Closing a nested modal renderes the parent unscrollable Closing a nested modal renderes the parent unscrollable (if body.classList is used) Aug 28, 2018
kraenhansen pushed a commit to kraenhansen/reactstrap that referenced this issue Aug 28, 2018
…ification

Calls to document.body.classList removes duplicate class names.
This commit makes the Modal more resilient to these modifications by keeping a static counter on open Modals and adds "modal-open" only when the
initializing the first modal and removes "modal-open" only when destroying the last modal.

This commit fixes reactstrap#1189
TheSharpieOne pushed a commit that referenced this issue Aug 31, 2018
…ification (#1190)

Calls to document.body.classList removes duplicate class names.
This commit makes the Modal more resilient to these modifications by keeping a static counter on open Modals and adds "modal-open" only when the
initializing the first modal and removes "modal-open" only when destroying the last modal.

fixes #1189
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant