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

Adapt dark mode based on system settings #12276

Closed
MariusBluem opened this issue Nov 5, 2018 · 23 comments · Fixed by #21366
Closed

Adapt dark mode based on system settings #12276

MariusBluem opened this issue Nov 5, 2018 · 23 comments · Fixed by #21366
Labels
4. to release Ready to be released and/or waiting for tests to finish design Design, UI, UX, etc. enhancement
Milestone

Comments

@MariusBluem
Copy link
Member

macOS Mojave is supporting dark mode and the Safari Technology Preview 68 is supporting automatically setting dark mode for homepages if set in system-settings. This seems to be handled via CSS prefers-color-scheme media query. Is it easy to support this? 🤔

Ref.: https://trac.webkit.org/changeset/237156/webkit

@skjnldsv @nextcloud/designers

@MariusBluem MariusBluem added design Design, UI, UX, etc. 1. to develop Accepted and waiting to be taken care of feature: accessibility labels Nov 5, 2018
@MariusBluem MariusBluem added this to the Nextcloud 15 milestone Nov 5, 2018
@nextcloud-bot
Copy link
Member

GitMate.io thinks possibly related issues are #1692 (Dark mode accessibility ), #11884 (Pretty URLs Breaks Dark Mode on Apache), #11656 (Dark Theme), #3289 (No WebDav Error when system is in single user mode), and #7218 (No feedback after password (re)set).

@rullzer rullzer removed this from the Nextcloud 15 milestone Nov 5, 2018
@skjnldsv
Copy link
Member

skjnldsv commented Nov 5, 2018

@MariusBluem I've indeed seen a doc passed by last week.
We'll need to wait for this to land on safari stable I guess :)

@MorrisJobke
Copy link
Member

@MariusBluem I've indeed seen a doc passed by last week.
We'll need to wait for this to land on safari stable I guess :)

Not that hard: https://developer.apple.com/safari/technology-preview/ 😉

@jancborchardt jancborchardt added this to the Nextcloud 16 milestone Nov 5, 2018
@MorrisJobke MorrisJobke added the good first issue Small tasks with clear documentation about how and in which place you need to fix things in. label Feb 25, 2019
@MorrisJobke
Copy link
Member

Looks like this needs to wait for 17.

@j-ed
Copy link
Contributor

j-ed commented May 8, 2019

Heise.de just reported (unfortunately only in German) that Apple asks website administrators to support the new function. Since end of April a first editors draft CSS specification "CSS Color Adjust Module Level 1" exists which describes this feature.

@juliusknorr
Copy link
Member

I just found that Safari, Chrome and Firefox already seem to support that:
https://developer.mozilla.org/en-US/docs/Web/CSS/@media/prefers-color-scheme

@skjnldsv
Copy link
Member

I like the day/night switch :)
Though if this is a css switch, it means we need to ship the two css and can't have such toggle via php. Though if we just differently declare css variables, the two should not be heavy to load 😉

Shall we do this for 18?
As well as switching to day/night icon instead of svg api?

@stefan-niedermann
Copy link
Member

@skjnldsv wouldn't it be possible to @import via css depending on the switch the right css-file? So one would be able to keep the css files separate instead of loading both always. Of course this would be a separate request and one had to check whether this costs more time than loading one big file or not.

@skjnldsv
Copy link
Member

@stefan-niedermann I thought about it, but it doesn't really matter :)
We should only need to change a few of our variables to completely change the theme :)

@raimund-schluessler
Copy link
Member

As far as I understand it #21366 is only for guest access and doesn’t implement this feature for authenticated users. Should we actually keep the issue open for this reason?

@nickvergessen
Copy link
Member

Yeah the autoclose was not intended, see comments on the PR

@nickvergessen nickvergessen reopened this Jun 22, 2020
@nickvergessen nickvergessen removed this from the Nextcloud 17.0.8 milestone Jun 22, 2020
@skjnldsv
Copy link
Member

skjnldsv commented Jul 16, 2020

https://caniuse.com/#search=prefers-color-scheme
Fully supported now.

@mcnesium
Copy link

As a quick workaround, I added this to core/css/server.css of the theme I use:

/* dark mode */
@media (prefers-color-scheme: dark) {
    #content {
        filter: invert(1) hue-rotate(180deg);
    }
    #content img {
        filter: invert(1) hue-rotate(180deg);
    }
}

This inverts everything, but then rotates the hue half around so that e.g. red stays red(ish), but black goes white and white goes black. Then, images are inverted back and again hue-rotated, so that they keep their original appearance.

@skjnldsv
Copy link
Member

This would make the pages barely usable for lots of apps.
Wide filters like those are crazy expensive on the browser when rendering.

@nickvergessen
Copy link
Member

Well if you want a workaround replace:

public function injectCss() {
// Inject the fake css on all pages if enabled and user is logged
$loggedUser = $this->userSession->getUser();
if (!is_null($loggedUser)) {
$userValues = $this->config->getUserKeys($loggedUser->getUID(), self::APP_NAME);
// we want to check if any theme or font is enabled.
if (count($userValues) > 0) {
$hash = $this->config->getUserValue($loggedUser->getUID(), self::APP_NAME, 'icons-css', md5(implode('-', $userValues)));
$linkToCSS = $this->urlGenerator->linkToRoute(self::APP_NAME . '.accessibility.getCss', ['md5' => $hash]);
\OCP\Util::addHeader('link', ['rel' => 'stylesheet', 'href' => $linkToCSS]);
}
} else {
$userValues = ['dark'];
$hash = md5(implode('-', $userValues));
$linkToCSS = $this->urlGenerator->linkToRoute(self::APP_NAME . '.accessibility.getCss', ['md5' => $hash]);
\OCP\Util::addHeader('link', ['rel' => 'stylesheet', 'media' => '(prefers-color-scheme: dark)', 'href' => $linkToCSS]);
}
}

with:

	public function injectCss() {
		$userValues = ['dark'];

		$hash = md5(implode('-', $userValues));
		$linkToCSS = $this->urlGenerator->linkToRoute(self::APP_NAME . '.accessibility.getCss', ['md5' => $hash]);
		\OCP\Util::addHeader('link', ['rel' => 'stylesheet', 'media' => '(prefers-color-scheme: dark)', 'href' => $linkToCSS]);
	}

then no matter what you select in the settings, the browsers system preference will be used.

@mcnesium
Copy link

@skjnldsv: True, that's why I called it a "workaround".

@nickvergessen: sounds fairly easy. Is there a way to implement that without screwing everything up and losing it on the next update?

@nickvergessen
Copy link
Member

No, because that ignores the users preference set in the settings.

@nickvergessen
Copy link
Member

Since we can only do things in CSS here a possible solution would be the following:

  1. We add a HTML part/notification alike think to the body, which is hidden
  2. If user has not selected dark mode nor ignore-dark-mode option set we load a "show dark-mode hint" css file
  3. The CSS file inside a prefers-color-scheme media selector unhides the note from 1. and click it leads you to the accessibility settings
  4. When note from 1. is closed via an X, we store a "ignore-dark-mode" option for the user and don't show it again.

@mcnesium
Copy link

Could you please explain, what makes this so complicated? What about this approach:

  • in the project there is a darktheme.scss file and a lighttheme.scss file
  • on release build, out of those files a darktheme.css, a lighttheme.css and a standard.css file are built, where the latest will have both styles within, separated by a prefers-color-scheme: dark and/or a prefers-color-scheme: light section
  • in the settings users can choose between three themes: dark, light and standard

Extended approaches would include CSS variables where there is a default theme and the prefers-color-scheme media queries only define the color variables.

Just curious…

@dartcafe
Copy link
Contributor

I am confused. For public pages, the dark mode is already supported. So it should be possible to adopt this for internal pages as well. Thought too easy?

@nickvergessen
Copy link
Member

The difference is that logged in users have a setting which should overrule the preference as far as I understand.
But also then it's really not complicated, I outlined the simple steps already above. Shouldn't take longer than an hour or something for someone with a bit of Nextcloud experience

@dartcafe
Copy link
Contributor

dartcafe commented Aug 21, 2021

The dark mode could clash with the themes, but the optimal way would be to let the user chose, which setting he prefers:

  • system setting (default)
  • dark mode
  • light mode (for compatibility with existing themes)

Aditionally, the dark theme could be deprecated or removed then.

@skjnldsv
Copy link
Member

#31751

@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed good first issue Small tasks with clear documentation about how and in which place you need to fix things in. 1. to develop Accepted and waiting to be taken care of labels Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish design Design, UI, UX, etc. enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.