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

react-bootstrap's Modal does not work with React 16 #2812

Closed
ducdigital opened this Issue Sep 27, 2017 · 37 comments

Comments

Projects
None yet
@ducdigital

ducdigital commented Sep 27, 2017

Look like the recent react's update has cause react-bootstrap modal to stop working. Here are the errors:

image

image

This issue is also discussed on react-overlays

react-bootstrap/react-overlays#188

@jerry-duke

This comment has been minimized.

Show comment
Hide comment
@jerry-duke

jerry-duke Sep 27, 2017

Trivially reproduced with the 2nd Bootstrap Modal sample class (the one opened by a Button) pasted into create-react-app's App.js file.

jerry-duke commented Sep 27, 2017

Trivially reproduced with the 2nd Bootstrap Modal sample class (the one opened by a Button) pasted into create-react-app's App.js file.

@andrewBalekha

This comment has been minimized.

Show comment
Hide comment
@andrewBalekha

andrewBalekha Sep 28, 2017

As a workaround one can create new component

import React from 'react'
import { Modal, Fade, utils } from 'react-bootstrap'
import { Modal as ReactOverlayModal } from 'react-overlays'
import classNames from 'classnames'

class BaseModal extends ReactOverlayModal {
  focus = () => {}
}

class BootstrapModal extends Modal {
  render () {
    const {
      backdrop,
      backdropClassName,
      animation,
      show,
      dialogComponentClass: Dialog,
      className,
      style,
      children, // Just in case this get added to BaseModal propTypes.
      onEntering,
      onExited,
      ...props
    } = this.props

    const [baseModalProps, dialogProps] = splitComponentProps(props, BaseModal)

    const inClassName = show && !animation && 'in'

    return (
      <BaseModal
        {...baseModalProps}
        ref={c => {
          this._modal = c
        }}
        show={show}
        onEntering={utils.createChainedFunction(
          onEntering,
          this.handleEntering
        )}
        onExited={utils.createChainedFunction(onExited, this.handleExited)}
        backdrop={backdrop}
        backdropClassName={classNames(
          utils.bootstrapUtils.prefix(props, 'backdrop'),
          backdropClassName,
          inClassName
        )}
        containerClassName={utils.bootstrapUtils.prefix(props, 'open')}
        transition={animation ? Fade : undefined}
        dialogTransitionTimeout={Modal.TRANSITION_DURATION}
        backdropTransitionTimeout={Modal.BACKDROP_TRANSITION_DURATION}
      >
        <Dialog
          {...dialogProps}
          style={{ ...this.state.style, ...style }}
          className={classNames(className, inClassName)}
          onClick={backdrop === true ? this.handleDialogClick : null}
        >
          {children}
        </Dialog>
      </BaseModal>
    )
  }
}

const splitComponentProps = (props, Component) => {
  const componentPropTypes = Component.propTypes

  const parentProps = {}
  const childProps = {}

  Object.entries(props).forEach(([propName, propValue]) => {
    if (componentPropTypes[propName]) {
      parentProps[propName] = propValue
    } else {
      childProps[propName] = propValue
    }
  })

  return [parentProps, childProps]
}

export default BootstrapModal

Or maybe someone has better solution would be nice.

andrewBalekha commented Sep 28, 2017

As a workaround one can create new component

import React from 'react'
import { Modal, Fade, utils } from 'react-bootstrap'
import { Modal as ReactOverlayModal } from 'react-overlays'
import classNames from 'classnames'

class BaseModal extends ReactOverlayModal {
  focus = () => {}
}

class BootstrapModal extends Modal {
  render () {
    const {
      backdrop,
      backdropClassName,
      animation,
      show,
      dialogComponentClass: Dialog,
      className,
      style,
      children, // Just in case this get added to BaseModal propTypes.
      onEntering,
      onExited,
      ...props
    } = this.props

    const [baseModalProps, dialogProps] = splitComponentProps(props, BaseModal)

    const inClassName = show && !animation && 'in'

    return (
      <BaseModal
        {...baseModalProps}
        ref={c => {
          this._modal = c
        }}
        show={show}
        onEntering={utils.createChainedFunction(
          onEntering,
          this.handleEntering
        )}
        onExited={utils.createChainedFunction(onExited, this.handleExited)}
        backdrop={backdrop}
        backdropClassName={classNames(
          utils.bootstrapUtils.prefix(props, 'backdrop'),
          backdropClassName,
          inClassName
        )}
        containerClassName={utils.bootstrapUtils.prefix(props, 'open')}
        transition={animation ? Fade : undefined}
        dialogTransitionTimeout={Modal.TRANSITION_DURATION}
        backdropTransitionTimeout={Modal.BACKDROP_TRANSITION_DURATION}
      >
        <Dialog
          {...dialogProps}
          style={{ ...this.state.style, ...style }}
          className={classNames(className, inClassName)}
          onClick={backdrop === true ? this.handleDialogClick : null}
        >
          {children}
        </Dialog>
      </BaseModal>
    )
  }
}

const splitComponentProps = (props, Component) => {
  const componentPropTypes = Component.propTypes

  const parentProps = {}
  const childProps = {}

  Object.entries(props).forEach(([propName, propValue]) => {
    if (componentPropTypes[propName]) {
      parentProps[propName] = propValue
    } else {
      childProps[propName] = propValue
    }
  })

  return [parentProps, childProps]
}

export default BootstrapModal

Or maybe someone has better solution would be nice.

@touqeerkhan11

This comment has been minimized.

Show comment
Hide comment
@touqeerkhan11

touqeerkhan11 Sep 28, 2017

I just lost the mount animation by using this.. But its working though with r16 so thats good

touqeerkhan11 commented Sep 28, 2017

I just lost the mount animation by using this.. But its working though with r16 so thats good

@DZuz14

This comment has been minimized.

Show comment
Hide comment
@DZuz14

DZuz14 Sep 28, 2017

@andrewBalekha answer is the quickest fix for me right now. I can attest as a temporary fix, that will work, minus the entry animation of the modal.

DZuz14 commented Sep 28, 2017

@andrewBalekha answer is the quickest fix for me right now. I can attest as a temporary fix, that will work, minus the entry animation of the modal.

@anaumov

This comment has been minimized.

Show comment
Hide comment
@anaumov

anaumov Sep 29, 2017

I used shim for react overlay modal inspired by react-bootstrap/react-overlays#188 (comment)

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

const focus = () => {};
const cDU = ReactOverlayModal.prototype.componentDidUpdate;
const cDM = ReactOverlayModal.prototype.componentDidMount;

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

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

export default Modal;

anaumov commented Sep 29, 2017

I used shim for react overlay modal inspired by react-bootstrap/react-overlays#188 (comment)

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

const focus = () => {};
const cDU = ReactOverlayModal.prototype.componentDidUpdate;
const cDM = ReactOverlayModal.prototype.componentDidMount;

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

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

export default Modal;
@andrewBalekha

This comment has been minimized.

Show comment
Hide comment
@andrewBalekha

andrewBalekha commented Sep 29, 2017

@anaumov nice, thanks

@gtwilliams03

This comment has been minimized.

Show comment
Hide comment
@gtwilliams03

gtwilliams03 Sep 29, 2017

@anaumov Thank you for the Modal shim - it got my project working again. However, I had to make a couple of changes to it to check if this exists because this was undefined even in cDM and cDU:

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

const focus = () => {}
const cDU = ReactOverlayModal.prototype.componentDidUpdate
const cDM = ReactOverlayModal.prototype.componentDidMount

ReactOverlayModal.prototype.componentDidUpdate = prevProps => {
  if (this) { // <- check for this exists
    if (this.focus !== focus) {
      this.focus = focus
    }
    cDU.call(this, prevProps)
  } //
}

ReactOverlayModal.prototype.componentDidMount = () => {
  if (this) { // <- check for this exists
    if (this.focus !== focus) {
      this.focus = focus
    }
    cDM.call(this)
  } //
}

export default Modal

And, the shim is still throwing an error onHide:

Modal.js:496 Uncaught TypeError: Cannot read property 'remove' of undefined
    at Modal.onHide (Modal.js:496)
    at Object.handleHidden [as onExited] (Modal.js:515)
    at Transition.js:99
    at Transition._this.nextCallback (Transition.js:134)
    at commitCallbacks (react-dom.development.js:7250)
    at commitLifeCycles (react-dom.development.js:11524)
    at commitAllLifeCycles (react-dom.development.js:12294)
    at HTMLUnknownElement.callCallback (react-dom.development.js:1299)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:1338)
    at invokeGuardedCallback (react-dom.development.js:1195)
onHide @ Modal.js:496
handleHidden @ Modal.js:515
(anonymous) @ Transition.js:99
_this.nextCallback @ Transition.js:134
commitCallbacks @ react-dom.development.js:7250
commitLifeCycles @ react-dom.development.js:11524
commitAllLifeCycles @ react-dom.development.js:12294
callCallback @ react-dom.development.js:1299
invokeGuardedCallbackDev @ react-dom.development.js:1338
invokeGuardedCallback @ react-dom.development.js:1195
commitAllWork @ react-dom.development.js:12415
workLoop @ react-dom.development.js:12687
callCallback @ react-dom.development.js:1299
invokeGuardedCallbackDev @ react-dom.development.js:1338
invokeGuardedCallback @ react-dom.development.js:1195
performWork @ react-dom.development.js:12800
scheduleUpdateImpl @ react-dom.development.js:13185
scheduleUpdate @ react-dom.development.js:13124
enqueueSetState @ react-dom.development.js:9646
760.ReactComponent.setState @ react.development.js:218
Transition._this.safeSetState @ Transition.js:123
(anonymous) @ Transition.js:98
_this.nextCallback @ Transition.js:134
react-dom.development.js:8305 The above error occurred in the <Transition> component:
    in Transition (created by Fade)
    in Fade (created by Modal)
    in div (created by Modal)

Hope this helps someone!

gtwilliams03 commented Sep 29, 2017

@anaumov Thank you for the Modal shim - it got my project working again. However, I had to make a couple of changes to it to check if this exists because this was undefined even in cDM and cDU:

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

const focus = () => {}
const cDU = ReactOverlayModal.prototype.componentDidUpdate
const cDM = ReactOverlayModal.prototype.componentDidMount

ReactOverlayModal.prototype.componentDidUpdate = prevProps => {
  if (this) { // <- check for this exists
    if (this.focus !== focus) {
      this.focus = focus
    }
    cDU.call(this, prevProps)
  } //
}

ReactOverlayModal.prototype.componentDidMount = () => {
  if (this) { // <- check for this exists
    if (this.focus !== focus) {
      this.focus = focus
    }
    cDM.call(this)
  } //
}

export default Modal

And, the shim is still throwing an error onHide:

Modal.js:496 Uncaught TypeError: Cannot read property 'remove' of undefined
    at Modal.onHide (Modal.js:496)
    at Object.handleHidden [as onExited] (Modal.js:515)
    at Transition.js:99
    at Transition._this.nextCallback (Transition.js:134)
    at commitCallbacks (react-dom.development.js:7250)
    at commitLifeCycles (react-dom.development.js:11524)
    at commitAllLifeCycles (react-dom.development.js:12294)
    at HTMLUnknownElement.callCallback (react-dom.development.js:1299)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:1338)
    at invokeGuardedCallback (react-dom.development.js:1195)
onHide @ Modal.js:496
handleHidden @ Modal.js:515
(anonymous) @ Transition.js:99
_this.nextCallback @ Transition.js:134
commitCallbacks @ react-dom.development.js:7250
commitLifeCycles @ react-dom.development.js:11524
commitAllLifeCycles @ react-dom.development.js:12294
callCallback @ react-dom.development.js:1299
invokeGuardedCallbackDev @ react-dom.development.js:1338
invokeGuardedCallback @ react-dom.development.js:1195
commitAllWork @ react-dom.development.js:12415
workLoop @ react-dom.development.js:12687
callCallback @ react-dom.development.js:1299
invokeGuardedCallbackDev @ react-dom.development.js:1338
invokeGuardedCallback @ react-dom.development.js:1195
performWork @ react-dom.development.js:12800
scheduleUpdateImpl @ react-dom.development.js:13185
scheduleUpdate @ react-dom.development.js:13124
enqueueSetState @ react-dom.development.js:9646
760.ReactComponent.setState @ react.development.js:218
Transition._this.safeSetState @ Transition.js:123
(anonymous) @ Transition.js:98
_this.nextCallback @ Transition.js:134
react-dom.development.js:8305 The above error occurred in the <Transition> component:
    in Transition (created by Fade)
    in Fade (created by Modal)
    in div (created by Modal)

Hope this helps someone!

@nelsongustavo

This comment has been minimized.

Show comment
Hide comment
@nelsongustavo

nelsongustavo commented Sep 30, 2017

@gtwilliams03 Same problem here...

@alberdonpi

This comment has been minimized.

Show comment
Hide comment
@alberdonpi

alberdonpi Oct 1, 2017

If it helps to anyone, I was having this issue after upgrading react, then checked the react-bootstrap package.json file and found this:

"peerDependencies": {
    "react": "^0.14.9 || >=15.3.0",
    "react-dom": "^0.14.9 || >=15.3.0"
},

So I downgraded the versions to the ones required and now it works fine.

I understand this is not a final solution, but if you don't need the last React version, this could help.

alberdonpi commented Oct 1, 2017

If it helps to anyone, I was having this issue after upgrading react, then checked the react-bootstrap package.json file and found this:

"peerDependencies": {
    "react": "^0.14.9 || >=15.3.0",
    "react-dom": "^0.14.9 || >=15.3.0"
},

So I downgraded the versions to the ones required and now it works fine.

I understand this is not a final solution, but if you don't need the last React version, this could help.

@touqeerkhan11

This comment has been minimized.

Show comment
Hide comment
@touqeerkhan11

touqeerkhan11 Oct 1, 2017

Given that we are loosing mount transitions when hacking it with a shim, and after messing with the transition api provided with react-overlays, I came to a conclusion that using new react portals is the best (and easiest) solution to hack your own modal, PS animating with react-motion

Here I made an example repo if anyone is interested https://github.com/touqeerkhan11/react-portal-example (demo https://touqeerkhan11.github.io/react-portal-example/)

touqeerkhan11 commented Oct 1, 2017

Given that we are loosing mount transitions when hacking it with a shim, and after messing with the transition api provided with react-overlays, I came to a conclusion that using new react portals is the best (and easiest) solution to hack your own modal, PS animating with react-motion

Here I made an example repo if anyone is interested https://github.com/touqeerkhan11/react-portal-example (demo https://touqeerkhan11.github.io/react-portal-example/)

@wzup

This comment has been minimized.

Show comment
Hide comment
@wzup

wzup Oct 5, 2017

Same here.
When will there be an upgrade for react-overlays so that overlays are compatible with react 16?
Meanwhile compelled to downgrade React back to v 15.5.4.

Looks like this is how third party plugins suspend penetration of progress)

contains.js:17 Uncaught TypeError: Cannot read property 'contains' of undefined

modal error - 2017-10-05 11 43 16

wzup commented Oct 5, 2017

Same here.
When will there be an upgrade for react-overlays so that overlays are compatible with react 16?
Meanwhile compelled to downgrade React back to v 15.5.4.

Looks like this is how third party plugins suspend penetration of progress)

contains.js:17 Uncaught TypeError: Cannot read property 'contains' of undefined

modal error - 2017-10-05 11 43 16

@TLiu2014

This comment has been minimized.

Show comment
Hide comment
@TLiu2014

TLiu2014 Oct 5, 2017

Same issue here. Then I go back to react and react-dom v15.6.2, everything works well.

TLiu2014 commented Oct 5, 2017

Same issue here. Then I go back to react and react-dom v15.6.2, everything works well.

@lunikernel

This comment has been minimized.

Show comment
Hide comment
@lunikernel

lunikernel Oct 7, 2017

I've upgraded my codebase to React 16.

I got the modals working, but the background is not darkened around the modal.

Also, the "show" argument doesn't seem to work.

lunikernel commented Oct 7, 2017

I've upgraded my codebase to React 16.

I got the modals working, but the background is not darkened around the modal.

Also, the "show" argument doesn't seem to work.

@GeorgeCrisan

This comment has been minimized.

Show comment
Hide comment
@GeorgeCrisan

GeorgeCrisan Oct 7, 2017

well I just had to run on my terminal $ npm install --save react@15.6.2 react-dom@15.6.2
and this sorted out all my problems

GeorgeCrisan commented Oct 7, 2017

well I just had to run on my terminal $ npm install --save react@15.6.2 react-dom@15.6.2
and this sorted out all my problems

@boaticus

This comment has been minimized.

Show comment
Hide comment
@boaticus

boaticus Oct 8, 2017

When I upgraded to React 16, everything in my app worked as normal except for react-bootstrap Modals.

I'm seeing the same context.contains error as the original issuer showed in their screenshot.

Any idea of a fix to Modal?

The shims mentioned above introduce their own problems, including this js warning caused by the appear attribute in Fade Warning: Received 'true' for non-boolean attribute 'appear'. If this is expected, cast the value to a string. Also with the shims above, my modal backdrop doesn't appear and the animations are lost.

Switching to the react-motion solution mentioned above isn't a drop-in solution, it requires reworking a lot of things, work I'd rather not undertake if there's a fix for bootstrap Modals coming soon.

Downgrading to previous (15.x) version of React is obviously less than ideal.

Anyone have a better fix?

UPDATE 2017.10.14:
As others mention below, version 0.7.3 of react-overlays seems to have fixed the issue. I reinstalled react-bootstrap with react-overlays via npm and Modals seems to work now with React 16.

boaticus commented Oct 8, 2017

When I upgraded to React 16, everything in my app worked as normal except for react-bootstrap Modals.

I'm seeing the same context.contains error as the original issuer showed in their screenshot.

Any idea of a fix to Modal?

The shims mentioned above introduce their own problems, including this js warning caused by the appear attribute in Fade Warning: Received 'true' for non-boolean attribute 'appear'. If this is expected, cast the value to a string. Also with the shims above, my modal backdrop doesn't appear and the animations are lost.

Switching to the react-motion solution mentioned above isn't a drop-in solution, it requires reworking a lot of things, work I'd rather not undertake if there's a fix for bootstrap Modals coming soon.

Downgrading to previous (15.x) version of React is obviously less than ideal.

Anyone have a better fix?

UPDATE 2017.10.14:
As others mention below, version 0.7.3 of react-overlays seems to have fixed the issue. I reinstalled react-bootstrap with react-overlays via npm and Modals seems to work now with React 16.

@jquense

This comment has been minimized.

Show comment
Hide comment
@jquense

jquense Oct 8, 2017

Member

There is no good fix until we can upgrade the underlying portal component to the new react v16 api, which is almost done, we are working through some api considerations in the PR until then tho the modal isn't supported in v16

Member

jquense commented Oct 8, 2017

There is no good fix until we can upgrade the underlying portal component to the new react v16 api, which is almost done, we are working through some api considerations in the PR until then tho the modal isn't supported in v16

@idangozlan

This comment has been minimized.

Show comment
Hide comment
@idangozlan

idangozlan Oct 9, 2017

@andrewBalekha workaround working great for until version 0.8 which using portals and other props than <0.8 versions. Thanks for the workaround.

idangozlan commented Oct 9, 2017

@andrewBalekha workaround working great for until version 0.8 which using portals and other props than <0.8 versions. Thanks for the workaround.

@Sram5368

This comment has been minimized.

Show comment
Hide comment
@Sram5368

Sram5368 Oct 10, 2017

@jquense can you please confirm when we will get this fix done for Modals? any release planned any time soon?

Sram5368 commented Oct 10, 2017

@jquense can you please confirm when we will get this fix done for Modals? any release planned any time soon?

@aljachimiak

This comment has been minimized.

Show comment
Hide comment
@aljachimiak

aljachimiak Oct 11, 2017

Hello - I ran into this issue last night...

Here is a very simple demo app that does a few things:

  • demonstrates how to use shim posted by @anaumov
  • makes it easy to comment/uncomment lines to flip back to the failing state.

Hope you get this sorted out soon - Cheers!

aljachimiak commented Oct 11, 2017

Hello - I ran into this issue last night...

Here is a very simple demo app that does a few things:

  • demonstrates how to use shim posted by @anaumov
  • makes it easy to comment/uncomment lines to flip back to the failing state.

Hope you get this sorted out soon - Cheers!

@mclapa

This comment has been minimized.

Show comment
Hide comment
@mclapa

mclapa Oct 11, 2017

An alternative to the show (until it gets fixed) is just wrap the modal in an if statement which is toggled in state change

{this.state.showModal ? (
<Modal.Dialog>
...
</Modal.Dialog>
) : null}

The only downside is the click-outside-box-to-close won't work but you can still add a 'close' button in the modal that will trigger the hide

mclapa commented Oct 11, 2017

An alternative to the show (until it gets fixed) is just wrap the modal in an if statement which is toggled in state change

{this.state.showModal ? (
<Modal.Dialog>
...
</Modal.Dialog>
) : null}

The only downside is the click-outside-box-to-close won't work but you can still add a 'close' button in the modal that will trigger the hide

@jharris4

This comment has been minimized.

Show comment
Hide comment
@jharris4

jharris4 Oct 14, 2017

Contributor

version 0.7.3 of react-overlays seems to have fixed the issue.

I reinstalled react-bootstrap from npm (which grabbed the new react-overlays version) and it seems to work now with React 16.

Contributor

jharris4 commented Oct 14, 2017

version 0.7.3 of react-overlays seems to have fixed the issue.

I reinstalled react-bootstrap from npm (which grabbed the new react-overlays version) and it seems to work now with React 16.

@kieran

This comment has been minimized.

Show comment
Hide comment
@kieran

kieran Oct 14, 2017

note for yarn users: you may have to remove, then add react-bootstrap for the dependencies to properly update.

Working for me now, thanks for the updates!

kieran commented Oct 14, 2017

note for yarn users: you may have to remove, then add react-bootstrap for the dependencies to properly update.

Working for me now, thanks for the updates!

@boaticus

This comment has been minimized.

Show comment
Hide comment
@boaticus

boaticus Oct 14, 2017

Fixed for me as well!

boaticus commented Oct 14, 2017

Fixed for me as well!

@dukedougal

This comment has been minimized.

Show comment
Hide comment
@dukedougal

dukedougal Oct 15, 2017

Is this actually fixed? Would someone mind giving a simple example please of a modal that implements the onExited event and the "show" property? I can't find the syntax to make it work.

thanks

dukedougal commented Oct 15, 2017

Is this actually fixed? Would someone mind giving a simple example please of a modal that implements the onExited event and the "show" property? I can't find the syntax to make it work.

thanks

cosm1c added a commit to cosm1c/health-mesh that referenced this issue Oct 15, 2017

juliozimmermann pushed a commit to juliozimmermann/udacity-readable that referenced this issue Oct 15, 2017

Júlio Zimmermann Júlio Zimmermann
refactor: Applyed suggestions from code review. I have changed packag…
…e.json for not accept React up than version 15 (react-bootstrap/react-bootstrap#2812) to try to fix the error Modal reported by the reviewer even that in my local environment, it is not happening.
@kanwar-saad

This comment has been minimized.

Show comment
Hide comment
@kanwar-saad

kanwar-saad Oct 15, 2017

Working for me now .. after reinstalling react-bootstrap from npm. But why this bug is still kept open .. Is there something left still?

kanwar-saad commented Oct 15, 2017

Working for me now .. after reinstalling react-bootstrap from npm. But why this bug is still kept open .. Is there something left still?

@taion

This comment has been minimized.

Show comment
Hide comment
@taion

taion Oct 16, 2017

Member

This is in fact fixed. Wipe cache and reinstall w/the latest react-overlays dep.

Member

taion commented Oct 16, 2017

This is in fact fixed. Wipe cache and reinstall w/the latest react-overlays dep.

@taion taion closed this Oct 16, 2017

@shokai

This comment has been minimized.

Show comment
Hide comment
@shokai

shokai Oct 16, 2017

After updating react-overlay npm, I confirmed that it works fine with React 16 on local development.

But I am using cdnjs.com on production. The react-bootstrap 0.31.3 hosted on cdnjs is bundling the old react-overlay npm.
https://cdnjs.com/libraries/react-bootstrap

I'm happy that you can also update CDN.

shokai commented Oct 16, 2017

After updating react-overlay npm, I confirmed that it works fine with React 16 on local development.

But I am using cdnjs.com on production. The react-bootstrap 0.31.3 hosted on cdnjs is bundling the old react-overlay npm.
https://cdnjs.com/libraries/react-bootstrap

I'm happy that you can also update CDN.

@jktravis

This comment has been minimized.

Show comment
Hide comment
@jktravis

jktravis Oct 16, 2017

λ npm info react-overlays dist-tags
{ latest: '0.7.3' }

λ npm info react-bootstrap dependencies

{ 'babel-runtime': '^6.11.6',
  classnames: '^2.2.5',
  'dom-helpers': '^3.2.0',
  invariant: '^2.2.1',
  keycode: '^2.1.2',
  'prop-types': '^15.5.10',
  'prop-types-extra': '^1.0.1',
  'react-overlays': '^0.7.0', // <--- Should this be 0.7.3? 
  'react-prop-types': '^0.4.0',
  uncontrollable: '^4.1.0',
  warning: '^3.0.0' }

jktravis commented Oct 16, 2017

λ npm info react-overlays dist-tags
{ latest: '0.7.3' }

λ npm info react-bootstrap dependencies

{ 'babel-runtime': '^6.11.6',
  classnames: '^2.2.5',
  'dom-helpers': '^3.2.0',
  invariant: '^2.2.1',
  keycode: '^2.1.2',
  'prop-types': '^15.5.10',
  'prop-types-extra': '^1.0.1',
  'react-overlays': '^0.7.0', // <--- Should this be 0.7.3? 
  'react-prop-types': '^0.4.0',
  uncontrollable: '^4.1.0',
  warning: '^3.0.0' }
@juanpicado

This comment has been minimized.

Show comment
Hide comment
@juanpicado

juanpicado Oct 17, 2017

I'd suggest removing either yarn.lock or package-lock.json in your local env otherwise react-overlays 0.7.3 will never be resolved properly.

Also, it would be helpful to move "react-overlays": "^0.7.0", to "^0.7.3" then remove lock files won't be necessary as @jktravis pointed out.

One more thing, the patch in "^0.7.3" fixed the issue for us 👍 .

juanpicado commented Oct 17, 2017

I'd suggest removing either yarn.lock or package-lock.json in your local env otherwise react-overlays 0.7.3 will never be resolved properly.

Also, it would be helpful to move "react-overlays": "^0.7.0", to "^0.7.3" then remove lock files won't be necessary as @jktravis pointed out.

One more thing, the patch in "^0.7.3" fixed the issue for us 👍 .

@taylorham

This comment has been minimized.

Show comment
Hide comment
@taylorham

taylorham Oct 17, 2017

Thanks for all the hard work getting this one ironed out!

Any idea when this will be published to the react-bootstrap cdn bundle?

It's also still affecting live editors like this example on codesandbox.io.

taylorham commented Oct 17, 2017

Thanks for all the hard work getting this one ironed out!

Any idea when this will be published to the react-bootstrap cdn bundle?

It's also still affecting live editors like this example on codesandbox.io.

@DaddyWarbucks

This comment has been minimized.

Show comment
Hide comment
@DaddyWarbucks

DaddyWarbucks Oct 17, 2017

Really impressive turnaround on this issue! Kudos to the maintainers and also the community.

DaddyWarbucks commented Oct 17, 2017

Really impressive turnaround on this issue! Kudos to the maintainers and also the community.

@adrienDog

This comment has been minimized.

Show comment
Hide comment
@adrienDog

adrienDog Oct 18, 2017

Can confirm it works!! Amazing thank you!

adrienDog commented Oct 18, 2017

Can confirm it works!! Amazing thank you!

galadhremmin added a commit to galadhremmin/Parf-Edhellen that referenced this issue Oct 18, 2017

Improved cache control and React Modal fix
- Fixed a bug which prevented modals from opening up after upgrading to
  React 16. For more information, see:
  react-bootstrap/react-bootstrap#2812
- Introduced ED_VERSION which will henceforth version assets, to ensure
  that the visiting clients always have the latest version of the site.

adamnagel pushed a commit to metamorph-inc/openmeta-mms that referenced this issue Oct 18, 2017

ksmyth pushed a commit to metamorph-inc/openmeta-visualizer that referenced this issue Oct 18, 2017

Revert upgrade to React 16 due to broken react-bootstrap modals.
(See Github issue react-bootstrap/react-bootstrap#2812)

This reverts commit d9b0d77e3f14c2d2619506958b2e3b87b8bb985d.
@jahlela

This comment has been minimized.

Show comment
Hide comment
@jahlela

jahlela Oct 23, 2017

Hey folks! I'm upgrading to React16 and also ran into the Cannot read property 'contains' of undefined error when trying to open a react-bootstrap modal. I followed the aboveadvice to delete package-lock.json and reinstall react-bootstrap@^0.31.3. Now my modal opens as expected, but the onHide is no longer working for click events (Close button, x in the corner, and outside the modal). Using this esc key is the only way to dismiss the modal.

The only thing that has changed is the upgrades for react and react-bootstrap, so I'm wondering if there is another step I missed?

Thanks!

const ExampleModal = ({
    isShownModal,
    onHide,
}) => (
    <Modal show={isShownModal} onHide={onHide}>
        <Modal.Header closeButton>
            <Modal.Title>Example Title</Modal.Title>
        </Modal.Header>

        <Modal.Body>
            <h5>Example body content</h5>
        </Modal.Body>

        <Modal.Footer>
            <Button onClick={onHide}>Close</Button>
        </Modal.Footer>
    </Modal>
)

export default class ExampleModal extends React.Component {    
    constructor(props) {
        super(props)
        this.state = {isShownModal: false}
    }

    openModal = () => {this.setState({ isShownModal: true })}
    closeModal = () => {this.setState({ isShownModal: false })}

    render() {
        return (
            <span onClick={this.openModal}>
                <i className="fa fa-question-circle mr-3"></i>
                <ExampleModal 
                    isShownModal={this.state.isShownModal} 
                    onHide={this.closeModal}
                />
            </span>
        )
    }
}

jahlela commented Oct 23, 2017

Hey folks! I'm upgrading to React16 and also ran into the Cannot read property 'contains' of undefined error when trying to open a react-bootstrap modal. I followed the aboveadvice to delete package-lock.json and reinstall react-bootstrap@^0.31.3. Now my modal opens as expected, but the onHide is no longer working for click events (Close button, x in the corner, and outside the modal). Using this esc key is the only way to dismiss the modal.

The only thing that has changed is the upgrades for react and react-bootstrap, so I'm wondering if there is another step I missed?

Thanks!

const ExampleModal = ({
    isShownModal,
    onHide,
}) => (
    <Modal show={isShownModal} onHide={onHide}>
        <Modal.Header closeButton>
            <Modal.Title>Example Title</Modal.Title>
        </Modal.Header>

        <Modal.Body>
            <h5>Example body content</h5>
        </Modal.Body>

        <Modal.Footer>
            <Button onClick={onHide}>Close</Button>
        </Modal.Footer>
    </Modal>
)

export default class ExampleModal extends React.Component {    
    constructor(props) {
        super(props)
        this.state = {isShownModal: false}
    }

    openModal = () => {this.setState({ isShownModal: true })}
    closeModal = () => {this.setState({ isShownModal: false })}

    render() {
        return (
            <span onClick={this.openModal}>
                <i className="fa fa-question-circle mr-3"></i>
                <ExampleModal 
                    isShownModal={this.state.isShownModal} 
                    onHide={this.closeModal}
                />
            </span>
        )
    }
}
@microdou

This comment has been minimized.

Show comment
Hide comment
@microdou

microdou Oct 23, 2017

@jahlela
I got the same issue.

It appears that upon React 16, a click on Modal's close sign (or a click outside Modal) that should have triggered this.closeModal only, now triggers not only this.closeModal in Modal component, but also the this.openModal in the outer wrapper of Modal.

I temporarily fixed it by moving Modal out of its wrapper by returning an array of both the wrapper/trigger and the Modal instead. You may try something as follows:

render() {
  return [
    <span key="trigger" onClick={this.openModal}>
      <i className="fa fa-question-circle mr-3"></i>
    </span>,
    <ExampleModal
      key="modal"
      isShownModal={this.state.isShownModal} 
      onHide={this.closeModal}
    />,
  ];
}

I suppose there should be some fix to Modal to reverse the incompatibility.

microdou commented Oct 23, 2017

@jahlela
I got the same issue.

It appears that upon React 16, a click on Modal's close sign (or a click outside Modal) that should have triggered this.closeModal only, now triggers not only this.closeModal in Modal component, but also the this.openModal in the outer wrapper of Modal.

I temporarily fixed it by moving Modal out of its wrapper by returning an array of both the wrapper/trigger and the Modal instead. You may try something as follows:

render() {
  return [
    <span key="trigger" onClick={this.openModal}>
      <i className="fa fa-question-circle mr-3"></i>
    </span>,
    <ExampleModal
      key="modal"
      isShownModal={this.state.isShownModal} 
      onHide={this.closeModal}
    />,
  ];
}

I suppose there should be some fix to Modal to reverse the incompatibility.

@jahlela

This comment has been minimized.

Show comment
Hide comment
@jahlela

jahlela Oct 24, 2017

@microdou Worked like a charm! I'll count that as our first array return implemented with React 16 :)

jahlela commented Oct 24, 2017

@microdou Worked like a charm! I'll count that as our first array return implemented with React 16 :)

@zxzl

This comment has been minimized.

Show comment
Hide comment
@zxzl

zxzl Dec 7, 2017

@microdou You saved me !!

zxzl commented Dec 7, 2017

@microdou You saved me !!

@StuartMartinho

This comment has been minimized.

Show comment
Hide comment
@StuartMartinho

StuartMartinho Sep 7, 2018

@microdou solution didn't work for me... any help?
thanks!

StuartMartinho commented Sep 7, 2018

@microdou solution didn't work for me... any help?
thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment