lionleaf / dwitter

Social network for short js demos
https://www.dwitter.net
Apache License 2.0
764 stars 69 forks source link

Add automatic dark mode #476

Closed veu closed 4 years ago

veu commented 4 years ago

Use dark theme if no theme preference is set and dark mode is preferred (via prefers-color-scheme).

Features:

PS: I ran the linter but all I got were errors in files I didn’t touch.

lionleaf commented 4 years ago

Good points. I like your suggestion of just ignoring "" and giving it a new value as a one time fix for the pull request.

On Sun, May 10, 2020 at 9:07 PM Rebecca König notifications@github.com wrote:

@veu commented on this pull request.

In dwitter/templates/base.html https://github.com/lionleaf/dwitter/pull/476#discussion_r422768510:

     };
  • // Use saved theme if localStorage is available

  • var theme = window.localStorage ? localStorage.getItem('theme') : null;

  • // Ignore unknown theme

  • if (themes.indexOf(theme) == -1) {

  • theme = null;

  • }

  • // Use dark mode preference if no theme is set

  • if (typeof theme != 'string' && window.matchMedia) {

As a one-time fix we could just use another key in local storage and ignore the old preference if it is '' and that could easily be done in this PR. I wouldn’t add the auto theme as long as it’s not displayed to the user because they’d cycle through the themes and see the “same” theme twice and wouldn’t know the difference.

As for long term behavior I’d prefer to move this to a place with a bit more room to show theme names and also explain what automatic means.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/lionleaf/dwitter/pull/476#discussion_r422768510, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEVE3MGTYIXKDHUHI3QZWTRQ52ZNANCNFSM4M5HRT7A .

veu commented 4 years ago

I renamed the light theme. The existing preference will now be treated like any other unknown theme, ie. default to automatic.