-
-
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
[core] Accessibility - increase default contrastThreshold for WCAG AA compliance #34901
Conversation
|
Argo CI failures are expected (improving contrast) |
Wow, these results are super impressive from just a few files changed. This is great, looking forward to seeing this one merged. |
Thanks for the comprehensive description! This is not something we often see :) The changes look good, however, some people may consider them a breaking change, especially if some of the components that depend on them have additional style overrides. I would personally merge the changes as they are improving the accessibility of the components, but I am tagging @michaldudak and @siriwatknp for a second opinion. |
Thanks for bringing this topic in! To me, the white text is much more readable than the black one. So, why do the a11y tools report this as a problem? WCAG 2.1 uses a color contrast algorithm that has flaws. W3C is considering replacing it with another algorithm, APCA, in WCAG 3.0 (https://colorjs.io/docs/contrast.html#accessible-perceptual-contrast-algorithm-apca). I found a good summary of cases where APCA gives better results: Myndex/SAPC-APCA#30 I checked several of the color combinations changed in this PR, and they scored worse according to APCA (and my subjective view). I don't think we should optimize for tools (especially if they are imperfect) but for people. |
Hey @michaldudak, thank you so much for your reply. I see what you are saying about the imperfectness of the current WCAG 2.1 standards. Sadly I think the WCAG 3 standards are at least several years out, as we have not even seen the finalization of the 2.2 standards yet. But, all that being said, again I am not trying to just optimize around a tool. I could easily change the contrast evaluation method to APCA and set the minimum value to 60. |
@michaldudak it is true that the APCA formula is more perceptually accurate. However, APCA is still not finalized, and WCAG 3 is still in progress as well. I would recommend adhering to current standards, especially with a library like this. |
I propose a simple solution to this, we can provide a copy-paste snippet in the docs for the theme to enhance the contrast ratio so that anyone who strictly follows the contrast ratio can opt-in. This workaround should make everyone happy! For those who rely on the standard WCAG 2.1, those issues are fixed. |
So @siriwatknp the end recommendation would be to keep it currently failing compliance to WCAG 2.1, and provide documentation on how to meet WCAG 2.1 standards within one's own config? |
@siriwatknp @michaldudak @mnajdova I have updated the PR to be non-breaking, and just adding documentation. |
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.
This looks much better to me. I am tagging @samuelsycamore for reviewing the docs.
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.
@michaldudak it is true that the APCA formula is more perceptually accurate. However, APCA is still not finalized, and WCAG 3 is still in progress as well. I would recommend adhering to current standards, especially with a library like this.
That's true. But leaving standards aside for a moment, do you think users prefer reading texts that look like this?
I don't mean to say that APCA is the silver bullet, but it's pretty clear that the WCAG formula is flawed.
Fortunately, there is a solution that should make everyone happy. Thanks, @siriwatknp for the suggestion!
@kennethbigler Could you please rebase on the latest master? This should solve the argos errors. |
Co-authored-by: Michał Dudak <michal.dudak@gmail.com> Signed-off-by: Ken Bigler <kennethbigler@gmail.com>
a2e9872
to
b683333
Compare
@michaldudak can we merge this now? |
Yep, we will merge it today. |
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.
👍 Thanks! pushed some changes to shorten the sentence.
… compliance (mui#34901) Signed-off-by: Ken Bigler <kennethbigler@gmail.com> Co-authored-by: Ken Bigler <ken_bigler@intuit.com> Co-authored-by: Michał Dudak <michal.dudak@gmail.com> Co-authored-by: siriwatknp <siriwatkunaporn@gmail.com>
… compliance (mui#34901) Signed-off-by: Ken Bigler <kennethbigler@gmail.com> Co-authored-by: Ken Bigler <ken_bigler@intuit.com> Co-authored-by: Michał Dudak <michal.dudak@gmail.com> Co-authored-by: siriwatknp <siriwatkunaporn@gmail.com>
Updating documentation to guide people towards WCAG 2.1 AA compliance, but removed any actual code changes. See below description and comments for context.
Below was for original PR and context, but this has all been reverted
Hello!
My name is Ken Bigler and I am the Accessibility Engineering Leader at Intuit.
I work on the Intuit Design System (internal to Intuit), and I am a big fan of your products.
Anyway, to the point of this PR:
TDLR - the default contrastThreshold needs to be increased.
Ok, now let me go into why.
I use MUI for my personal website, and when I was adding lighthouse tests, I noticed that I started getting a lot of color contrast errors. I know that MUI has wonderful accessibility, but then I read your accessibility page and it sounds like you wanted contribution from experts, so I thought I could step in.
Verify the problem
The easiest way to do this is to run lighthouse or Axe on your documentation website, and you will see:
Oh no! Ok, we have some contrast issues.
Well, not exactly. That is only for large text, for smaller text (which is all over not only your documentation site, but the entire internet), we need a ratio of 4.5:1.
Fixing the problem
Why yes it did! See the results!
Why not fix everything?
Well, great question, in an effort to keep this PR small, I wanted to keep the scope limited.
Visual Updates
You can kind of see this in the argo CI, but here it is slightly easier to see.
Before
After