reactjs / react-modal

Accessible modal dialog component for React
http://reactcommunity.org/react-modal
MIT License
7.37k stars 809 forks source link

2+ modals on same page make an issue #308

Closed rowdyroad closed 7 years ago

rowdyroad commented 7 years ago

Summary:

I what to use < body > class 'ReactModal__Body--open' to freeze background page (overflow:hidden). But there is some issue if we have 2+ on same page (component)

Steps to reproduce:

  1. Create component
  2. Add 2+ < ReactModal > components on page
  3. Set for some of them isOpen attribute to true

Expected behavior:

There is class name of < body > as 'ReactModal__Body--open' if some of ReactModal component isOpen.

Additional notes:

In 1.6.5 version we have this code: if (props.isOpen) { elementClass(document.body).add('ReactModal__Body--open'); } else { elementClass(document.body).remove('ReactModal__Body--open'); }

So, one of < ReactModal > is open and add class to body, some is close and remove it. In Google Dev Console we can see, what react changes body tag constantly (CPU for tab is about 50% for i7).

It can be resolved(workaround) by JSX-condition wrapping and set permanently isOpen={true}

MarkMurphy commented 7 years ago

I think one potential solution here would be to implement a ModalManager that keeps track of open modals. Each modal instance would have the responsibility of registering itself and its state with the ModalManager.

Here's a quick and dirty example:

const registry = new Set()

class Modal extends Component {
  renderPortal(props) {
    const prevRegistrySize = registry.size

    if (props.isOpen) {
      registry.add(this)
    } else {
      registry.delete(this)
    }

    if (registry.size && prevRegistrySize === 0) {
      elementClass(document.body).add('ReactModal__Body--open')
    }
    else if (registry.size === 0 && prevRegistrySize !== 0) {
      elementClass(document.body).remove('ReactModal__Body--open')
    }

    // ...
  }
}
diasbruno commented 7 years ago

Marked as duplicated. #231 See PR #326.