[Table] Migrate TablePagination to emotion

[siriwatknp]

• I have followed (at least) the PR section of the contributing guide.

One of #24405

[mui-pr-bot]

Details of bundle changes

@material-ui/core: parsed: +0.33% , gzip: +0.28%

Generated by 🚫 dangerJS against abf563b

[siriwatknp]

Styling is a bit ugly, is this the expected result? (v4 implementation does not work, I think it is because emotion merge the styles, so I need to add breakpoints to overrides)

const TablePaginationToolbar = experimentalStyled(
  Toolbar,
  { shouldForwardProp: (prop) => shouldForwardProp(prop) || prop === 'classes' },
  {
    name: 'MuiTablePagination',
    slot: 'Toolbar',
    overridesResolver: makeOverridesResolver('toolbar'),
  },
)(({ theme }) => ({
  minHeight: 52,
  paddingRight: 2,
  [`${theme.breakpoints.up('xs')} and (orientation: landscape)`]: {
    minHeight: 52,
  },
  [theme.breakpoints.up('sm')]: {
    minHeight: 52,
  },
}));

[oliviertassinari]

@siriwatknp use the as prop

[siriwatknp]

@siriwatknp use the as prop

So, the demo should change from using component="div" to as="div" right? If yes, do I need to add types to .d.ts?

[siriwatknp]

Open another issue to fix className overwritten #25814

[oliviertassinari]

@siriwatknp I have pushed extra commits, as small as I could, please review. I have tried to make so the changes have as few breaking changes as possible.

@mnajdova The most interesting aspect seems the overridesResolver structure. I like how it's more isolated, how the CSS specificity is flatter, and how it enables having a default behavior (automatically allowing a slot in the theme from the name of the styled slot). Is there a reason for not doing this in the whole codebase (e.g. performance)?

[siriwatknp]

Looks good to me 😊

[mnajdova]

@mnajdova The most interesting aspect seems the overridesResolver structure. I like how it's more isolated, how the CSS specificity is flatter, and how it enables having a default behavior (automatically allowing a slot in the theme from the name of the styled slot). Is there a reason for not doing this in the whole codebase (e.g. performance)?

I didn't want to push forward with this strictly because of perf, we would need for each slot to resolver theme overrides and merge... It is useful for containers or other non-child slots, but I wouldn't add it everywhere... I remember we had this discussion when we were comparing one styled() component in the root, or multiple styled components, this would be similar..

Anyway, it would be interesting to measure how big this difference would be now that we have some actual components migrated. Not sure what the benefit would be, as developers are either going to use theme for overrides, or use styled() for wrapping slots, so both of those independently work. If they want to mix these two, than using overrides resolver on each slot would help

[mnajdova]

@siriwatknp @oliviertassinari we should not however add this new overridesResolver on each slot paradigm in just few components before we measure how expensive it is and describing what issues it will solve. We should have a unified way of defining styles & overrides. Especially that we have few components left to be migrated.

@siriwatknp if you would like to create RFC with the proposed changes on one component with details on what problems the new definition of the styles solve and measure the perf difference it would be great.
Created #25850 seems like we can go with this option :).

Edit: One thing it would help us with is #25075 (comment), but again not sure what the cost would be.

Seems like in the end we will go with overridesResolver per slot. No changes needed.

[mnajdova]

Where are these styles coming from?

[siriwatknp]

same as above, to override Toolbar styles

[oliviertassinari]

Do we need to have a Toolbar in the first place? It seems odd in hindsight.

[siriwatknp]

I think let's keep it because the PR is only about migration.

[mnajdova]

Nice! Sorry it took me some time to wrap up the review