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

Update mkdocs-material, better dark mode for code blocks #63

Closed

Conversation

StefanScherer
Copy link
Member

  • Update mkdocs and all dependencies
  • Update mkdocs-material to 5.3.3 to have better dark mode support
  • Cleaned up the CSS a bit using the defaults

Replaces #45

Signed-off-by: Stefan Scherer <stefan.scherer@docker.com>
Copy link
Member

@mikesir87 mikesir87 left a comment

Choose a reason for hiding this comment

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

Did a build locally and it all worked as expected. I actually do like the subtle changes to the dark theme coloring. Not a big deal, but the previous dark mode automatically changed the UI when you changed the system preferences. This update does it at page load, so you have to refresh to see the changes applied. Not sure if that's really a big deal though.

I noticed the diff view on "Updating our App" is readable, but that was already the case beforehand, so no need to hold this PR up for that.

@StefanScherer
Copy link
Member Author

Yes I saw the previous immediate dark mode toggle.
I don‘t know why it doesn‘t work, maybe worth filing an issue in upstream, so more people could benefit from it.

@mikesir87
Copy link
Member

mikesir87 commented Jun 26, 2020

It's because they are doing JS-based detection of the OS at page start and adding an attribute to the body to swap the theme being used, rather than using a CSS media query to swap it in the CSS stylesheet itself. We were using the media query previously, which is what gave the immediate update.

matchMedia("(prefers-color-scheme: dark)").matches&&document.body.setAttribute("data-md-color-scheme","slate")

@Den-dp
Copy link

Den-dp commented Jun 26, 2020

Overall looks good but still can see some unreadable code snippets (not a regression but neither a complete fix)

image
image

Looks like this is related with codehilite extension and its dark theme support

@mikesir87
Copy link
Member

Yeah... the first screenshot was a problem before this MR, as I mentioned in a comment earlier. The second is new. The first two CSS blocks should fix both problems.

I'm still not 100% sold on the yellow-y color (just looks kinda ugly to me), so the third block adjusts that.

The fourth one also helps brighten up some of the other keys, as I noticed in the second screenshot that some of it might be hard to read with the little amount of contrast between the dark colors.

[data-md-color-scheme="slate"] .highlight .gd, [data-md-color-scheme="slate"] .highlight .gi {
    color: #2e303e;
}

[data-md-color-scheme="slate"] .highlight .hll .nv, [data-md-color-scheme="slate"] .highlight .hll .nt {
  color: #012363;
}

[data-md-color-scheme="slate"] .highlight .hll {
  background-color: rgba(255, 255, 255, 0.5);
}

[data-md-color-scheme="slate"] .highlight .nv, [data-md-color-scheme="slate"] .highlight .se, [data-md-color-scheme="slate"] .highlight .si, [data-md-color-scheme="slate"] .highlight .nt {
  color: #0d9cec;
}

image

image

@squidfunk
Copy link

FYI, syntax coloring was improved in the last iterations and does now even provide a 4.5:1 contrast ratio, so even better accessibility. My advice would be to update to 5.5.7, which was released today:

90330244-890db500-dfab-11ea-905c-82c8b69aa56d

90330246-8ad77880-dfab-11ea-8a72-ba7317699b1f

Den-dp added a commit to Den-dp/getting-started that referenced this pull request May 10, 2021
Add script for restoring preferred theme setting.
Update config for newer `mkdocs-material`.
Use native `mkdocs-material` styles where possible.

Closes: docker#45 docker#63
@Den-dp
Copy link

Den-dp commented May 10, 2021

Bumped mkdocs-material in #149 with some extra features like theme switcher with persistence in localStorage

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.

None yet

5 participants