You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
error after upgrading react-bootstrap - CustomModalDialog is out of date #3364
After one of the latest versions update, with react-bootstrap update, maybe #3249,
my environment is with the latest react and react-dom and my tests started to fail with the following error:
console.error node_modules/jest-prop-type-error/index.js:8
onMouseDownDialog
in div (created by CustomModalDialog)
in CustomModalDialog (created by Modal)
in Transition (created by Fade)
in Fade (created by DialogTransition)
in DialogTransition (created by Modal)
in RefHolder (created by Modal)
in div (created by Modal)
in Portal (created by Modal)
in Modal (created by Modal)
in Modal (created by DiffModal)
I was able to reproduce this in v2v, but for some reason I can't reproduce it with the Modal examples in patternfly-react's storybook, which I think should be producing the same error. I'll keep investigating on Monday.
Ok, I think I figured out what's going on here. This is the version of the error I'm seeing in v2v:
Warning: Unknown event handler property `onMouseDownDialog`. It will be ignored.
in div (created by CustomModalDialog)
in CustomModalDialog (created by Modal)
in Transition (created by Fade)
in Fade (created by DialogTransition)
in DialogTransition (created by Modal)
in RefHolder (created by Modal)
in div (created by Modal)
in Portal (created by Modal)
in Modal (created by Modal)
[truncated]
It looks like the problem is that an onMouseDownDialog prop is being applied to a plain <div> element, causing a warning since this is not a valid prop for a <div>. I can only find one place in react-bootstrap@0.33.0 where this prop is passed, and it appears to be coming from this line: https://github.com/react-bootstrap/react-bootstrap/blob/bs3-dev/src/Modal.js#L287
That <Dialog> the prop is being passed to there is actually defined by the dialogComponentClass prop here, which defaults to ModalDialoghere. If passed to ModalDialog as is the default behavior, it's handled correctly and won't be applied to a <div>.
So this means that somewhere, a <Modal> is being rendered with dialogComponentClass set to either a div or something that passes all its props to a div. I can't find anywhere in the react-bootstrap code where that is happening, so I started thinking it might be our usage in patternfly-react.
So, the bug is happening because CustomModalDialog ends up including the unexpected onMouseDownDialog prop in its elementProps object, which it then spreads directly onto the <div>here.
While we're at it, we should probably just update that whole file to match the new react-bootstrap version in case anything else changed, and preserve whatever it was that we modified to be custom. Maybe we can even just stop using that custom component. We should look for others like it that we copy-pasted... we may have caused other issues by essentially upgrading react-bootstrap while overriding bits of it with its old code.
changed the title [-]error after upgrading react-bootstrap[/-][+]error after upgrading react-bootstrap - CustomModalDialog is out of date[/+]on Dec 13, 2019
Activity
rachael-phillips commentedon Dec 2, 2019
Hi @LaViro our team has seen this, and we will be taking a look at it this week.
martinpovolny commentedon Dec 4, 2019
FYI: ManageIQ and V2V also blocked on this one: ManageIQ/manageiq-v2v#1037
mturley commentedon Dec 13, 2019
I was able to reproduce this in v2v, but for some reason I can't reproduce it with the Modal examples in patternfly-react's storybook, which I think should be producing the same error. I'll keep investigating on Monday.
mturley commentedon Dec 13, 2019
Ok, I think I figured out what's going on here. This is the version of the error I'm seeing in v2v:
It looks like the problem is that an
onMouseDownDialogprop is being applied to a plain<div>element, causing a warning since this is not a valid prop for a<div>. I can only find one place in react-bootstrap@0.33.0 where this prop is passed, and it appears to be coming from this line: https://github.com/react-bootstrap/react-bootstrap/blob/bs3-dev/src/Modal.js#L287That
<Dialog>the prop is being passed to there is actually defined by thedialogComponentClassprop here, which defaults toModalDialoghere. If passed toModalDialogas is the default behavior, it's handled correctly and won't be applied to a<div>.So this means that somewhere, a
<Modal>is being rendered withdialogComponentClassset to either a div or something that passes all its props to a div. I can't find anywhere in the react-bootstrap code where that is happening, so I started thinking it might be our usage in patternfly-react.Searching patternfly-react for
dialogComponentClassreveals that we pass our ownCustomModalDialoginto this prop in our wrapper for react-bootstrap's Modal: https://github.com/patternfly/patternfly-react/blob/master/packages/patternfly-3/patternfly-react/src/components/Modal/Modal.js#L23. It turns out, we made a copy of the original r-bsModalDialogas our own CustomModalDialog with some tweaks.So, the bug is happening because
CustomModalDialogends up including the unexpectedonMouseDownDialogprop in itselementPropsobject, which it then spreads directly onto the<div>here.To fix this, we simply need to update our
CustomModalDialogto replicate this change react-bootstrap made to their original ModalDialog: react-bootstrap/react-bootstrap@87a9a97#diff-e62f630dfbeeba1084a3724f9d4dbf98 and pull theonMouseDownDialogprop out and apply it to the div asonMouseDown.While we're at it, we should probably just update that whole file to match the new react-bootstrap version in case anything else changed, and preserve whatever it was that we modified to be custom. Maybe we can even just stop using that custom component. We should look for others like it that we copy-pasted... we may have caused other issues by essentially upgrading react-bootstrap while overriding bits of it with its old code.
mturley commentedon Dec 13, 2019
I'll draft a PR to fix this next week. Cheers 🍻
[-]error after upgrading react-bootstrap[/-][+]error after upgrading react-bootstrap - CustomModalDialog is out of date[/+]Ron-Lavi commentedon Dec 15, 2019
Thanks @mturley 🥇
mturley commentedon Dec 16, 2019
#3409