Emotion fix select

[garronej]

Hello @mnajdova,

Sorry to bother you again, this is yet another following up to 33205.
It was reported to me that the <Select /> component hadn't been fixed by my previous PR.

Best regards,

[garronej]

In this component there is no root class. select is expected to be used instead.

[mui-bot]

Details of bundle changes

Generated by 🚫 dangerJS against 922f1ee

[mnajdova]

Hey, sorry for the delay, it missed my radar 😁 This class haven't been handled before in this file, so we may be doing something wrong here. Previously this was handled here: https://github.com/mui/material-ui/blob/master/packages/mui-material/src/Select/SelectInput.js#L521 If we change the behavior, we likely need to remove the classes.select from the SelectInput logic no?

[garronej]

Hey, sorry for the delay, it missed my radar 😁

No need to apoogies @mnajdova , thank you for reveiwing!

This case is no different from every other case, I applied the same fix.
It's just that in the <Select /> component there's no root property in classes but a select instead.
I don't know why it is so. Do you know? I'm I missing something big?

Previously this was handled here: https://github.com/mui/material-ui/blob/master/packages/mui-material/src/Select/SelectInput.js#L521 If we change the behavior, we likely need to remove the classes.select from the SelectInput logic no?

I got you. I was worried by this as well.
Like for example classes.root beein applied to <Button /> but still passed to the underling <ButtonRoo />
I even patched it it my initial PR:

Only to realize that it whas done like that everywhere where there is a sub component (take for example <Checkbox /> and <CheckboxRoot /> for as another example ) so I rolled back my change and just conformed to the pattern:

I hope I'm not missing the point.

Have a good day 😊

[garronej]

A little up on this :)
Let me know if you expect something more.

[mnajdova]

I apologize for the delay. The problem with this PR is that the classes.select is now being handled two times (by two different components) and applied on two different slots. See https://codesandbox.io/s/damp-cookies-3t3wrz?file=/src/App.tsx

This is why I proposed for this to be handled in the NativeSelectInput and SelectInput components if possible. If this is not possible, we should remove the logic in those components before handling it. In my opinion this class should be applied here:

material-ui/packages/mui-material/src/Select/Select.js

Line 113 in 922f1ee

classes: inputProps ? deepmerge(classes, inputProps.classes) : classes,

We could try if this would solve the issue.

[garronej]

@mnajdova, thank you for coming back to me on this.
I have to invesigate futher.
To be continued.

[ZeeshanTamboli]

@garronej May I inquire if there are any planned updates regarding this PR?

[garronej]

Thank you for bringing this to my attention. I've yet to address @mnajdova's last message as it remains in my backlog, and I haven't had the opportunity to investigate it.

However, I put a great deal of care into crafting this PR. I believe I had valid reasons for the choices I made.