search

rugk / website-dark-mode-switcher

This is a (Firefox) add-on (WebExtension) that lets you invert the website's color scheme by inverting/changing the prefers-color-scheme media feature of CSS.
https://addons.mozilla.org/firefox/addon/dark-mode-website-switcher/?src=external-github-top
Other
58 stars 4 forks source link

Simplify dramatically now that mediaText is read-write #29

Closed ix5 closed 3 years ago

ix5 commented 3 years ago

Background

This extension could be dramatically simplified using cssRules - media.mediaText.

We can de- or re-activate the matching condition. Each CSSMediaRule has a mediaText that, for dark-mode-only elements, contains. (prefers-color-scheme: dark). It is, to my great surprise, read-writable.

Proposed solution

See dark-mode-toggle as a proof-of-concept. You can see it in action here: Toggling dark mode manually via javascript

I know the iteration over all styles etc is a tragic sight. Javascript isn't my strong suit and I avoid it wherever possible. Also, I guess, styles that are explicitly for light mode only would get inadvertently inverted. We'd have to mark them specifically.

rugk commented 3 years ago

Awesome! Thanks for letting me know and noticing me of this possibility. Indeed, this is a new thing for me.

I know the iteration over all styles etc is a tragic sight.

Yeah, that is not a problem, actually we do that already here. Additionally, we need to use it recursively (as stylesheets can load other stylesheets etc.), but this is also already implemented in the latest version (see https://github.com/rugk/website-dark-mode-switcher/commit/fa7086ef875eaf350b605cc55f1db914c4a31b00). This file has the logic for applying that, actually, so if you want to create a PR, feel free to do that. Even a draft would already be very much appreciated! :smiley:

rugk commented 3 years ago

Do you want to take this on @ix5?

ix5 commented 3 years ago

The main issue I'm facing is no-preference. You cannot invert it. I had hoped to scrap all the insertCSS/removeCSS injection logic by simply inverting the relevant mediaText clauses.

Next try: swapping mediaText for placeholders. But... you cannot work with placeholders since at least FF turns non-valid media queries into the string not all rather than keeping the value, so you cannot use MediaList.appendMedium(<placeholder-text>).

So the great "simplification" is not to be. Sorry.

1.4k LOC just because Mozilla won't let us overwrite ui.systemUsesDarkTheme ourselves from an addon :-/

EDIT: Just to clarify, of course we could shim the MediaList objects and extend them to hold placeholders, but then the whole objective of simplifying would be kinda moot ;)

rugk commented 3 years ago

Oh okay, that's sad to hear, but thanks a lot for trying! And yes, obviously as you can see from my Bugzilla report and so on, I consider this whole add-on a big “hack”. I'm also not satisfied with that situation. That said, if there are “better” hacks, as you indicated in your edit, I'm obviously always open to change that. If it's better than the current solution, of course.