-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[StepIcon] Fix text centering when changing browser font size #32706
Conversation
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.
I could verify that it works 👍 Let's add a regression test to ensure that it won't break in the future. You can follow the examples in https://github.com/mui/material-ui/tree/master/test/regressions/fixtures You can use the GlobalStyles
component for injecting: html { font-size: 24px; }
as an example.
Great! thanks for your review. Is there documentation on how to write tests for this? I don't fully understand how these fixtures are being tested (I see no assertions). |
Nevermind, It looks like you are using Playwright to take screenshots |
7c4d5cf
to
6022d11
Compare
6022d11
to
e135e69
Compare
e135e69
to
854d664
Compare
This should be ready to go |
854d664
to
ed8311f
Compare
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.
I've updated to latest master, we can merge after the CI is green. Thanks for the contribution, and once again nice write up on how you use MUI at your company :)
you bet looking forward to more contributions |
What I did
Changed the StepIcon in a backwards compatible way to fix the centering of the text if you change your browser font size
What you need to test
Before
Text is NOT centered
After
Text is centered