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: implement dark mode #1308

Merged
merged 53 commits into from
Mar 12, 2024
Merged

feat: implement dark mode #1308

merged 53 commits into from
Mar 12, 2024

Conversation

e111077
Copy link
Collaborator

@e111077 e111077 commented Feb 28, 2024

Large CL that implements dark mode using mostly material 3's token system generated in monochrome with some site-specific additions. Some color refactors were necessary since they were either random magic colors or did not fit into the token system.

This Also adds reddot to the list of users.

Also there are some... color choices I had to make. The BG is very dark so that the tokens would match light and dark more more correctly. If I tried one-offing the tokens for light mode vs dark mode, there were lots of inconsistencies that looked bad in dark but good in light. So now we have quite contrasty blacks and saturated primary, lit blues around a lot of the site unfortunately

Copy link

github-actions bot commented Feb 28, 2024

A live preview of this PR will be available at the URL(s) below.
The latest URL will be appended to this comment on each push.
Each build takes ~5-10 minutes, and will 404 until finished.

https://pr1308-dad7a9d---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-b7c8f34---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-30003fe---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-f39d7ce---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-f2fdc6c---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-3c64548---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-3020c31---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-605045a---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-3a59290---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-006b891---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-d3d0078---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-5ac013a---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-7dd7a98---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-00bcc7e---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-6546965---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-1f1b461---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-6d60d8c---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-5b53491---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-f795cfa---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-77abcbf---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-cb39110---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-4764247---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-11359ea---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-a3a43bc---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-075f26f---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-8754d9a---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-56367c8---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-c7663dd---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-5023d0e---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-9c4b190---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-2a73c2a---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-ece11a6---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-cacf113---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-8c57577---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-a1dedc8---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-4b81320---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-3029c0e---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-0dcc11a---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1308-7556179---lit-dev-5ftespv5na-uc.a.run.app/

@e111077 e111077 marked this pull request as ready for review February 29, 2024 01:55
@sorvell
Copy link
Member

sorvell commented Feb 29, 2024

Amazing, love it!

  • Lit logo is invisible until hover on front page.
  • the contrast on some of the header text seems too subtle. I wonder if all the text should just be light, rather than have a blue-ish tint, e.g. the main header/footer text on the front page.
  • the button background seems a little too purple-ish.
  • learn page pill buttons don't seem to match the other colors.
  • what about adding a (can be short) blog post in conjunction with this?

@justinfagnani
Copy link
Contributor

The diagram on https://pr1308-9c4b190---lit-dev-5ftespv5na-uc.a.run.app/docs/composition/component-composition/#passing-data-up-and-down-the-tree is difficult to see because it has a transparent background and black lines.

@e111077
Copy link
Collaborator Author

e111077 commented Mar 4, 2024

Tests are gonna fail because of screenshots, but i think this might be good for another pass. Changed the Blue and due to this, I had to add a background to the active nav section on the top nav bar for contrast reasons

I added the /design/ page for your convenience as well.

@sorvell Thanks for looking into it, here's my responses.

  • Lit logo is invisible until hover on front page.

This is the default behavior on prod

  • the contrast on some of the header text seems too subtle. I wonder if all the text should just be light, rather than have a blue-ish tint, e.g. the main header/footer text on the front page.

Contrast should all be AA+ rated on this. I removed the dimmest one which was AA and turned h4's into the same color as h3 which is AAA rated.

  • the button background seems a little too purple-ish.

Changed color PTAL

  • learn page pill buttons don't seem to match the other colors.

They should now with the color change. PTAL

  • what about adding a (can be short) blog post in conjunction with this?

I'll write one up after this lands depending on how our comms should be considered going forward

@e111077 e111077 requested a review from AndrewJakubowicz March 6, 2024 21:46
@e111077
Copy link
Collaborator Author

e111077 commented Mar 11, 2024

ping for another review. All comments should have been addressed in some respect

Copy link
Member

@augustjk augustjk left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! This looks good to me.

packages/lit-dev-content/site/home/5-who-is-using.html Outdated Show resolved Hide resolved
Copy link
Member

@augustjk augustjk left a comment

Choose a reason for hiding this comment

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

LGTM

@e111077
Copy link
Collaborator Author

e111077 commented Mar 12, 2024

will happily address further suggestions in another PR

@e111077 e111077 merged commit 1399bea into main Mar 12, 2024
15 checks passed
@e111077 e111077 deleted the dark branch March 12, 2024 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants