Skip to content

Commit

Permalink
feat: add nativeTheme.themeSource to allow apps to override Chromiums…
Browse files Browse the repository at this point in the history
… theme choice (#19960)

* feat: add nativeTheme.shouldUseDarkColorsOverride to allow apps to override Chromiums theme choice

* spec: add tests for shouldUseDarkColorsOverride

* chore: add missing forward declarations

* refactor: rename overrideShouldUseDarkColors to themeSource

* chore: only run appLevelAppearance specs on Mojave and up

* chore: update patch with more info and no define

* Update spec-main/api-native-theme-spec.ts

Co-Authored-By: Jeremy Apthorp <jeremya@chromium.org>

* Update api-native-theme-spec.ts

* Update api-native-theme-spec.ts

* Update api-native-theme-spec.ts
  • Loading branch information
MarshallOfSound committed Sep 5, 2019
1 parent 1376229 commit 0d16be9
Show file tree
Hide file tree
Showing 10 changed files with 294 additions and 3 deletions.
33 changes: 32 additions & 1 deletion docs/api/native-theme.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,38 @@ The `nativeTheme` module has the following properties:
### `nativeTheme.shouldUseDarkColors` _Readonly_

A `Boolean` for if the OS / Chromium currently has a dark mode enabled or is
being instructed to show a dark-style UI.
being instructed to show a dark-style UI. If you want to modify this value you
should use `themeSource` below.

### `nativeTheme.themeSource`

A `String` property that can be `system`, `light` or `dark`. It is used to override and supercede
the value that Chromium has chosen to use internally.

Setting this property to `system` will remove the override and
everything will be reset to the OS default. By default `themeSource` is `system`.

Settings this property to `dark` will have the following effects:
* `nativeTheme.shouldUseDarkColors` will be `true` when accessed
* Any UI Electron renders on Linux and Windows including context menus, devtools, etc. will use the dark UI.
* Any UI the OS renders on macOS including menus, window frames, etc. will use the dark UI.
* The [`prefers-color-scheme`](https://developer.mozilla.org/en-US/docs/Web/CSS/@media/prefers-color-scheme) CSS query will match `dark` mode.
* The `updated` event will be emitted

Settings this property to `light` will have the following effects:
* `nativeTheme.shouldUseDarkColors` will be `false` when accessed
* Any UI Electron renders on Linux and Windows including context menus, devtools, etc. will use the light UI.
* Any UI the OS renders on macOS including menus, window frames, etc. will use the light UI.
* The [`prefers-color-scheme`](https://developer.mozilla.org/en-US/docs/Web/CSS/@media/prefers-color-scheme) CSS query will match `light` mode.
* The `updated` event will be emitted

The usage of this property should align with a classic "dark mode" state machine in your application
where the user has three options.
* `Follow OS` --> `themeSource = 'system'`
* `Dark Mode` --> `themeSource = 'dark'`
* `Light Mode` --> `themeSource = 'light'`

Your application should then always use `shouldUseDarkColors` to determine what CSS to apply.

### `nativeTheme.shouldUseHighContrastColors` _macOS_ _Windows_ _Readonly_

Expand Down
1 change: 1 addition & 0 deletions filenames.gni
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ filenames = {
"shell/common/api/atom_api_native_image_mac.mm",
"shell/common/api/atom_api_native_theme.cc",
"shell/common/api/atom_api_native_theme.h",
"shell/common/api/atom_api_native_theme_mac.mm",
"shell/common/api/atom_api_shell.cc",
"shell/common/api/atom_api_v8_util.cc",
"shell/common/api/electron_bindings.cc",
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"@types/fs-extra": "^5.0.5",
"@types/mocha": "^5.2.6",
"@types/node": "^12.0.10",
"@types/semver": "^6.0.1",
"@types/send": "^0.14.5",
"@types/split": "^1.0.0",
"@types/webpack": "^4.4.32",
Expand Down Expand Up @@ -130,4 +131,4 @@
"git add filenames.auto.gni"
]
}
}
}
1 change: 1 addition & 0 deletions patches/chromium/.patches
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,4 @@ picture-in-picture.patch
disable_compositor_recycling.patch
allow_new_privileges_in_unsandboxed_child_processes.patch
expose_setuseragent_on_networkcontext.patch
feat_add_set_theme_source_to_allow_apps_to.patch
89 changes: 89 additions & 0 deletions patches/chromium/feat_add_set_theme_source_to_allow_apps_to.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Samuel Attard <sattard@slack-corp.com>
Date: Mon, 26 Aug 2019 14:32:41 -0700
Subject: feat: add set_theme_source to allow apps to override chromiums
internal theme choice

This patch is required as Chromium doesn't currently let folks using
//ui override the theme choice in NativeTheme. It defaults to
respecting the OS theme choice and some apps don't always want to do
that. With this patch we can override the theme value that Chromium
uses internally for things like menus and devtools.

We can remove this patch once it has in some shape been upstreamed.

diff --git a/ui/native_theme/native_theme.cc b/ui/native_theme/native_theme.cc
index 2370d15332c8c6c7dc7e3403b38891c885704d9f..171214379437f319d3feccc289a5d91e74b77f9e 100644
--- a/ui/native_theme/native_theme.cc
+++ b/ui/native_theme/native_theme.cc
@@ -40,6 +40,8 @@ NativeTheme::NativeTheme()
NativeTheme::~NativeTheme() = default;

bool NativeTheme::ShouldUseDarkColors() const {
+ if (theme_source() == ThemeSource::kForcedLight) return false;
+ if (theme_source() == ThemeSource::kForcedDark) return true;
return should_use_dark_colors_;
}

diff --git a/ui/native_theme/native_theme.h b/ui/native_theme/native_theme.h
index 70389e0245993faa2c17e9deefeb000280ef2368..cef1c0d4706e7e720a4004ca54765a39fc29c5e8 100644
--- a/ui/native_theme/native_theme.h
+++ b/ui/native_theme/native_theme.h
@@ -429,6 +429,22 @@ class NATIVE_THEME_EXPORT NativeTheme {
ColorId color_id,
ColorScheme color_scheme = ColorScheme::kDefault) const = 0;

+ enum ThemeSource {
+ kSystem,
+ kForcedDark,
+ kForcedLight,
+ };
+
+ ThemeSource theme_source() const {
+ return theme_source_;
+ }
+
+ void set_theme_source(ThemeSource theme_source) {
+ bool original = ShouldUseDarkColors();
+ theme_source_ = theme_source;
+ if (ShouldUseDarkColors() != original) NotifyObservers();
+ }
+
// Returns a shared instance of the native theme that should be used for web
// rendering. Do not use it in a normal application context (i.e. browser).
// The returned object should not be deleted by the caller. This function is
@@ -547,6 +563,8 @@ class NATIVE_THEME_EXPORT NativeTheme {
PreferredColorScheme preferred_color_scheme_ =
PreferredColorScheme::kNoPreference;

+ ThemeSource theme_source_ = ThemeSource::kSystem;
+
DISALLOW_COPY_AND_ASSIGN(NativeTheme);
};

diff --git a/ui/native_theme/native_theme_dark_aura.cc b/ui/native_theme/native_theme_dark_aura.cc
index a8fbfee3b13672902aac05fd5a65fa8ee81f9f7e..1be6369acf0b7c02a6f862636c2b2de1fbf8cb5a 100644
--- a/ui/native_theme/native_theme_dark_aura.cc
+++ b/ui/native_theme/native_theme_dark_aura.cc
@@ -20,6 +20,8 @@ SkColor NativeThemeDarkAura::GetSystemColor(ColorId color_id,
}

bool NativeThemeDarkAura::ShouldUseDarkColors() const {
+ if (theme_source() == ThemeSource::kForcedLight) return false;
+ if (theme_source() == ThemeSource::kForcedDark) return true;
return true;
}

diff --git a/ui/native_theme/native_theme_win.cc b/ui/native_theme/native_theme_win.cc
index 3003643bfb78cec2f5e84fc9e1471e1ef54aae41..06f2cbc84401958d49445f4ce6acb1b2fef0aa04 100644
--- a/ui/native_theme/native_theme_win.cc
+++ b/ui/native_theme/native_theme_win.cc
@@ -611,6 +611,8 @@ bool NativeThemeWin::ShouldUseDarkColors() const {
// ...unless --force-dark-mode was specified in which case caveat emptor.
if (UsesHighContrastColors() && !IsForcedDarkMode())
return false;
+ if (theme_source() == ThemeSource::kForcedLight) return false;
+ if (theme_source() == ThemeSource::kForcedDark) return true;
return NativeTheme::ShouldUseDarkColors();
}

56 changes: 56 additions & 0 deletions shell/common/api/atom_api_native_theme.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include "shell/common/api/atom_api_native_theme.h"

#include <string>

#include "native_mate/dictionary.h"
#include "native_mate/object_template_builder.h"
#include "shell/common/node_includes.h"
Expand All @@ -28,6 +30,20 @@ void NativeTheme::OnNativeThemeUpdated(ui::NativeTheme* theme) {
Emit("updated");
}

void NativeTheme::SetThemeSource(ui::NativeTheme::ThemeSource override) {
theme_->set_theme_source(override);
#if defined(OS_MACOSX)
// Update the macOS appearance setting for this new override value
UpdateMacOSAppearanceForOverrideValue(override);
#endif
// TODO(MarshallOfSound): Update all existing browsers windows to use GTK dark
// theme
}

ui::NativeTheme::ThemeSource NativeTheme::GetThemeSource() const {
return theme_->theme_source();
}

bool NativeTheme::ShouldUseDarkColors() {
return theme_->ShouldUseDarkColors();
}
Expand Down Expand Up @@ -68,6 +84,8 @@ void NativeTheme::BuildPrototype(v8::Isolate* isolate,
prototype->SetClassName(mate::StringToV8(isolate, "NativeTheme"));
mate::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate())
.SetProperty("shouldUseDarkColors", &NativeTheme::ShouldUseDarkColors)
.SetProperty("themeSource", &NativeTheme::GetThemeSource,
&NativeTheme::SetThemeSource)
.SetProperty("shouldUseHighContrastColors",
&NativeTheme::ShouldUseHighContrastColors)
.SetProperty("shouldUseInvertedColorScheme",
Expand All @@ -94,4 +112,42 @@ void Initialize(v8::Local<v8::Object> exports,

} // namespace

namespace mate {

v8::Local<v8::Value> Converter<ui::NativeTheme::ThemeSource>::ToV8(
v8::Isolate* isolate,
const ui::NativeTheme::ThemeSource& val) {
switch (val) {
case ui::NativeTheme::ThemeSource::kForcedDark:
return mate::ConvertToV8(isolate, "dark");
case ui::NativeTheme::ThemeSource::kForcedLight:
return mate::ConvertToV8(isolate, "light");
case ui::NativeTheme::ThemeSource::kSystem:
default:
return mate::ConvertToV8(isolate, "system");
}
}

bool Converter<ui::NativeTheme::ThemeSource>::FromV8(
v8::Isolate* isolate,
v8::Local<v8::Value> val,
ui::NativeTheme::ThemeSource* out) {
std::string theme_source;
if (mate::ConvertFromV8(isolate, val, &theme_source)) {
if (theme_source == "dark") {
*out = ui::NativeTheme::ThemeSource::kForcedDark;
} else if (theme_source == "light") {
*out = ui::NativeTheme::ThemeSource::kForcedLight;
} else if (theme_source == "system") {
*out = ui::NativeTheme::ThemeSource::kSystem;
} else {
return false;
}
return true;
}
return false;
}

} // namespace mate

NODE_LINKED_MODULE_CONTEXT_AWARE(atom_common_native_theme, Initialize)
20 changes: 20 additions & 0 deletions shell/common/api/atom_api_native_theme.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "native_mate/handle.h"
#include "shell/browser/api/event_emitter.h"
#include "ui/native_theme/native_theme.h"
#include "ui/native_theme/native_theme_observer.h"

namespace electron {
Expand All @@ -25,6 +26,12 @@ class NativeTheme : public mate::EventEmitter<NativeTheme>,
NativeTheme(v8::Isolate* isolate, ui::NativeTheme* theme);
~NativeTheme() override;

void SetThemeSource(ui::NativeTheme::ThemeSource override);
#if defined(OS_MACOSX)
void UpdateMacOSAppearanceForOverrideValue(
ui::NativeTheme::ThemeSource override);
#endif
ui::NativeTheme::ThemeSource GetThemeSource() const;
bool ShouldUseDarkColors();
bool ShouldUseHighContrastColors();
bool ShouldUseInvertedColorScheme();
Expand All @@ -42,4 +49,17 @@ class NativeTheme : public mate::EventEmitter<NativeTheme>,

} // namespace electron

namespace mate {

template <>
struct Converter<ui::NativeTheme::ThemeSource> {
static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
const ui::NativeTheme::ThemeSource& val);
static bool FromV8(v8::Isolate* isolate,
v8::Local<v8::Value> val,
ui::NativeTheme::ThemeSource* out);
};

} // namespace mate

#endif // SHELL_COMMON_API_ATOM_API_NATIVE_THEME_H_
37 changes: 37 additions & 0 deletions shell/common/api/atom_api_native_theme_mac.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright (c) 2019 Slack Technologies, Inc.
// Use of this source code is governed by the MIT license that can be
// found in the LICENSE file.

#include "shell/common/api/atom_api_native_theme.h"

#include "base/mac/sdk_forward_declarations.h"
#include "shell/browser/mac/atom_application.h"

namespace electron {

namespace api {

void NativeTheme::UpdateMacOSAppearanceForOverrideValue(
ui::NativeTheme::ThemeSource override) {
if (@available(macOS 10.14, *)) {
NSAppearance* new_appearance;
switch (override) {
case ui::NativeTheme::ThemeSource::kForcedDark:
new_appearance =
[NSAppearance appearanceNamed:NSAppearanceNameDarkAqua];
break;
case ui::NativeTheme::ThemeSource::kForcedLight:
new_appearance = [NSAppearance appearanceNamed:NSAppearanceNameAqua];
break;
case ui::NativeTheme::ThemeSource::kSystem:
default:
new_appearance = nil;
break;
}
[[NSApplication sharedApplication] setAppearance:new_appearance];
}
}

} // namespace api

} // namespace electron
52 changes: 51 additions & 1 deletion spec-main/api-native-theme-spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { expect } from 'chai'
import { nativeTheme } from 'electron'
import { nativeTheme, systemPreferences } from 'electron'
import * as os from 'os'
import * as semver from 'semver'

import { ifdescribe } from './spec-helpers'

describe('nativeTheme module', () => {
describe('nativeTheme.shouldUseDarkColors', () => {
Expand All @@ -8,6 +12,52 @@ describe('nativeTheme module', () => {
})
})

describe('nativeTheme.themeSource', () => {
afterEach(() => {
nativeTheme.themeSource = 'system'
})

it('is system by default', () => {
expect(nativeTheme.themeSource).to.equal('system')
})

it('should override the value of shouldUseDarkColors', () => {
nativeTheme.themeSource = 'dark'
expect(nativeTheme.shouldUseDarkColors).to.equal(true)
nativeTheme.themeSource = 'light'
expect(nativeTheme.shouldUseDarkColors).to.equal(false)
})

it('should emit the "updated" event when it is set and the resulting "shouldUseDarkColors" value changes', () => {
nativeTheme.themeSource = 'dark'
let called = false
nativeTheme.once('updated', () => {
called = true
})
nativeTheme.themeSource = 'light'
expect(called).to.equal(true)
})

it('should not emit the "updated" event when it is set and the resulting "shouldUseDarkColors" value is the same', () => {
nativeTheme.themeSource = 'dark'
let called = false
nativeTheme.once('updated', () => {
called = true
})
nativeTheme.themeSource = 'dark'
expect(called).to.equal(false)
})

ifdescribe(process.platform === 'darwin' && semver.gte(os.release(), '18.0.0'))('on macOS 10.14', () => {
it('should update appLevelAppearance when set', () => {
nativeTheme.themeSource = 'dark'
expect(systemPreferences.appLevelAppearance).to.equal('dark')
nativeTheme.themeSource = 'light'
expect(systemPreferences.appLevelAppearance).to.equal('light')
})
})
})

describe('nativeTheme.shouldUseInvertedColorScheme', () => {
it('returns a boolean', () => {
expect(nativeTheme.shouldUseInvertedColorScheme).to.be.a('boolean')
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,11 @@
resolved "https://registry.yarnpkg.com/@types/range-parser/-/range-parser-1.2.3.tgz#7ee330ba7caafb98090bece86a5ee44115904c2c"
integrity sha512-ewFXqrQHlFsgc09MK5jP5iR7vumV/BYayNC6PgJO2LPe8vrnNFyjQjSppfEngITi0qvfKtzFvgKymGheFM9UOA==

"@types/semver@^6.0.1":
version "6.0.1"
resolved "https://registry.yarnpkg.com/@types/semver/-/semver-6.0.1.tgz#a984b405c702fa5a7ec6abc56b37f2ba35ef5af6"
integrity sha512-ffCdcrEE5h8DqVxinQjo+2d1q+FV5z7iNtPofw3JsrltSoSVlOGaW0rY8XxtO9XukdTn8TaCGWmk2VFGhI70mg==

"@types/send@^0.14.5":
version "0.14.5"
resolved "https://registry.yarnpkg.com/@types/send/-/send-0.14.5.tgz#653f7d25b93c3f7f51a8994addaf8a229de022a7"
Expand Down

0 comments on commit 0d16be9

Please sign in to comment.