search

mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
94.02k stars 32.3k forks source link

[test] Do not enforce the presence of `ownerState.className` in `describeConformance` #44479

Open flaviendelangle opened 1 day ago

flaviendelangle commented 1 day ago

In @mui/x-date-pickers and @mui/x-date-pickers-pro, we are updating all our ownerState (both the one used in slotProps and the one used in styled() or styleOverrides to be a small object shared across all the slots of the library.

The idea behind this change is that, from a user point of view, it is unclear which part of this UI resolves those slots since the component are very large. So the props used for the ownerState is hardly predictable and does not contain the same information from one slot to another, often missing key information of the current state of the picker.

I encountered a small problem because describeConformance uses ownerState.className which is no longer present in our components for some tests. Do you see a problem hardcoding a value instead?

mui-bot commented 1 day ago

Netlify deploy preview

https://deploy-preview-44479--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad) No bundle size changes

Generated by :no_entry_sign: dangerJS against 7f9ef65e8114ec96f789a919c5998806388c8356

michaldudak commented 1 day ago

This changes the meaning of this test entirely. The purpose of it is to test if the ownerState in callback has the props passed to the component. Since your components work differently, IMO skipping this test would make more sense.

flaviendelangle commented 1 day ago

We should update the name of the test then, IMHO it does not describe what you are saying. It would be great if we could have 2 tests then, one that tests the props and what that only test the firing of the callback. That way we can skip the one that tests the props without loosing what is interesting to us (the firing of the callback).

michaldudak commented 1 day ago

Yup, 2 different tests are a good option as well.

flaviendelangle commented 17 hours ago

I added the 2nd test :+1: And the CI is passing