-
Notifications
You must be signed in to change notification settings - Fork 399
Light mode in selected sections #1771
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
Conversation
|
Marked as controversial, not because I think anyone has any objection, but because I know @cart will have opinions on the palette etc. I think it looks really nice :) He's heads-down in BSN work right now though, so it may be a while until you get a review. |
|
@cart dark-mode now should look the same as Light-mode will be enabled only for: Getting Started, Migration Guides, Contributing Guide and Errors. News light-mode was started, but must be finished & activated in another PR. |
| <h1> | ||
| {{ section_or_page.extra.long_title | default(value=section_or_page.title) | safe }} | ||
| {% if section_or_page.extra.subtitle %} | ||
| <span class="docs-page-subtitle">{{ section_or_page.extra.subtitle }}</span> |
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.
is this no longer used?
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.
I couldn't find any instance of this being used, I think it's a feature no one knows about.
|
Left a few comments but overall the restructuring totally makes sense. The stepwise approach with the intersection of class and media queries is also great. |
|
Yup the palette looks good to me now! You've made inline code blocks "squarer". I prefer the "rounder" style / I think it fits our vibes more. Can you revert that? Additionally, the text in the headerbar is still pure white. Can you switch back to the slightly off-white color? |
|
I fixed the reported issues and found a couple of extra more. I think now it's done… 🤞🤞🤞 |
|
Changes look good to me |
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.
Leaving my approval, the css changes look well scoped to not cause issues with the current experience, and allow the gradual rollout of the media query based preference. I'd still defer to actual maintainers for approval of the color/style values.
|
Haha ok one more change and then we're good to merge: the cards on the Donate page no longer use the standard card background color. Can you switch that back? |
🙈🙈🙈🙈🙈🙈I just pushed A new bg-color rule slipped for the donate cards, now removed. I've also fixed a couple of issues with the link colors in light-mode. |
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.
We're good to go now. Thanks for bearing with me!
|
Yay!! This might technically be cosmetic but this is my most highly anticipated Bevy feature :D Thank you so much for all this hard work, I'm excited to see this on the Bevy website |
|
No problem @cart, the PR had out of scope changes. |
This PR builds on top of #1603 and hence on the huge effort spearheaded by @lynnpepin.
There are some changes though:
maincommit.--color-white: #000;. All BEM components and pages use token for colors now.docs.htmlwhich means that the quickstart, book, contributing guide and migration guides will show the light theme IF the user prefers light themes.layouts/base.html. This will add thelight-modeclass to the<html/>element.bevy-maybe-light-mode.mp4