Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Table] Migrate TableBody to emotion #24703

Merged
merged 2 commits into from
Feb 7, 2021
Merged

Conversation

natac13
Copy link
Contributor

@natac13 natac13 commented Jan 30, 2021

#24405

@mui-pr-bot
Copy link

mui-pr-bot commented Jan 30, 2021

@material-ui/core: parsed: +0.08% , gzip: +0.06%

Details of bundle changes

Generated by 🚫 dangerJS against 424efe6

@oliviertassinari oliviertassinari added the component: table This is the name of the generic UI component, not the React module! label Feb 2, 2021
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

@mnajdova mnajdova merged commit a91bf00 into mui:next Feb 7, 2021
@@ -0,0 +1,7 @@
import { TableBodyClassKey } from './TableBody';

declare const tableBodyClasses: Record<TableBodyClassKey, string>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that it will be the way forward, at least, we haven't yet decided with #24736 (comment).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah apologizes, I've missed that conversation, I thought the PR was merged. In my opinion, it makes sense that we use the props.classes the most, as far as I remember that was the first proposal in the PR. I don't mind if we use the ComponentClassesKeys, but we will have the pseudo keys there as well, which may be misleading, as the ComponentClassesKeys are used for the stylesOverrides in the theme. I'd say we shouldn't block the open migration PRs because of this anyway, as we will anyway need to update the classes in all components once we decide on the best option we will have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: table This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants