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

Docs dark mode #28449

Closed
wants to merge 9 commits into from
Closed

Docs dark mode #28449

wants to merge 9 commits into from

Conversation

mdo
Copy link
Member

@mdo mdo commented Mar 11, 2019

This is still a little WIP and not perfect given how many live examples we have in our docs, but it's not bad. I've gone the route of overriding default values surgically. Still have some work to do on seeing how I could maybe improve the live examples.

Screen Shot 2019-03-10 at 7 32 37 PM

Live preview: https://deploy-preview-28449--twbs-bootstrap.netlify.com/

TODO:

  • lighten ToC items color
  • adapt to the recent doc changes
  • lighten text-muted so that its contrast ratio is >= 4.5
  • search for #767676 and #77757a and adapt the cases
  • examples and versions page: make sure the page header color is dark too
  • fix version toggler background
  • Move JS toggle code into the head to prevent flashing
  • Fix search input color to be lighter like white

@Johann-S
Copy link
Member

Nice ! 👍

Just a question, how do you switch to dark mode ?

@CyrilKrylatov
Copy link

CyrilKrylatov commented Mar 11, 2019

Nice ! 👍

Just a question, how do you switch to dark mode ?

Third line of the PR you have this @media (prefers-color-scheme: dark) wich is

used to detect if the user has requested the system use a light or dark color theme.

(source on MDN)

@bershanskiy
Copy link
Contributor

@mdo This looks nice.
According to MDN, the media query is supported only by yet unreleased Firefox 67 and masOS Safari 12.1. Is there a plan to create a manual toggle in JS that does not require media query for all other browsers to use (e.g., Chromium and derivatives and older Firefox, iOS Safari)?

@mdo
Copy link
Member Author

mdo commented Mar 11, 2019

According to MDN, the media query is supported only by yet unreleased Firefox 67 and masOS Safari 12.1. Is there a plan to create a manual toggle in JS that does not require media query for all other browsers to use (e.g., Chromium and derivatives and older Firefox, iOS Safari)?

Yeah, support is real bad right now for this, but currently no plans for manual toggle at this time. Maybe there's a polyfill or something else we can look into though?

@patrickhlauke
Copy link
Member

patrickhlauke commented Mar 12, 2019

won't be properly polyfillable, as it needs the ability of the browser to read a system/OS preference and set the media query accordingly. a polyfill obviously can't access system/OS specific flags.

either we double the whole dark mode media query up with a "or there's this class on the body" or similar approach (and then authors can write their own toggle that sets/unsets dark mode), or...we just leave it as it is.

[edit] addendum: similarly, we don't do anything for prefers-reduced-motion either (and that has the same "can't be polyfilled" issue)

@Johann-S
Copy link
Member

I would like a button to switch to the dark mode. A simple button which add a css class on the body or html element

@patrickhlauke
Copy link
Member

patrickhlauke commented Mar 12, 2019

Hmmm...I don't feel like it's a good common component as part of vanilla bootstrap to be honest. It seems to cross a line for me between providing some general purpose building blocks and a very specific control that affects the entire page/site (would also need to include cookie handling or session storage to remember the setting across multiple pages of a site, etc).

[edit] if you mean a button for docs specifically (i.e. we're not talking about a component in bootstrap itself), then ok, maybe...feels clunky though (and you can bet there will then be issues filed along the lines of "why does bootstrap not ship with that control so i can easily add it to my site?")

@Johann-S
Copy link
Member

Yep I thought about that only for our docs, not in Bootstrap itself

@XhmikosR
Copy link
Member

^^ yeah, agreed. We need a toggle button, and save the preference in local storage.

@mdo
Copy link
Member Author

mdo commented Mar 12, 2019

@Johann-S Can you explore options for stylesheets and local storage without jQuery? Ideally we can treat this new _dark-mode.scss partial as a separate stylesheet that'd enable us to toggle it on and off via a button, and then save that preference to local storage.

@XhmikosR
Copy link
Member

XhmikosR commented Mar 12, 2019

We can just build this scss to a separate CSS file, and load it when the button is toggled.

EDIT:

Pushed a couple of patches, the last one will be removed when we have the JS part.

@XhmikosR XhmikosR force-pushed the docs-dark-mode branch 2 times, most recently from 9b8ce02 to 564a3cb Compare March 12, 2019 16:46
@mdo
Copy link
Member Author

mdo commented Mar 12, 2019

We can just build this scss to a separate CSS file, and load it when the button is toggled.

Yup that's what I was saying in my last comment :). Thanks for the commit to add the separate file!

@mdo mdo mentioned this pull request Mar 13, 2019
13 tasks
@XhmikosR XhmikosR force-pushed the docs-dark-mode branch 3 times, most recently from 2e1287a to a40ab37 Compare March 14, 2019 00:19
@XhmikosR
Copy link
Member

XhmikosR commented Mar 17, 2019

BTW, I'm waiting for FF 67 to hit the beta channel which should be this week to try this and add the button. Where should we add the button?

EDIT:

All right, I tested this and works fine on Windows 10 with FF 67. So, now the button placement is left. Maybe it should be in the navbar? If so, we should have an SVG icon, suggestions welcome.

@mdo
Copy link
Member Author

mdo commented Mar 19, 2019

So, now the button placement is left. Maybe it should be in the navbar? If so, we should have an SVG icon, suggestions welcome.

Place it anywhere for now on a page and I can style/place it beyond that :).

@XhmikosR
Copy link
Member

XhmikosR commented Mar 20, 2019 via email

@Johann-S
Copy link
Member

Hi everybody,

I changed a lot of things, and I removed @media (prefers-color-scheme: dark) { because we store preferences in the localStorage, so I don't think it's necessary here.

I hope my changes won't bother you, feel free to nuke everything 👍

@XhmikosR XhmikosR force-pushed the docs-dark-mode branch 2 times, most recently from 48daa73 to ce48d9c Compare March 21, 2019 14:28
@XhmikosR
Copy link
Member

This needs an update @mdo. I rebased the branch BTW.

@josephgeis
Copy link

I filed a pull request that introduces mix-ins for prefers-color-scheme.
See #29552

@MartijnCuppens
Copy link
Member

Personally, I'm not a fan of this yet. This will be easier to achieve when we have CSS variables available, otherwise this will be a PITA to maintain.

We can't also just add a dark mode to our docs and don't support it in our components. It will just look too weird to have light components (like modals or dropdowns) in a dark documentation.

An other aspect is color contrast, we already struggle to get the color contrast on point without the dark mode, checking two modes won't make that easier.

@XhmikosR
Copy link
Member

Yeah, I agree. We need to postpone this for when we support CSS variable only. Unless there's a way to do this right now with a fallback, but I'm not sure what the overhead will be.

@mdo mdo added this to Inbox in v5.0.0-alpha3 via automation Feb 3, 2020
@mdo mdo removed this from Inbox in v5 Feb 3, 2020
@mdo
Copy link
Member Author

mdo commented Jun 16, 2020

Closing as stale.

@mdo mdo closed this Jun 16, 2020
@mdo mdo deleted the docs-dark-mode branch June 16, 2020 20:18
@XhmikosR
Copy link
Member

This worked up to some point but we can use CSS variables now so this can be simplified a lot.

@mdo mdo removed this from Inbox in v5.0.0-alpha3 Sep 16, 2020
@mdo mdo mentioned this pull request Mar 10, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants