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: add themed css to getting started snippet #512

Merged
merged 2 commits into from Aug 27, 2021

Conversation

bennypowers
Copy link
Contributor

@bennypowers bennypowers commented Aug 26, 2021

Adds media queries to theme css links, and a deferred snippet to apply the dark theme class to the body if the user prefers dark color scheme. This should cover most cases except <noscript>

@vercel
Copy link

vercel bot commented Aug 26, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/shoelace/shoelace/BdTNwKMc9zUkc2sT1vhrF6XWk25j
✅ Preview: https://shoelace-git-fork-bennypowers-docs-themed-insta-28410b-shoelace.vercel.app

@bennypowers
Copy link
Contributor Author

alternative:

<link rel="stylesheet" media="(prefers-color-scheme:light)" href="https://cdn.jsdelivr.net/npm/@shoelace-style/shoelace@%VERSION%/dist/themes/light.css">
<link rel="stylesheet" media="(prefers-color-scheme:dark)"
      href="https://cdn.jsdelivr.net/npm/@shoelace-style/shoelace@%VERSION%/dist/themes/dark.css"
      onload="document.documentElement.classList.add('sl-theme-dark');">
<script type="module" src="https://cdn.jsdelivr.net/npm/@shoelace-style/shoelace@%VERSION%/dist/shoelace.js"></script>

@claviska
Copy link
Member

I kinda like the alternative approach. Since not everyone wants to support both themes (e.g. people adopting only a few of components in an existing app where the app doesn't support both), I wonder if this section should be broken down to demonstrate all use cases:

  1. Light theme only (how it currently is)
  2. Dark theme only (not written yet)
  3. Light + dark theme (how you've proposed)

Do you mind if I pull this in as the recommended way to support both and add the other two examples?

@bennypowers
Copy link
Contributor Author

Yeah sounds good. In that case please mention that the "alternative" snippet here is good to support user preferences, and maybe add a sentence about the media attribute which is maybe not so well known

@bennypowers
Copy link
Contributor Author

@claviska
Copy link
Member

Ugh, I missed this before today's release. I'll get it into beta.50.

@claviska claviska merged commit 147a8d2 into shoelace-style:next Aug 27, 2021
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

2 participants