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

feat: update website & init template palette to pass WCAG test; include contrast check in ColorGenerator #5822

Merged
merged 28 commits into from
Jan 20, 2022

Conversation

Josh-Cena
Copy link
Collaborator

@Josh-Cena Josh-Cena commented Oct 29, 2021

Motivation

Resolve #4947. The theme colors we use are too light in light mode. We can test this on the website first, and also encourage setting different primary color palettes in the docs and init templates.

Would there be branding concerns if we darken the color? What would be a good compromise?

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Preview; the lighthouse accessibility score should now be 100

Try the new Color generator! https://deploy-preview-5822--docusaurus-2.netlify.app/docs/styling-layout/#styling-your-site-with-infima

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Oct 29, 2021
@Josh-Cena Josh-Cena added the pr: polish This PR adds a very minor behavior improvement that users will enjoy. label Oct 29, 2021
@Josh-Cena Josh-Cena changed the title docs: update website palette to pass WCAG test docs: update website & init template palette to pass WCAG test Oct 29, 2021
@Josh-Cena Josh-Cena marked this pull request as draft October 29, 2021 10:46
@netlify
Copy link

netlify bot commented Oct 29, 2021

✔️ [V2]

🔨 Explore the source changes: dcb56b1

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/61e8f83d79ea0800070f92f8

😎 Browse the preview: https://deploy-preview-5822--docusaurus-2.netlify.app

@github-actions
Copy link

github-actions bot commented Oct 29, 2021

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 75
🟢 Accessibility 100
🟢 Best practices 93
🟢 SEO 100
🟢 PWA 92

Lighthouse ran on https://deploy-preview-5822--docusaurus-2.netlify.app/

@github-actions
Copy link

github-actions bot commented Oct 29, 2021

Size Change: +28.1 kB (+4%)

Total Size: 706 kB

Filename Size Change
website/.docusaurus/globalData.json 43.5 kB +617 B (+1%)
website/build/assets/css/styles.********.css 103 kB +638 B (+1%)
website/build/assets/js/main.********.js 530 kB +26.9 kB (+5%) 🔍
ℹ️ View Unchanged
Filename Size
website/build/index.html 29.6 kB

compressed-size-action

@slorber
Copy link
Collaborator

slorber commented Oct 29, 2021

Would there be branding concerns if we darken the color? What would be a good compromise?

🤷‍♂️ Apart text contrast, I think having too dark backgrounds is also not a very good practice. Not sure where I read that

@Josh-Cena
Copy link
Collaborator Author

I think having too dark backgrounds is also not a very good practice.

True, the dark mode is a bit too sharp, but we are in line with the GitHub dark style so we are not alone. I'm loving the design of VP v2 (https://v2.vuepress.vuejs.org) and they overall have some good design points.

The dark mode theme palette would need to be on the Infima side though, and here I'm more tweaking the primary colors https://webaim.org/resources/contrastchecker/?fcolor=25C2A0&bcolor=FFFFFF

@Josh-Cena Josh-Cena marked this pull request as ready for review November 7, 2021 10:55
@Josh-Cena Josh-Cena changed the title docs: update website & init template palette to pass WCAG test feat: update website & init template palette to pass WCAG test Nov 7, 2021
@Josh-Cena
Copy link
Collaborator Author

@slorber This is what I can do on the user-side: using alternative palettes. Honestly there are few theme colors that work equally well on light & dark backgrounds so this should be encouraged practice. Making the dark background lighter in Infima is also 👍

@Josh-Cena Josh-Cena changed the title feat: update website & init template palette to pass WCAG test feat: update website & init template palette to pass WCAG test; include contrast check in ColorGenerator Nov 8, 2021
@Josh-Cena
Copy link
Collaborator Author

@slorber Ready for review. I personally don't see much branding concern since the color is visually similar. Just realized the Jest site uses alternative palettes as well

@Josh-Cena Josh-Cena added the status: awaiting review This PR is ready for review, will be merged after maintainers' approval label Dec 2, 2021
@slorber
Copy link
Collaborator

slorber commented Dec 3, 2021

I'm not a designer so I don't know too much if the new colors are better 😅 I'm fine with those.

If this helps us have better lighthouse score, maybe we should have a way to test for this?

What I understand is this PR moves from 98 to 100 on accessibility score. Is there a way to ensure we stay at 100 and fail if it's not the case? That seems like a good lighthouse practice to require a 100% score and to make the config stricter over time.


I like the new color generator, that's a great improvement.

What I find confusing is that it displays "Fail" on the first load, this is a weird UX and users may not understand.

image

What about ensuring the ability to select a dark and light background too?
Eventually, use different primary color for dark/light too?

Was wondering if we could also generate hue/hsl vars instead of hex codes?

image

That could make it easier for users to tweak the primary color without using the tool every time.
In an ideal world most users should just declare 1 primary color and Docusaurus should compute a good palette by default.

Technically Docusaurus could compute the boundaries at which WCAG passes AA or AAA, and then compute the "intermediate colors"?


Some ideas I have for a while is to have the color generator directly impact the docusaurus site colors (not necessarily persisted, but can be nice to see the look and feel live of a given color).

CF https://blog.maximeheckel.com/posts/the-power-of-composition-with-css-variables/

image

I'd love to even have a little color picker button directly in our navbar, where you can just select a hue

image

(+ be able to navigate to a dedicated page for this color generator for more customization options)

@Josh-Cena
Copy link
Collaborator Author

I'm not a designer so I don't know too much if the new colors are better 😅 I'm fine with those.

There's no such thing as a "good theme color" anyways🤪 You choose one that conforms to a11y rules and you stick to it

What I understand is this PR moves from 98 to 100 on accessibility score. Is there a way to ensure we stay at 100 and fail if it's not the case? That seems like a good lighthouse practice to require a 100% score and to make the config stricter over time.

Hmm, our lighthouse report right now is a little ad hoc because it only checks the landing page and only checks light mode. A score of 100 doesn't tell much about the site's a11y anyways.

What I find confusing is that it displays "Fail" on the first load, this is a weird UX and users may not understand.

It's because we didn't put an admonition in the current version where I explain what the "contrast" is. I'll try to improve that

What about ensuring the ability to select a dark and light background too?

That could happen👍 Since both of us also agree that the background is too dark right now

Technically Docusaurus could compute the boundaries at which WCAG passes AA or AAA, and then compute the "intermediate colors"?

I believe we don't need the entire palette to pass WCAG, just the primary color. It's fine for the lightest or darkest to fail since they are nearly never used anyways.

Some ideas I have for a while is to have the color generator directly impact the docusaurus site colors (not necessarily persisted, but can be nice to see the look and feel live of a given color).

Absolutely! I've been thinking about it as well and even made a prototype. It would be persisted across pages but not in local storage. I think we can even try making the code block editable and let users enter arbitrary CSS?

I'd love to even have a little color picker button directly in our navbar, where you can just select a hue

We can try🧐

@Josh-Cena Josh-Cena added status: awaiting review This PR is ready for review, will be merged after maintainers' approval pr: documentation This PR works on the website or other text documents in the repo. and removed status: don't merge yet This PR is completed, but we are still waiting for other things to be ready. labels Dec 19, 2021
@Josh-Cena
Copy link
Collaborator Author

There's one very important thing I can't figure out: after the user leaves the styling & layout page, when she toggles color mode, the styles are not updated properly, because the style has been explicitly set on the element. Maybe we need to register the colors to the Layout component?

@Josh-Cena Josh-Cena added the status: don't merge yet This PR is completed, but we are still waiting for other things to be ready. label Dec 20, 2021
@Josh-Cena Josh-Cena marked this pull request as draft December 20, 2021 14:08
@Josh-Cena Josh-Cena marked this pull request as ready for review January 17, 2022 03:53
@Josh-Cena Josh-Cena removed the status: don't merge yet This PR is completed, but we are still waiting for other things to be ready. label Jan 17, 2022
Copy link
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks good to me 👍

What I don't like is that the new color is not very noticeable 😓 it's hard to have that green light mode looks nice and be accessible at the same time 😅

image

The issue is particularly visible for smaller test like links and the TOC

image

website/src/utils/colorUtils.ts Outdated Show resolved Hide resolved
@Josh-Cena
Copy link
Collaborator Author

True... I noticed that v1's green color is actually accessible, so I just stole that one for light mode. Looks slightly better.

@Josh-Cena Josh-Cena removed the status: awaiting review This PR is ready for review, will be merged after maintainers' approval label Jan 20, 2022
@Josh-Cena Josh-Cena merged commit abdcad7 into main Jan 20, 2022
@Josh-Cena Josh-Cena deleted the jc/website-wcag branch January 20, 2022 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: documentation This PR works on the website or other text documents in the repo. pr: polish This PR adds a very minor behavior improvement that users will enjoy.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default theme colors are inaccessible (links, sidebar text)
3 participants