[Popover] ModalClasses prop regression

[jeffshaver]

On Popover, we were using the ModalClasses prop to pass a class to the div that gets mounted for the Modal.

This is broken now. I am not 100% sure on when this regression occurred (maybe on the functional rewrite?).

The problem (I think) is that Modal doesn't pull off classes as a known prop, it just gets spread using {...other}. Which means the div gets an attribute called classes and its value is [Object object]. I think the solution is to have modal pull of classes and as a prop.

Another alternative would be to give Popover a ModalProps prop instead of ModalClasses and then spread that onto the Modal component.

Second solution seems more flexible.

• [ x ] This is not a v0.x issue.
• [ x ] I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 🤔

The div that Modal creates should have the correct classnames.

Current Behavior 😯

The div that Modal creates gets a classes="[Object object] attribute

Steps to Reproduce 🕹

Link: https://codesandbox.io/s/create-react-app-with-typescript-787wb

1. All you have to do is inspect the popover on the demo and see that the attribute is created wrong

Context 🔦

I am trying to pass a class name to the div that the Modal component creates

Your Environment 🌎

Tech	Version
Material-UI	v4.1.2
React	16.8.19
Browser	Chrome 74.0.3729.169
TypeScript	3.5.2

[oliviertassinari]

@jeffshaver Your issue is very similar to #16442. The classes API was removed in #15466.

You can use the class name API directly.

-<Popover open={true} ModalClasses={{ root: classes.test }} />
+<Popover open={true} className={{classes.test}} />

Does it solve your issue?

[francoiscabrol]

@oliviertassinari it solved the issue for me but I had to add an important in the css to override the z-index.

[oliviertassinari]

@francoiscabrol Right, same issue as #16442.

[jedwards1211]

I guess this wasn't released as a breaking change because the new React warnings are just warnings?

[jedwards1211]

AFAICT I'm going to have to have a conditional on the version number of @material-ui/core to support all versions of v4 in material-ui-popup-state now without React warnings...kinda not cool

[oliviertassinari]

I would recommend dropping v3 support.

[jedwards1211]

Okay, I was getting confused because somehow no warnings were showing about the ModalClasses prop getting passed to a <div> in @material-ui/core/4.0.2. FWIW it's pretty surprising and confusing that classes passed to Popover affects the Popover, but the className prop goes through to the Modal.

[jedwards1211]

@oliviertassinari I was thinking I would have to do different things for different sub-versions of v4, but fortunately I was confused, sorry.

[oliviertassinari]

Yeah, you are right classes doesn't inherit, while className does.