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

[docs] Migrate onepirate premium template to emotion part1 #26671

Merged
merged 2 commits into from
Jun 12, 2021

Conversation

vicasas
Copy link
Member

@vicasas vicasas commented Jun 9, 2021

This is a first part of the migration

One of #16947

https://deploy-preview-26671--material-ui.netlify.app/premium-themes/onepirate/

@vicasas vicasas added the docs Improvements or additions to the documentation label Jun 9, 2021
@mui-pr-bot
Copy link

mui-pr-bot commented Jun 9, 2021

No bundle size changes (experimental)

Generated by 🚫 dangerJS against 5edb27d

marginTop: theme.spacing(8),
},
} as const);
const item: SxProps<Theme> = {
Copy link
Member Author

Choose a reason for hiding this comment

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

SxProps<Theme> Sometimes it is necessary and sometimes it is not. I feel confused why this is happening.

Copy link
Member

Choose a reason for hiding this comment

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

Interesting, if fails with flexDirection: 'column',.

Copy link
Member

Choose a reason for hiding this comment

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

I'll create an issue, so that I don't forget to look into it.

},
sizeSmall: {
...(size === 'small' && {
Copy link
Member Author

Choose a reason for hiding this comment

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

While this works ... when inspecting the element the class that it will always assign is MuiButton-sizeMedium. It causes me a little noise, shouldn't it be something else?

The same happens for large

Copy link
Member

Choose a reason for hiding this comment

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

Fixed in vicasas#1, if you merge it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @oliviertassinari. Should we stop using this? shouldForwardProp: (prop) => prop! == 'size', why did I understand this was to not pass the property to the DOM element? They correct me if I'm wrong.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops! I just saw that size is a Button prop 😅

marginTop: theme.spacing(8),
},
} as const);
const item: SxProps<Theme> = {
Copy link
Member

Choose a reason for hiding this comment

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

Interesting, if fails with flexDirection: 'column',.

* better to forward the size

* add context
@mnajdova mnajdova merged commit 8f7b501 into mui:next Jun 12, 2021
@vicasas vicasas deleted the feature/migrate-template-onepirate-part1 branch June 12, 2021 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants