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.

Sign up for GitHub

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

Jump to bottom

Light/Dark mode switch refactor: toggle on <html>, use CSS vars #11

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Light/Dark mode switch refactor: toggle on <html>, use CSS vars #11

wants to merge 3 commits into from

Conversation

peterkos
Copy link

@peterkos peterkos commented Jan 4, 2024 •
edited

Background

Hello!

I tried using the theme, but the way dark/light mode was implemented made it hard to customize variables:

  • Everything was nested in body.dark, requiring all styles to be duplicated across that and body
  • SASS variables were separate for light/dark and reused
// Before
body.dark-mode {
    a, a:link, a:visited {
	color: orange;
    }
}

// After
a, a:link, a:visited {
    color: orange;
}

Variables can now be overridden too:

:root.dark-mode {
    --accent-color: orange;
}

Contents

  • Replaced SASS vars with CSS var(--...) to allow overriding
    • One set of variables, set on :root.dark-mode or :root.light-mode
    • (prevents all style code from being duplicated!)
  • No longer nesting all styles in body
  • Removed @media (prefers-dark...), since JS logic already handles this

peterkos
peterkos commented Jan 4, 2024
View reviewed changes
sass/_theme.scss
#dark-mode-off {
display: inline;
thead, tr:nth-child(even) {
background-color: color-mix(in hsl, var(--tertiary) 10%, white);
Copy link
Author

Choose a reason for hiding this comment

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

Rough approximation (old vs. new)

background-color: lighten($tertiary, 10%);
background-color: color-mix(in hsl, var(--tertiary) 10%, white);

Relative color syntax isn't supported on FFox yet :( so this feels like an okay workaround.

@peterkos peterkos changed the title Refactor to remove extra class, and use CSS vars Light/Dark mode switch refactor -- class on <html>, use CSS vars Jan 5, 2024
@peterkos peterkos changed the title Light/Dark mode switch refactor -- class on <html>, use CSS vars Light/Dark mode switch refactor: toggle on <html>, use CSS vars Jan 5, 2024
@peterkos
Toggle is consistent and functional
ca0c5e0 
- Repsects prefers-dark by default in JS (no override in SASS)
- light/dark-mode class correctly applied to <html>, not <body>
@peterkos
Copy link
Author

peterkos commented Jan 5, 2024 •
edited

Reviewing in split veiw vs. unified may be easier here, git diff went crazy 😆

Also, everything in _theme.scss can now be put into style.css too! #less duplication

InputUsername
InputUsername requested changes Jan 28, 2024
View reviewed changes
Copy link
Owner

@InputUsername InputUsername left a comment •
edited

Choose a reason for hiding this comment

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

Thanks! Yeah, I didn't really like how the themes were implemented so this is very helpful.

Removed @media (prefers-dark...), since JS logic already handles this

I actually would prefer not to handle this in JS in case people have disabled it. Could you undo that change or at least make sure it also respects the user's dark mode setting without JS enabled?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@InputUsername InputUsername InputUsername requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@peterkos @InputUsername