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: Add dark mode support #110

Merged
merged 1 commit into from
Dec 6, 2023
Merged

Conversation

zbynek
Copy link
Contributor

@zbynek zbynek commented Nov 3, 2023

Adds theme attribute that can be either light (default), dark or auto. That should allow adding dark mode to plugin site before we add it to the others and allow adding a color theme toggle to the page in the future.

@@ -26,7 +26,6 @@ export default {
input,
plugins: [
litcss({
include: '/**/*.css',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It matches css files by default, using regex. It seems that regex works cross-platform and glob is POSIX only.

@@ -21,7 +21,7 @@ export const setLocaleFromUrl = async () => {
const locale = url.searchParams.get('locale') || (new Intl.NumberFormat()).resolvedOptions().locale || sourceLocale;
await setLocale(locale);
};
setLocaleFromUrl().catch(e => console.error(`Error loading locale: ${(e as Error).message}`));
setLocaleFromUrl().catch(e => console.log(`Error loading locale: ${(e as Error).message}`));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated to dark mode, but not showing this as error on each page load helps with debuging.

Copy link
Member

Choose a reason for hiding this comment

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

Can you make this a seperate fix pr?

@zbynek
Copy link
Contributor Author

zbynek commented Nov 29, 2023

@halkeye could you please check/merge this?

@halkeye
Copy link
Member

halkeye commented Nov 29, 2023

https://deploy-preview-110--jenkins-io-components.netlify.app/?path=/story/example-navbar--same-property-active-link&args=theme:dark

when you change the control it doesn't seem to change anything. Is that expected? Can you setup a story to demo it?

@zbynek
Copy link
Contributor Author

zbynek commented Dec 6, 2023

when you change the control it doesn't seem to change anything

It should change the header background from pitch-black (dark mode) to dark gray (light mode); auto mode is identical to one of these, depending on your system theme. If you're using some auto-dark-mode browser feature, the difference may not be visible at all.

@halkeye
Copy link
Member

halkeye commented Dec 6, 2023

Okay I see it now, the colors on my laptop are not that vivid, with the white background it wasn't hugely noticable shift.

Btw, I think https://storybook.js.org/addons/storybook-dark-mode is installed if you want to hook it up to that instead of a custom property, though I think the property override is a good plan.

@halkeye halkeye self-requested a review December 6, 2023 06:19
@halkeye halkeye merged commit 7f253cd into jenkins-infra:main Dec 6, 2023
13 checks passed
@jenkins-io-components
Copy link

🎉 This PR is included in version 1.27.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants