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-infra] Improve demos toolbar #37762

Merged
merged 2 commits into from
Jun 30, 2023

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Jun 30, 2023

I noticed small bugs with the demo, so I created a playground to debug and fixed a few instances.

Preview: https://deploy-preview-37762--material-ui.netlify.app/experiments/docs/demos/

@oliviertassinari oliviertassinari added design This is about UI or UX design, please involve a designer scope: docs-infra Specific to the docs-infra product labels Jun 30, 2023
@mui-bot
Copy link

mui-bot commented Jun 30, 2023

Netlify deploy preview

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

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 6d9bb2a

{
display: 'none',
border: `1px solid ${(theme.vars || theme).palette.divider}`,
marginTop: -1,
marginTop: demoOptions.bg === 'inline' ? 8 : -1,
Copy link
Member Author

Choose a reason for hiding this comment

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

The negative margin when the bg is inline is strange, see https://mui.com/x/react-data-grid/#mit-version and https://mui.com/x/react-date-pickers/shortcuts/#customization as two examples.

Comment on lines -285 to -287
...(hiddenToolbar && {
paddingTop: theme.spacing(2),
}),
Copy link
Member Author

Choose a reason for hiding this comment

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

This style seem irrelevant

@@ -233,7 +230,7 @@ const DemoRootMaterial = styled('div', {
}),
/* Prepare the background to display an inner elevation. */
...(bg === true && {
padding: theme.spacing(4),
padding: theme.spacing(3),
Copy link
Member Author

Choose a reason for hiding this comment

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

Match Joy UI

@@ -210,7 +210,7 @@ const DemoRootMaterial = styled('div', {
display: 'flex',
justifyContent: 'center',
[theme.breakpoints.up('sm')]: {
borderRadius: '12px 12px 0 0',
borderRadius: hiddenToolbar ? 12 : '12px 12px 0 0',
Copy link
Member Author

Choose a reason for hiding this comment

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

Fix border-radius at the footer of https://mui.com/material-ui/react-grid/#interactive

Comment on lines -222 to -224
...(hiddenToolbar && {
paddingTop: theme.spacing(1),
}),
Copy link
Member Author

Choose a reason for hiding this comment

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

This style seem irrelevant

Copy link
Contributor

@danilo-leal danilo-leal left a comment

Choose a reason for hiding this comment

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

Sweet, good one, thanks for the tips left in the comments & for adding all variants in the experiments docs, super useful! Also pushed a minor tweak to the bg: true demo.

@danilo-leal danilo-leal merged commit 4887bd5 into mui:master Jun 30, 2023
18 checks passed
@oliviertassinari oliviertassinari deleted the polish-display-demo branch June 30, 2023 14:37
@oliviertassinari
Copy link
Member Author

@danilo-leal 👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design This is about UI or UX design, please involve a designer scope: docs-infra Specific to the docs-infra product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants