From 6ecf729487b3778ab5aadf4728dff4843e7c4022 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Sun, 29 Mar 2020 23:02:16 -0700 Subject: [PATCH] feat: default gtk darkTheme option to nativeTheme.shouldUseDarkColors for better platform support (#20138) * feat: default gtk darkTheme option to nativeTheme.shouldUseDarkColors for better platform support * chore: update syntax for PR feedback * refactor: only define SetGTKDarkThemeEnabled when needed Co-authored-by: Cheng Zhao --- docs/api/browser-window.md | 2 +- .../browser/api/electron_api_native_theme.cc | 11 ++++- .../api/electron_api_top_level_window.cc | 4 -- .../api/electron_api_top_level_window.h | 1 - shell/browser/native_window.h | 2 - shell/browser/native_window_mac.h | 1 - shell/browser/native_window_views.cc | 47 ++++++++++--------- shell/browser/native_window_views.h | 6 ++- 8 files changed, 38 insertions(+), 36 deletions(-) diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index 1fa06de157c..a78c7fb0907 100644 --- a/docs/api/browser-window.md +++ b/docs/api/browser-window.md @@ -207,7 +207,7 @@ It creates a new `BrowserWindow` with native properties as set by the `options`. * `opacity` Number (optional) - Set the initial opacity of the window, between 0.0 (fully transparent) and 1.0 (fully opaque). This is only implemented on Windows and macOS. * `darkTheme` Boolean (optional) - Forces using dark theme for the window, only works on - some GTK+3 desktop environments. Default is `false`. + some GTK+3 desktop environments. Default is [`nativeTheme.shouldUseDarkColors`](native-theme.md). * `transparent` Boolean (optional) - Makes the window [transparent](frameless-window.md#transparent-window). Default is `false`. On Windows, does not work unless the window is frameless. * `type` String (optional) - The type of window, default is normal window. See more about diff --git a/shell/browser/api/electron_api_native_theme.cc b/shell/browser/api/electron_api_native_theme.cc index e0d333d8383..643f0d6d286 100644 --- a/shell/browser/api/electron_api_native_theme.cc +++ b/shell/browser/api/electron_api_native_theme.cc @@ -10,6 +10,8 @@ #include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_thread.h" #include "gin/handle.h" +#include "shell/browser/native_window_views.h" +#include "shell/browser/window_list.h" #include "shell/common/gin_converters/std_converter.h" #include "shell/common/gin_helper/dictionary.h" #include "shell/common/gin_helper/object_template_builder.h" @@ -47,8 +49,13 @@ void NativeTheme::SetThemeSource(ui::NativeTheme::ThemeSource override) { // 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 +#if defined(USE_X11) + const bool dark_enabled = ShouldUseDarkColors(); + for (auto* window : WindowList::GetWindows()) { + static_cast(window)->SetGTKDarkThemeEnabled( + dark_enabled); + } +#endif } ui::NativeTheme::ThemeSource NativeTheme::GetThemeSource() const { diff --git a/shell/browser/api/electron_api_top_level_window.cc b/shell/browser/api/electron_api_top_level_window.cc index 469098d612b..d92b558a80e 100644 --- a/shell/browser/api/electron_api_top_level_window.cc +++ b/shell/browser/api/electron_api_top_level_window.cc @@ -906,10 +906,6 @@ void TopLevelWindow::CloseFilePreview() { window_->CloseFilePreview(); } -void TopLevelWindow::SetGTKDarkThemeEnabled(bool use_dark_theme) { - window_->SetGTKDarkThemeEnabled(use_dark_theme); -} - v8::Local TopLevelWindow::GetContentView() const { if (content_view_.IsEmpty()) return v8::Null(isolate()); diff --git a/shell/browser/api/electron_api_top_level_window.h b/shell/browser/api/electron_api_top_level_window.h index 2becb245bd4..0102f4a8e88 100644 --- a/shell/browser/api/electron_api_top_level_window.h +++ b/shell/browser/api/electron_api_top_level_window.h @@ -208,7 +208,6 @@ class TopLevelWindow : public gin_helper::TrackableObject, void SetAspectRatio(double aspect_ratio, gin_helper::Arguments* args); void PreviewFile(const std::string& path, gin_helper::Arguments* args); void CloseFilePreview(); - void SetGTKDarkThemeEnabled(bool use_dark_theme); // Public getters of NativeWindow. v8::Local GetContentView() const; diff --git a/shell/browser/native_window.h b/shell/browser/native_window.h index 8f6f5ade614..a0077f3104c 100644 --- a/shell/browser/native_window.h +++ b/shell/browser/native_window.h @@ -232,8 +232,6 @@ class NativeWindow : public base::SupportsUserData, const std::string& display_name); virtual void CloseFilePreview(); - virtual void SetGTKDarkThemeEnabled(bool use_dark_theme) = 0; - // Converts between content bounds and window bounds. virtual gfx::Rect ContentBoundsToWindowBounds( const gfx::Rect& bounds) const = 0; diff --git a/shell/browser/native_window_mac.h b/shell/browser/native_window_mac.h index 1c64a0fd730..11bb4b008a9 100644 --- a/shell/browser/native_window_mac.h +++ b/shell/browser/native_window_mac.h @@ -137,7 +137,6 @@ class NativeWindowMac : public NativeWindow, public ui::NativeThemeObserver { std::vector items) override; void RefreshTouchBarItem(const std::string& item_id) override; void SetEscapeTouchBarItem(gin_helper::PersistentDictionary item) override; - void SetGTKDarkThemeEnabled(bool use_dark_theme) override {} gfx::Rect ContentBoundsToWindowBounds(const gfx::Rect& bounds) const override; gfx::Rect WindowBoundsToContentBounds(const gfx::Rect& bounds) const override; diff --git a/shell/browser/native_window_views.cc b/shell/browser/native_window_views.cc index e9605a376ed..a4169343d02 100644 --- a/shell/browser/native_window_views.cc +++ b/shell/browser/native_window_views.cc @@ -34,6 +34,7 @@ #include "ui/base/hit_test.h" #include "ui/gfx/image/image.h" #include "ui/gfx/native_widget_types.h" +#include "ui/native_theme/native_theme.h" #include "ui/views/background.h" #include "ui/views/controls/webview/unhandled_keyboard_event_handler.h" #include "ui/views/controls/webview/webview.h" @@ -212,10 +213,10 @@ NativeWindowViews::NativeWindowViews(const gin_helper::Dictionary& options, window_state_watcher_ = std::make_unique(this); // Set _GTK_THEME_VARIANT to dark if we have "dark-theme" option set. - bool use_dark_theme = false; - if (options.Get(options::kDarkTheme, &use_dark_theme) && use_dark_theme) { - SetGTKDarkThemeEnabled(use_dark_theme); - } + bool use_dark_theme = + ui::NativeTheme::GetInstanceForNativeUi()->ShouldUseDarkColors(); + options.Get(options::kDarkTheme, &use_dark_theme); + SetGTKDarkThemeEnabled(use_dark_theme); // Before the window is mapped the SetWMSpecState can not work, so we have // to manually set the _NET_WM_STATE. @@ -325,25 +326,6 @@ NativeWindowViews::~NativeWindowViews() { #endif } -void NativeWindowViews::SetGTKDarkThemeEnabled(bool use_dark_theme) { -#if defined(USE_X11) - XDisplay* xdisplay = gfx::GetXDisplay(); - if (use_dark_theme) { - XChangeProperty(xdisplay, GetAcceleratedWidget(), - XInternAtom(xdisplay, "_GTK_THEME_VARIANT", x11::False), - XInternAtom(xdisplay, "UTF8_STRING", x11::False), 8, - PropModeReplace, - reinterpret_cast("dark"), 4); - } else { - XChangeProperty(xdisplay, GetAcceleratedWidget(), - XInternAtom(xdisplay, "_GTK_THEME_VARIANT", x11::False), - XInternAtom(xdisplay, "UTF8_STRING", x11::False), 8, - PropModeReplace, - reinterpret_cast("light"), 5); - } -#endif -} - void NativeWindowViews::SetContentView(views::View* view) { if (content_view()) { root_view_->RemoveChildView(content_view()); @@ -1292,6 +1274,25 @@ void NativeWindowViews::SetIcon(const gfx::ImageSkia& icon) { } #endif +#if defined(USE_X11) +void NativeWindowViews::SetGTKDarkThemeEnabled(bool use_dark_theme) { + XDisplay* xdisplay = gfx::GetXDisplay(); + if (use_dark_theme) { + XChangeProperty(xdisplay, GetAcceleratedWidget(), + XInternAtom(xdisplay, "_GTK_THEME_VARIANT", x11::False), + XInternAtom(xdisplay, "UTF8_STRING", x11::False), 8, + PropModeReplace, + reinterpret_cast("dark"), 4); + } else { + XChangeProperty(xdisplay, GetAcceleratedWidget(), + XInternAtom(xdisplay, "_GTK_THEME_VARIANT", x11::False), + XInternAtom(xdisplay, "UTF8_STRING", x11::False), 8, + PropModeReplace, + reinterpret_cast("light"), 5); + } +} +#endif + void NativeWindowViews::OnWidgetActivationChanged(views::Widget* changed_widget, bool active) { if (changed_widget != widget()) diff --git a/shell/browser/native_window_views.h b/shell/browser/native_window_views.h index d4eab6dfaee..a330bed8e14 100644 --- a/shell/browser/native_window_views.h +++ b/shell/browser/native_window_views.h @@ -127,8 +127,6 @@ class NativeWindowViews : public NativeWindow, bool IsVisibleOnAllWorkspaces() override; - void SetGTKDarkThemeEnabled(bool use_dark_theme) override; - content::DesktopMediaID GetDesktopMediaID() const override; gfx::AcceleratedWidget GetAcceleratedWidget() const override; NativeWindowHandle GetNativeWindowHandle() const override; @@ -157,6 +155,10 @@ class NativeWindowViews : public NativeWindow, void SetIcon(const gfx::ImageSkia& icon); #endif +#if defined(USE_X11) + void SetGTKDarkThemeEnabled(bool use_dark_theme); +#endif + SkRegion* draggable_region() const { return draggable_region_.get(); } #if defined(OS_WIN)