search

caksoylar / keymap-drawer

Visualize keymaps that use advanced features like hold-taps and combos, with automatic parsing
https://caksoylar.github.io/keymap-drawer
MIT License
660 stars 57 forks source link

dark mode option #90

Closed snoyer closed 5 months ago

snoyer commented 5 months ago

Proposal to add "dark mode" capability by overriding the colors in CSS. The overriding can be ignored (by default, same as current behavior), forced/hardcoded, or conditional to match the client's @media (prefers-color-scheme: dark) setting.

I'm not sure what the defaults should be or how the option needs to be handled to play nice in the web app. Also the proposed dark color values are auto-generated by flipping their HSL value, picking them by hand could achieve better results but I'm not competent for that.

The following rendering of examples/showcase.yaml with --dark-mode=auto should adapt to the client browser's or OS's preference: showcase-auto

snoyer commented 5 months ago

Screen capture of expected behavior when changing the OS theme which gets propagated to the browser and then the generated SVG:

keymap-drawer-dark-mode.webm

caksoylar commented 5 months ago

Thank you for the PR, honestly this is an issue that's been bugging me for some time since the current theme doesn't look great on dark backgrounds and I had also tried implementing an auto mode. It's good to see that it works well embedded on Github, since that's where most people view it. I also have eventual plans to introduce theming, in which case dark mode would be one of the themes. But your implementation seems pretty nice right now, and I don't want perfect to be the enemy of good.

Given that I think I will merge this, but I'll probably spend some time tweaking colors.

Re: web app, it has issues regarding detection of light/dark (e.g. https://github.com/streamlit/streamlit/issues/6456) so the auto mode doesn't work. But I'll add this setting to the quick config boxes and probably default to auto for the app nevertheless.

snoyer commented 5 months ago

Cool, happy to help even if this PR ends up being just a proof of concept :) I just fixed a dumb bug when I forgot to remove the original CSS output line and the whole thing was duplicated.

Not sure if you want me to investigate pylint's complaints or if you want to refactor anyway?

caksoylar commented 5 months ago

Thanks again. Yeah, I'll probably end up refactoring it before merging, I'll handle the issues.

snoyer commented 5 months ago

nice! Your hand-picked colors do look better than my procedural ones. Just needs auto dark mode on the logo now ;)

caksoylar commented 5 months ago

Glad you like it. Replaced the logo as well now :)