Skip to content

error after upgrading react-bootstrap - CustomModalDialog is out of date #3364

Closed

Description

@Ron-Lavi

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: styled-components/styled-components#2218 (comment)

opened an issue to react-bootstrap - react-bootstrap/react-bootstrap#4872

Activity

  1. rachael-phillips commented on Dec 2, 2019

    @rachael-phillips
    Contributor

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

  2. added this to the Prioritized Backlog milestone on Dec 3, 2019
  3. martinpovolny commented on Dec 4, 2019

    @martinpovolny
    Contributor

    FYI: ManageIQ and V2V also blocked on this one: ManageIQ/manageiq-v2v#1037

  4. mturley commented on Dec 13, 2019

    @mturley
    Contributor

    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.

  5. mturley commented on Dec 13, 2019

    @mturley
    Contributor

    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 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: react-bootstrap/react-bootstrap@87a9a97#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.

  6. mturley commented on Dec 13, 2019

    @mturley
    Contributor

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

  7. changed the title [-]error after upgrading react-bootstrap[/-] [+]error after upgrading react-bootstrap - CustomModalDialog is out of date[/+] on Dec 13, 2019
  8. Ron-Lavi commented on Dec 15, 2019

    @Ron-Lavi
    CollaboratorAuthor

    Thanks @mturley 🥇

  9. mturley commented on Dec 16, 2019

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions