Page MenuHomePhabricator

Dark mode does not persist between pages
Open, Needs TriagePublic

Description

If I enable Dark Mode, I want it to stay this way until it is disabled rather than reset on page load.

Downstream report: https://phabricator.miraheze.org/T5065

Fixing this requires two steps:

Event Timeline

RhinosF1 created this task.Jan 5 2020, 9:56 AM
Restricted Application added a project: Community-Tech. · View Herald TranscriptJan 5 2020, 9:56 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
RhinosF1 moved this task from Radar to Miraheze-Linked on the User-RhinosF1 board.Jan 5 2020, 9:57 AM

Change 592766 had a related patch set uploaded (by MusikAnimal; owner: MusikAnimal):
[mediawiki/core@master] OutputPage: allow additional CSS classes to be added to <html>

https://gerrit.wikimedia.org/r/592766

MusikAnimal added a comment.EditedApr 27 2020, 9:43 PM

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/592766 is just part of it. That permits us to put the client-dark-mode CSS class on the <html> element on page load (rather than adding it with JS, which will cause a flashing effect).

The other part is actually storing and reading the hidden user preference, and calling OutputPage::addHtmlClasses( 'client-dark-mode' ) accordingly.

Another issue that I don't think has been raised yet is that on the WMF cluster the Varnish cache will prevent this code from running for logged-out users (since the whole document is cached). So I think either the dark mode feature will have to be disabled for logged out users (perhaps offer a wg config variable, since non-Varnish installations won't be effected), or logged out users just don't get to have a sticky dark mode.

Demian added a subscriber: Demian.EditedApr 27 2020, 10:52 PM

on the WMF cluster the Varnish cache will prevent this code from running for logged-out users (since the whole document is cached). So I think either the dark mode feature will have to be disabled for logged out users (perhaps offer a wg config variable, since non-Varnish installations won't be effected), or logged out users just don't get to have a sticky dark mode.

Afaik logged-out users don't have server-side settings, thus this setting can be stored only client-side. A synchronous (no flash/FOUC) solution for a similar feature in ca. 300 bytes (uncompressed): sidebar open state. The update logic is loaded asynchronously.

Afaik logged-out users don't have server-side settings, thus this setting can be stored only client-side. A synchronous (no flash/FOUC) solution for a similar feature in ca. 300 bytes (uncompressed): sidebar open state. The update logic is loaded asynchronously.

Good point! I haven't gotten that far but I was assuming it'd be cookie-based, which should work for logged-out users. Otherwise, I guess dark mode is a no go for them. Once you add JS into the picture you risk the "flashing" issue I was talking about, which can be quite annoying, especially on slower clients.

Afaik logged-out users don't have server-side settings, thus this setting can be stored only client-side. A synchronous (no flash/FOUC) solution for a similar feature in ca. 300 bytes (uncompressed): sidebar open state. The update logic is loaded asynchronously.

I was assuming it'd be cookie-based, which should work for logged-out users.

When I introduced useskin= as a cookie (T243920) the concern was that either the cache has to contain copies for each value (cache split / vary -> a waste of space) or the cache has to be skipped (pass -> increased backend load). It's also difficult to get a cookie through legal. LocalStorage has no such issues.

Once you add JS into the picture you risk the "flashing" issue I was talking about, which can be quite annoying, especially on slower clients.

Above-the-fold inline scripts are render-blocking. If you put the script element before the content then it will execute before first draw.

Change 592766 merged by jenkins-bot:
[mediawiki/core@master] OutputPage: allow additional CSS classes to be added to <html>

https://gerrit.wikimedia.org/r/592766

MusikAnimal updated the task description. (Show Details)May 8 2020, 8:26 PM
MusikAnimal updated the task description. (Show Details)