-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[test][docs] Improve demos for better regression screenshots #43742
Conversation
Netlify deploy previewhttps://deploy-preview-43742--material-ui.netlify.app/ Bundle size report |
This one is not fixed in this PR: https://app.argos-ci.com/mui/material-ui/builds/32051/108773834. It was broken before #43656 and it looks like a CSS order issue. See attached screenshots and notice the style order in the dev tools. |
@@ -19,6 +19,7 @@ export default function ExampleAlignmentButtons() { | |||
onChange={(event: React.ChangeEvent<HTMLInputElement>) => | |||
setAlignment(event.target.value) | |||
} | |||
sx={{ display: 'inline-flex' }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not ideal to override the display
property of the RadioGroup
component in our own demos (RadioGroup
uses flex
). In hindsight, RadioGroup
should've probably used inline-flex
from the start.
Another option is to wrap the entire demo in an inline-block/flex
element, but that probably adds more noise to the demo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps something we can introduce as a breaking change in a future major?
7bf38e6
to
1ca843b
Compare
The 3 demos being updated look good in Argos:
There are 2 other unrelated screenshots that changed:
|
Taking a look at those in |
In that case I think we can move forward with this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👌
Nice 👍 |
Follow up on #43656 (comment)
Some demos were using
flex
instead ofinline-flex
in their top-level elements, causing them to look off when rendered inside ablock
element instead of aflex
element. For context, our demo wrapper usesflex
but our visual regression wrapper usesblock
since #43656.Before: https://deploy-preview-43740--material-ui.netlify.app/material-ui/react-divider/#flex-item
After: https://deploy-preview-43742--material-ui.netlify.app/material-ui/react-divider/#flex-item