react-overlays: Modal does not work with React 16

Probably the same issue as https://github.com/facebook/react/issues/9808. When the Modal is shown and tries to get focus (via componentDidUpdate -> onShow - focus), setModalNode will not have been called and getDialogElement will return undefined.

About this issue

  • Original URL
  • State: closed
  • Created 7 years ago
  • Reactions: 24
  • Comments: 44 (10 by maintainers)

Commits related to this issue

Most upvoted comments

FYI React 16 is released, so this just became a blocker for people to be able to upgrade.

Fo v16 we’ll want to switch to the new (YAH!) portal api

I threw this in my startup file for now:

import { Modal } from 'react-overlays';

Modal.prototype.componentWillMount = function () {
    this.focus = () => {};
};

That way I don’t have to change every single place I import Modal(there’s a lot of em!).

version 0.7.3 seems to have fixed the issue.

Thanks for the work on this!

This shim has worked fine for us - it bypasses some functionality but it got us unblocked.

const {Modal} = require('react-overlays');

const focus = () => {};

const cDU = Modal.prototype.componentDidUpdate;

Modal.prototype.componentDidUpdate = function(prevProps: any) {
  if (this.focus !== focus) {
    this.focus = focus;
  }
  cDU.call(this, prevProps);
};

If you are using webpack you need to use this fix like this:

import Modal from 'react-bootstrap/node_modules/react-overlays/lib/Modal';

Modal.prototype.componentWillMount = function componentWillMount() {
  this.focus = function focus() {};
};

I dug into this a bit to see what the root problem was. The core problem is react-dom’s commitAllLifeCycles invokes commitAttachRef after commitLifeCycles. This means that setModalNode (which assigns the local modalNode field) is invoked after componentDidMount (which calls onShow and by extension focus).

What this means is that (given that react-dom does not change its commitAllLifeCycles function), we remove the onShow from componentDidMount and move it to setModalNode. I have tested this and it most definitely works as expected.

  setModalNode = (ref) => {
    this.modalNode = ref;
    if (ref && this.props.show) {
      this.onShow();
    }
  }

as a temporary shim, this should work for most setups.

import { Modal } from 'react-overlays';
Modal.prototype.componentWillMount = function() {
  this.setModalNode = function(ref: any) {
    this.modalNode = ref;
    if (ref != null && this.props.show) {
      this.onShow();
    }
  }.bind(this);
};
Modal.prototype.componentDidMount = function() {
  this._isMounted = true;
};

you can see that there are a few efforts to fix this. IF you want stuff to go faster jump in on the PR’s and review/test them

@geminiyellow this has been fixed in 0.7.3, but not yet in the 0.8 branch (as of 0.8.2).

I worked around that issue by creating a custom Modal wrapper component which monkey-patches cDM and cDU as outlined above:

import Modal from 'react-overlays/lib/Modal';

const focus = () => {};

const cDU = Modal.prototype.componentDidUpdate;
const cDM = Modal.prototype.componentDidMount;

// react-overlays v0.8.2 doesn't work with React 16, so we have to monkey-patch it.
// TODO remove this once react-overlays is compatible with React 16.
Modal.prototype.componentDidUpdate = function componentDidUpdatePatched(prevProps: any) {
  if (this.focus !== focus) {
    this.focus = focus;
  }
  cDU.call(this, prevProps);
};

Modal.prototype.componentDidMount = function componentDidMountPatched() {
  if (this.focus !== focus) {
    this.focus = focus;
  }
  cDM.call(this);
};

export default Modal;

Once 0.7.3 was released, I just downgraded the react-overlays version from ^0.8.2 to ^0.7.3, so that patch isn’t needed any more.

@seeden i try to use your solution in the component but i get some error, below is the error i got

Modal.prototype.componentWillMount = function componentWillMount() {
     |        ^
  20 |     this.focus = () => {};
  21 |   };
  22 | 

i have put the code out of render in component.

@connorjburton actually i urgently need to fix the issue in my project. so if i get any working patch solution than that’s fine, we can fix the temporary with the official update later. but right now i need a fix. if you can give me some idea it will be great.

i already try to integrate the patches mentioned above in my project, but couldn’t be able to make it workable.

still not working

TypeError: Cannot read property 'contains' of undefined
    at app-bundle-9f3367cd4c8da193f2df7875ec037f39ff0441e251ba61c728a43c8363174343.js:24030
    at Modal.focus (app-bundle-9f3367cd4c8da193f2df7875ec037f39ff0441e251ba61c728a43c8363174343.js:65592)
    at Modal.onShow (app-bundle-9f3367cd4c8da193f2df7875ec037f39ff0441e251ba61c728a43c8363174343.js:65521)
    at Modal.componentDidUpdate (app-bundle-9f3367cd4c8da193f2df7875ec037f39ff0441e251ba61c728a43c8363174343.js:65278)
    at commitLifeCycles (vendor-69769756228c5b97a31cdefe85d1499ffa9865cebae44c24682bb47a8949d3eb.js:20358)
    at c (vendor-69769756228c5b97a31cdefe85d1499ffa9865cebae44c24682bb47a8949d3eb.js:20369)
    at k (vendor-69769756228c5b97a31cdefe85d1499ffa9865cebae44c24682bb47a8949d3eb.js:20372)
    at p (vendor-69769756228c5b97a31cdefe85d1499ffa9865cebae44c24682bb47a8949d3eb.js:20373)
    at batchedUpdates (vendor-69769756228c5b97a31cdefe85d1499ffa9865cebae44c24682bb47a8949d3eb.js:20379)
    at qb (vendor-69769756228c5b97a31cdefe85d1499ffa9865cebae44c24682bb47a8949d3eb.js:20230)

@nelsongustavo maybe try

import { Modal as ReactOverlayModal } from 'react-overlays';

class Modal extends ReactOverlayModal {
  focus() {};
}

export default Modal;