Patternfly-react: error after upgrading react-bootstrap - CustomModalDialog is out of date

[laviro]

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)

and the warning:
Warning: Unknown event handler property%s. It will be ignored.%s
maybe it is something about a naming convention, see: https://github.com/styled-components/styled-components/issues/2218#issuecomment-489661463

opened an issue to react-bootstrap - https://github.com/react-bootstrap/react-bootstrap/issues/4872

[rachael-phillips]

Hi @laviro our team has seen this, and we will be taking a look at it this week.

[martinpovolny]

FYI: ManageIQ and V2V also blocked on this one: https://github.com/ManageIQ/manageiq-v2v/issues/1037

[mturley]

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]

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 [email protected] 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 ModalDialog here. 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.

Searching patternfly-react for dialogComponentClass reveals that we pass our own CustomModalDialog into 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-bs ModalDialog as our own CustomModalDialog with some tweaks.

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.

To fix this, we simply need to update our CustomModalDialog to replicate this change react-bootstrap made to their original ModalDialog: https://github.com/react-bootstrap/react-bootstrap/commit/87a9a97f8670f3a02436f8f520caf36f88e4bdab#diff-e62f630dfbeeba1084a3724f9d4dbf98 and pull the onMouseDownDialog prop out and apply it to the div as onMouseDown.

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]

I'll draft a PR to fix this next week. Cheers 🍻

[laviro]

Thanks @mturley :1st_place_medal:

[mturley]

https://github.com/patternfly/patternfly-react/pull/3409