feat: enable customizing symbolColor

electron · Apr 5, 2024 · 056e89a · 056e89a
1 parent b0230d2
commit 056e89a
Show file tree

Hide file tree

Showing 6 changed files with 105 additions and 14 deletions.
diff --git a/docs/api/base-window.md b/docs/api/base-window.md
@@ -1374,13 +1374,12 @@ removed in future Electron releases.
 
 * `options` Object
   * `color` String (optional) - The CSS color of the Window Controls Overlay when enabled.
-  * `symbolColor` String (optional) _Windows_ - The CSS color of the symbols on the Window Controls Overlay when enabled.
+  * `symbolColor` String (optional) - The CSS color of the symbols on the Window Controls Overlay when enabled.
   * `height` Integer (optional) - The height of the title bar and Window Controls Overlay in pixels.
 
-On a Window with Window Controls Overlay already enabled, this method updates
-the style of the title bar overlay.
+On a Window with Window Controls Overlay already enabled, this method updates the style of the title bar overlay.
 
-On Linux, the `symbolColor` is automatically calculated to have minimum accessible contrast to the `color`.
+On Linux, the `symbolColor` is automatically calculated to have minimum accessible contrast to the `color` if not explicitly set.
 
 [quick-look]: https://en.wikipedia.org/wiki/Quick_Look
 [vibrancy-docs]: https://developer.apple.com/documentation/appkit/nsvisualeffectview?preferredLanguage=objc

diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md
@@ -1645,13 +1645,12 @@ with `addBrowserView` or `setBrowserView`. The top-most BrowserView is the last
 
 * `options` Object
   * `color` String (optional) - The CSS color of the Window Controls Overlay when enabled.
-  * `symbolColor` String (optional) _Windows_ - The CSS color of the symbols on the Window Controls Overlay when enabled.
+  * `symbolColor` String (optional) - The CSS color of the symbols on the Window Controls Overlay when enabled.
   * `height` Integer (optional) - The height of the title bar and Window Controls Overlay in pixels.
 
-On a window with Window Controls Overlay already enabled, this method updates
-the style of the title bar overlay.
+On a window with Window Controls Overlay already enabled, this method updates the style of the title bar overlay.
 
-On Linux, the `symbolColor` is automatically calculated to have minimum accessible contrast to the `color`.
+On Linux, the `symbolColor` is automatically calculated to have minimum accessible contrast to the `color` if not explicitly set.
 
 [page-visibility-api]: https://developer.mozilla.org/en-US/docs/Web/API/Page_Visibility_API
 [quick-look]: https://en.wikipedia.org/wiki/Quick_Look

diff --git a/patches/chromium/.patches b/patches/chromium/.patches
@@ -129,3 +129,4 @@ build_run_reclient_cfg_generator_after_chrome.patch
 fix_suppress_clang_-wimplicit-const-int-float-conversion_in.patch
 fix_getcursorscreenpoint_wrongly_returns_0_0.patch
 fix_add_support_for_skipping_first_2_no-op_refreshes_in_thumb_cap.patch
+feat_enable_customizing_symbol_color_in_framecaptionbutton.patch
diff --git a/patches/chromium/feat_enable_customizing_symbol_color_in_framecaptionbutton.patch b/patches/chromium/feat_enable_customizing_symbol_color_in_framecaptionbutton.patch
@@ -0,0 +1,80 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Shelley Vohr <shelley.vohr@gmail.com>
+Date: Fri, 5 Apr 2024 11:07:22 +0200
+Subject: feat: enable customizing symbol color in FrameCaptionButton
+
+This enables customizing the symbol color on a given FrameCaptionButton
+for the Window Controls Overlay API on Linux. By default, the symbol color
+is dynamically calculated based on the background color of the button to
+ensure it has minimum contrast required to be accessible.
+
+This should be upstreamed to Chromium if possible.
+
+diff --git a/ui/views/window/frame_caption_button.cc b/ui/views/window/frame_caption_button.cc
+index 73e6020e3b9b6e0d12a8dea991f189b3ddeab14c..b38e5bd1408c26cbbfc995fc2ac5dc5983cc0db7 100644
+--- a/ui/views/window/frame_caption_button.cc
++++ b/ui/views/window/frame_caption_button.cc
+@@ -107,7 +107,7 @@ FrameCaptionButton::FrameCaptionButton(PressedCallback callback,
+ FrameCaptionButton::~FrameCaptionButton() = default;
+
+ // static
+-SkColor FrameCaptionButton::GetButtonColor(SkColor background_color) {
++SkColor FrameCaptionButton::GetAccessibleButtonColor(SkColor background_color) {
+   // Use IsDark() to change target colors instead of PickContrastingColor(), so
+   // that DefaultFrameHeader::GetTitleColor() (which uses different target
+   // colors) can change between light/dark targets at the same time.  It looks
+@@ -124,6 +124,22 @@ SkColor FrameCaptionButton::GetButtonColor(SkColor background_color) {
+       .color;
+ }
+
++SkColor FrameCaptionButton::GetButtonColor(SkColor background_color) {
++  // If the button color has been overridden, return that.
++  if (button_color_ != SkColor())
++    return button_color_;
++
++  return GetAccessibleButtonColor(background_color);
++}
++
++void FrameCaptionButton::SetButtonColor(SkColor button_color) {
++  if (button_color_ == button_color)
++    return;
++
++  button_color_ = button_color;
++  MaybeRefreshIconAndInkdropBaseColor();
++}
++
+ // static
+ float FrameCaptionButton::GetInactiveButtonColorAlphaRatio() {
+   return 0.38f;
+diff --git a/ui/views/window/frame_caption_button.h b/ui/views/window/frame_caption_button.h
+index 0d20ec5891d08187b4dae7a6b46a9a6de2f7c39c..eb396811a39c4e6021916dbba1f89baa83d77d31 100644
+--- a/ui/views/window/frame_caption_button.h
++++ b/ui/views/window/frame_caption_button.h
+@@ -43,8 +43,18 @@ class VIEWS_EXPORT FrameCaptionButton : public Button {
+   FrameCaptionButton& operator=(const FrameCaptionButton&) = delete;
+   ~FrameCaptionButton() override;
+
++  // Gets the color to use for a frame caption button with accessible contrast
++  // to the given background color.
++  static SkColor GetAccessibleButtonColor(SkColor background_color);
++
+   // Gets the color to use for a frame caption button.
+-  static SkColor GetButtonColor(SkColor background_color);
++  SkColor GetButtonColor(SkColor background_color);
++
++  // Sets the color to use for a frame caption button.
++  // The color is by default calculated to be an accessible contrast
++  // to the background color, so you should keep that in mind when
++  // overriding that behavior.
++  void SetButtonColor(SkColor button_color);
+
+   // Gets the alpha ratio for the colors of inactive frame caption buttons.
+   static float GetInactiveButtonColorAlphaRatio();
+@@ -133,6 +143,7 @@ class VIEWS_EXPORT FrameCaptionButton : public Button {
+   // TODO(b/292154873): Store the foreground color instead of the background
+   // color for the SkColor type.
+   absl::variant<ui::ColorId, SkColor> color_ = gfx::kPlaceholderColor;
++  SkColor button_color_ = SkColor();
+
+   // Whether the button should be painted as active.
+   bool paint_as_active_ = false;
diff --git a/shell/browser/ui/views/opaque_frame_view.cc b/shell/browser/ui/views/opaque_frame_view.cc
@@ -101,6 +101,7 @@ int OpaqueFrameView::ResizingBorderHitTest(const gfx::Point& point) {
 
 void OpaqueFrameView::InvalidateCaptionButtons() {
   UpdateCaptionButtonPlaceholderContainerBackground();
+  UpdateFrameCaptionButtons();
   LayoutWindowControlsOverlay();
   InvalidateLayout();
 }
@@ -204,10 +205,23 @@ void OpaqueFrameView::OnPaint(gfx::Canvas* canvas) {
   if (frame()->IsFullscreen())
     return;
 
+  UpdateFrameCaptionButtons();
+}
+
+void OpaqueFrameView::PaintAsActiveChanged() {
+  if (!window()->IsWindowControlsOverlayEnabled())
+    return;
+
+  UpdateCaptionButtonPlaceholderContainerBackground();
+  UpdateFrameCaptionButtons();
+}
+
+void OpaqueFrameView::UpdateFrameCaptionButtons() {
   const bool active = ShouldPaintAsActive();
-  const SkColor symbol_color = window()->overlay_button_color();
+  const SkColor symbol_color = window()->overlay_symbol_color();
+  const SkColor background_color = window()->overlay_button_color();
   SkColor frame_color =
-      symbol_color == SkColor() ? GetFrameColor() : symbol_color;
+      background_color == SkColor() ? GetFrameColor() : background_color;
 
   for (views::Button* button :
        {minimize_button_, maximize_button_, restore_button_, close_button_}) {
@@ -216,14 +230,11 @@ void OpaqueFrameView::OnPaint(gfx::Canvas* canvas) {
     views::FrameCaptionButton* frame_caption_button =
         static_cast<views::FrameCaptionButton*>(button);
     frame_caption_button->SetPaintAsActive(active);
+    frame_caption_button->SetButtonColor(symbol_color);
     frame_caption_button->SetBackgroundColor(frame_color);
   }
 }
 
-void OpaqueFrameView::PaintAsActiveChanged() {
-  UpdateCaptionButtonPlaceholderContainerBackground();
-}
-
 void OpaqueFrameView::UpdateCaptionButtonPlaceholderContainerBackground() {
   if (caption_button_placeholder_container_) {
     const SkColor obc = window()->overlay_button_color();

diff --git a/shell/browser/ui/views/opaque_frame_view.h b/shell/browser/ui/views/opaque_frame_view.h
@@ -70,6 +70,7 @@ class OpaqueFrameView : public FramelessView {
   void PaintAsActiveChanged();
 
   void UpdateCaptionButtonPlaceholderContainerBackground();
+  void UpdateFrameCaptionButtons();
   void LayoutWindowControls();
   void LayoutWindowControlsOverlay();