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

(Automatic) dark mode #186

Closed
rugk opened this issue Apr 24, 2019 · 6 comments · Fixed by #204
Closed

(Automatic) dark mode #186

rugk opened this issue Apr 24, 2019 · 6 comments · Fixed by #204
Labels

Comments

@rugk
Copy link
Owner

rugk commented Apr 24, 2019

Current experience

image

on GNOME (Fedora/Linux) with Firefox 68 here.

In contrast to Firefox' own/internal popups:
image

Problems

  • A common issue I see on websites (but possibly also an issue of Firefox as it should not handle it like this?), is that input fields inherit the system's white font color by default.
  • Generally, the white popup stands out a lot…

Expected

  1. Possibly report the first thing as an issue in Firefox. (don't know how it looks on other OSes)
  2. Implement a dark mode that is triggered, when the system uses a dark theme…

To do so, there is this new fancy CSS media query: https://developer.mozilla.org/docs/Web/CSS/@media/prefers-color-scheme aka prefers-color-scheme
But should the need arise, you can use this JS API: https://developer.mozilla.org/en-US/docs/Web/API/Window/matchMedia

Note: There must not be any manual selection of this dark mode. I'd consider this an unnecessary option…

Of course, as always, adhere to the Photon design.

  1. (optional) As for the QR code itself, that would obviously usually also need to be inverted. So it could be an idea to also change the default colors of the QR code, if it is in dark mode. (of course, do not overwrite user settings though)
    However, this requires some JS changes (which is possible as explained above), but actually even worse: Many QR code scanners read them. (see Warning for inverted QR codes (when background color is dark and foreground light) #20)
    So, if we do so, we definitively need to implement Warning for inverted QR codes (when background color is dark and foreground light) #20 first. Or find some way that is still scannable, but looks decent in the dark mode.

Good first issue

This is a good first issue, as it is likely a CSS only change. (And hopefully with not-so many CSS changes.)

Also any more ideas on this are appreciated. 😃

@davidfloyd91
Copy link
Contributor

I'd be happy to take this one if it's available

@rugk
Copy link
Owner Author

rugk commented Jun 3, 2019

Sure… feel free to do so.

@davidfloyd91
Copy link
Contributor

So I hit an interesting snag here. prefers-color-scheme reads the user's preference from the OS, not the browser. If I enable dark mode on macOS Mojave (10.14.5) but select light mode on Firefox (67.0 (64-bit)), the CSS reads prefers-color-scheme: dark as true:

image

If I opt for light mode on Mac but dark mode on Firefox, prefers-color-scheme: dark is false:

image

It's only when both the browser and OS are set to dark (or light) that the desired behavior results:

image

Let me know if you're alright with that. I'll look into ways to read the theme preference directly from the browser in the mean time.

(By the way this issue arises whether you use the CSS media query or JS, eg let mql = window.matchMedia('(prefers-color-scheme: dark)');.)

@rugk
Copy link
Owner Author

rugk commented Jun 3, 2019

Let me know if you're alright with that.

Yes, I think this is a system limitation I am totally fine with… 😄

I'll look into ways to read the theme preference directly from the browser in the mean time.

Given the current APIs, i doubt it's possible.

(By the way this issue arises whether you use the CSS media query or JS, eg let mql = window.matchMedia('(prefers-color-scheme: dark)');.)

Hint: I've made an add-on to toggle that/that overrides this.
So I am very well aware of that, yes. 😄
(But of course, the add-on cannot influence other add-ons [yet], so it won't help for testing here.)

@rugk
Copy link
Owner Author

rugk commented Jun 3, 2019

BTW

reads the user's preference from the OS, not the browser

I've also already complained here about that:
https://discourse.mozilla.org/t/how-to-detect-the-dark-theme-in-a-webextension/38604?u=rugkx

@davidfloyd91
Copy link
Contributor

Given the current APIs, i doubt it's possible [but

Yeah it's not looking promising ...

So I am very well aware of that, yes. 😄

Ha I see you've been contending with this issue for a while. Definitely seems like something FF should improve

Yes, I think this is a system limitation I am totally fine with… 😄

In that case I'll submit a PR. Please let me know of any issues with the changes I've made. If something seems obvious, let's just say I'm taking you up on the "good-first-issue" tag here 😄

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 a pull request may close this issue.

2 participants