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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[a11y] Add first class support for REM #32703

Open
2 tasks done
alansouzati opened this issue May 9, 2022 · 3 comments
Open
2 tasks done

[a11y] Add first class support for REM #32703

alansouzati opened this issue May 9, 2022 · 3 comments
Labels
accessibility a11y new feature New feature or request

Comments

@alansouzati
Copy link
Contributor

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Summary 馃挕

As of today MUI does a terrific job with Accessibility, including default REM support for Typography.

This feature request intends to raise the issue with limiting REM support to just Typography and Iconography.

For example, using default MUI I go to my browser settings and I change the font size to "Very Large" in Chrome and here is how the Stepper component looks like:

Screen Shot 2022-05-09 at 11 43 26 AM

Notice that just changing the font size in the Browser makes MUI change the font size of the stepper text and now things are not centered

Examples 馃寛

Button

Here is what I've done to change the button defaults to use rems

https://codesandbox.io/s/mui-rem-overrides-0cntz9

Stepper

This one seems more complicated to fix given that the centering of the SVG text is done in Javascript I believe

https://codesandbox.io/s/mui-rem-overrides-stepper-bl1f0s?file=/index.js

Motivation 馃敠

I intend to use MUI out of the box and have REM support across the board where changing the Browser font size will not result in default MUI components looking weird and not aligned well

@alansouzati alansouzati added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label May 9, 2022
@alansouzati
Copy link
Contributor Author

For the Stepper I think this fixes the issue:

#32706

But, the feature request here is more on a higher level to make it consistent REM usage throughout MUI

@danilo-leal danilo-leal changed the title Accessibility: first class support for REM [a11y] Add first class support for REM May 10, 2022
@danilo-leal danilo-leal added accessibility a11y new feature New feature or request labels May 10, 2022
@oliviertassinari oliviertassinari removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label May 18, 2022
@oliviertassinari
Copy link
Member

https://www.joshwcomeau.com/css/surprising-truth-about-pixels-and-accessibility/ looks about right when it comes to choosing between rem and px.

@alansouzati
Copy link
Contributor Author

Oh yeah. I just finished reading this (thanks for sharing by the way)!

I agree with everything this article has shared, especially the part about using your intuition.

An example here is LinkedIn at 200% zoom:

Screen Shot 2022-05-18 at 9 47 23 AM

Pretty slick.

Now check out the same app increasing the browser font size to "very large":

Screen Shot 2022-05-18 at 9 49 14 AM

Things are not looking as good. They are limiting REM support to Typography. So things are getting cropped and content is overlapping (look at the LinkedIn app bar).

This is where I see an "Accessibility" issue given that this is harder to read and use.

When trying to decide to use Pixel or REM, it is also important to test things out in the real world and see what it feels right.

When it comes to Material UI, I think there are things to be improved in this area (I'm down to help send PRs to fix them up). I wanted to sense how open the maintainers are to exploring changing things in that space.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility a11y new feature New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants