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

[StepIcon] Fix text centering when changing browser font size #32706

Merged
merged 4 commits into from Jun 8, 2022

Conversation

alansouzati
Copy link
Contributor

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

  1. Go to your browser and increase your font size (e.g using Very Large on Chrome)

Before

Text is NOT centered

Screen Shot 2022-05-09 at 12 23 04 PM

After

Text is centered

Screen Shot 2022-05-09 at 12 22 40 PM

@mui-bot
Copy link

mui-bot commented May 9, 2022

Details of bundle changes

Generated by 🚫 dangerJS against e3c2aeb

@danilo-leal danilo-leal added the component: stepper This is the name of the generic UI component, not the React module! label May 10, 2022
@oliviertassinari oliviertassinari added the bug 🐛 Something doesn't work label May 18, 2022
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.

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.

@alansouzati
Copy link
Contributor Author

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).

@alansouzati
Copy link
Contributor Author

Nevermind, It looks like you are using Playwright to take screenshots

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label May 18, 2022
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label May 18, 2022
@alansouzati
Copy link
Contributor Author

This should be ready to go

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.

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 :)

@mnajdova mnajdova merged commit 9b19d91 into mui:master Jun 8, 2022
@alansouzati
Copy link
Contributor Author

you bet looking forward to more contributions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: stepper 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.

None yet

5 participants