From 0d16be95600067a1f4d6f8e14b917e9edb2af8e3 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Thu, 5 Sep 2019 10:57:04 -0700 Subject: [PATCH] feat: add nativeTheme.themeSource to allow apps to override Chromiums 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 * Update api-native-theme-spec.ts * Update api-native-theme-spec.ts * Update api-native-theme-spec.ts --- docs/api/native-theme.md | 33 ++++++- filenames.gni | 1 + package.json | 3 +- patches/chromium/.patches | 1 + ...dd_set_theme_source_to_allow_apps_to.patch | 89 +++++++++++++++++++ shell/common/api/atom_api_native_theme.cc | 56 ++++++++++++ shell/common/api/atom_api_native_theme.h | 20 +++++ shell/common/api/atom_api_native_theme_mac.mm | 37 ++++++++ spec-main/api-native-theme-spec.ts | 52 ++++++++++- yarn.lock | 5 ++ 10 files changed, 294 insertions(+), 3 deletions(-) create mode 100644 patches/chromium/feat_add_set_theme_source_to_allow_apps_to.patch create mode 100644 shell/common/api/atom_api_native_theme_mac.mm diff --git a/docs/api/native-theme.md b/docs/api/native-theme.md index 815143c7a73a..c72e96e12559 100644 --- a/docs/api/native-theme.md +++ b/docs/api/native-theme.md @@ -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_ diff --git a/filenames.gni b/filenames.gni index da606270fef7..a7e8f1427311 100644 --- a/filenames.gni +++ b/filenames.gni @@ -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", diff --git a/package.json b/package.json index e97049e37a9e..68f1c9ba62ff 100644 --- a/package.json +++ b/package.json @@ -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", @@ -130,4 +131,4 @@ "git add filenames.auto.gni" ] } -} \ No newline at end of file +} diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 4feb743aef9c..5ad65be39d9d 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -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 diff --git a/patches/chromium/feat_add_set_theme_source_to_allow_apps_to.patch b/patches/chromium/feat_add_set_theme_source_to_allow_apps_to.patch new file mode 100644 index 000000000000..6e485f4927df --- /dev/null +++ b/patches/chromium/feat_add_set_theme_source_to_allow_apps_to.patch @@ -0,0 +1,89 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Samuel Attard +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(); + } + diff --git a/shell/common/api/atom_api_native_theme.cc b/shell/common/api/atom_api_native_theme.cc index a917d1fe3c47..af7db95e484d 100644 --- a/shell/common/api/atom_api_native_theme.cc +++ b/shell/common/api/atom_api_native_theme.cc @@ -4,6 +4,8 @@ #include "shell/common/api/atom_api_native_theme.h" +#include + #include "native_mate/dictionary.h" #include "native_mate/object_template_builder.h" #include "shell/common/node_includes.h" @@ -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(); } @@ -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", @@ -94,4 +112,42 @@ void Initialize(v8::Local exports, } // namespace +namespace mate { + +v8::Local Converter::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::FromV8( + v8::Isolate* isolate, + v8::Local 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) diff --git a/shell/common/api/atom_api_native_theme.h b/shell/common/api/atom_api_native_theme.h index 8c1405e38e20..ff86b07000fa 100644 --- a/shell/common/api/atom_api_native_theme.h +++ b/shell/common/api/atom_api_native_theme.h @@ -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 { @@ -25,6 +26,12 @@ class NativeTheme : public mate::EventEmitter, 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(); @@ -42,4 +49,17 @@ class NativeTheme : public mate::EventEmitter, } // namespace electron +namespace mate { + +template <> +struct Converter { + static v8::Local ToV8(v8::Isolate* isolate, + const ui::NativeTheme::ThemeSource& val); + static bool FromV8(v8::Isolate* isolate, + v8::Local val, + ui::NativeTheme::ThemeSource* out); +}; + +} // namespace mate + #endif // SHELL_COMMON_API_ATOM_API_NATIVE_THEME_H_ diff --git a/shell/common/api/atom_api_native_theme_mac.mm b/shell/common/api/atom_api_native_theme_mac.mm new file mode 100644 index 000000000000..47350586cae6 --- /dev/null +++ b/shell/common/api/atom_api_native_theme_mac.mm @@ -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 diff --git a/spec-main/api-native-theme-spec.ts b/spec-main/api-native-theme-spec.ts index cee34c83a058..c9c105fdfb00 100644 --- a/spec-main/api-native-theme-spec.ts +++ b/spec-main/api-native-theme-spec.ts @@ -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', () => { @@ -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') diff --git a/yarn.lock b/yarn.lock index 3178f8f18b55..0a93442e6bc2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -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"