Darkmode toggle take 2 #323
Conversation
|
Looks… good to me, I think. Just a small notice about icons: the Moon is filled (white), but the Sun is outlined (transparent). Let's look what icons are used… oh, Unicode symbols. I suggest black one, not white: https://unicode-table.com/en/2600/ But it can be rendered colorful, like Emoji, and probably we should use some (SVG) monotone external icons. Also I've faced recently with problems of star icon (this one) on macOS and iOS, so we should check Unicode symbols there. |
| @@ -71,4 +71,18 @@ pre:not(.ruby) { | |||
| } | |||
| } | |||
|
|
|||
| .light-switch:before { | |||
| content: '☾'; | |||
There was a problem hiding this comment.
I've been using https://fontawesome.com for the UI icons, which you can find here. Would we be able to use the icons here as well?
There was a problem hiding this comment.
Let‘s do this in later Pull Request? I did not find ones on fontawesome that are better than current unicode ones. Ideally we could have CSS icons like the ones in https://justjavascript.com/.
There was a problem hiding this comment.
I did not find ones on fontawesome that are better than current unicode ones.
Moon: https://fontawesome.com/icons/moon?style=solid
Sun: https://fontawesome.com/icons/sun?style=solid (a bit another, but seems OK)
Bulb icons (like in Twitter) are paid in Font Awesome.
Also sometimes I think we can have too many icons/buttons in the header, and there will be some drop-down menu, and then it'd be better to do just toggle with text, without icon.
JuanitoFatas
force-pushed
the
jf.darkmode-toggle-v2
branch
from
77f3286 to
7bfac6a
Compare
|
@JuanitoFatas there are conflicts. Can you please resolve them? And do you need a help? I'm ready to try. ❤️ |
JuanitoFatas
force-pushed
the
jf.darkmode-toggle-v2
branch
from
7bfac6a to
6460bfc
Compare
|
@AlexWayfer Conflicts resolved. Feel free to send a PR to this branch. |
Done in #357. |
|
Apologies for the delay again 🙇♂️. I am looking at this now. |
No problem. I just tried to simplify selectors and actions in #357, and waiting when @JuanitoFatas will look at this. |
|
@JuanitoFatas can you please rebase this PR? Or should we a new one with such changes? |
|
Ah i have a couple of big changes to this PR that i will try to push in the next few days. I'm happy to do the rebase myself |
|
👋 @JuanitoFatas I'm really really sorry for the super long delay in getting this merged. I had a close look at the tailwindcss theming plugin and ended up not liking having to specify colours in a separate file. I really like specifying the colours inline via the tailwindcss colours, ie: Your stimulus component was great and really liked the approach! But I ended up using the the awesome-font icon from #357 to keep the UI consistent. The header is a bit of a mess at the moment, so I've hidden the theme icon on every page except the homepage for mobile devices. I'm about 1/2 way through redesigning the header from scratch and adding a hamburger menu option that will look much better and fit everything more nicely! Thanks so much again for working on this! |
Summary
This Pull Request adds a dark mode switch (html button) in the top right nav menu. The switch defaults to user‘s system darkmode setting. Once toggled will store preference in the
Window.localStorage. The switch icons are☼, U+263Cand☾, U+263E.The switch is managed by a Stimulus controller
theme-switch_controller.js. When the page loads, we check the OS and localStorage to find user‘s preference and update the switch css class. When user clicks it, we toggle the css class and store preference in localStorage.When we use tailwindcss-theming plugin, the tailwind‘s original colors are all gone. So I added back what we are using in
theme.config.js, then extract all places using colors in the view into CSS classes.Added css class for development search query‘s code block display.
Implements #290.
Try at local
Review
We could use the css classes separately or extract the common values into generic names. Suggestions welcome.
Screenshots