feat: make trafficLightPosition work for customButtonOnHover (electron#26789)

Author: zcbenz

docs/api/browser-window.md

@@ -227,7 +227,9 @@ It creates a new `BrowserWindow` with native properties as set by the `options`.
       unless hovered over in the top left of the window. These custom buttons prevent
       issues with mouse events that occur with the standard window toolbar buttons.
       **Note:** This option is currently experimental.
-  * `trafficLightPosition` [Point](structures/point.md) (optional) - Set a custom position for the traffic light buttons. Can only be used with `titleBarStyle` set to `hidden`
+  * `trafficLightPosition` [Point](structures/point.md) (optional) - Set a
+    custom position for the traffic light buttons. Can only be used with
+    `titleBarStyle` set to `hidden` or `customButtonsOnHover`.
   * `fullscreenWindowTitle` Boolean (optional) - Shows the title in the
     title bar in full screen mode on macOS for all `titleBarStyle` options.
     Default is `false`.
@@ -1737,12 +1739,13 @@ deprecated and will be removed in an upcoming version of macOS.
 
 * `position` [Point](structures/point.md)
 
-Set a custom position for the traffic light buttons. Can only be used with `titleBarStyle` set to `hidden`.
+Set a custom position for the traffic light buttons. Can only be used with
+`titleBarStyle` set to `hidden` or `customButtonsOnHover`.
 
 #### `win.getTrafficLightPosition()` _macOS_
 
-Returns `Point` - The current position for the traffic light buttons. Can only be used with `titleBarStyle`
-set to `hidden`.
+Returns `Point` - The current position for the traffic light buttons. Can only
+be used with `titleBarStyle` set to `hidden` or `customButtonsOnHover`.
 
 #### `win.setTouchBar(touchBar)` _macOS_
 

shell/browser/api/electron_api_base_window.cc

@@ -842,11 +842,16 @@ void BaseWindow::SetVibrancy(v8::Isolate* isolate, v8::Local<v8::Value> value) {
 
 #if defined(OS_MAC)
 void BaseWindow::SetTrafficLightPosition(const gfx::Point& position) {
-  window_->SetTrafficLightPosition(position);
+  // For backward compatibility we treat (0, 0) as reseting to default.
+  if (position.IsOrigin())
+    window_->SetTrafficLightPosition(base::nullopt);
+  else
+    window_->SetTrafficLightPosition(position);
 }
 
 gfx::Point BaseWindow::GetTrafficLightPosition() const {
-  return window_->GetTrafficLightPosition();
+  // For backward compatibility we treat default value as (0, 0).
+  return window_->GetTrafficLightPosition().value_or(gfx::Point());
 }
 #endif
 

shell/browser/native_window.h

@@ -14,6 +14,7 @@
 
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
+#include "base/optional.h"
 #include "base/strings/string16.h"
 #include "base/supports_user_data.h"
 #include "base/values.h"
@@ -198,8 +199,8 @@ class NativeWindow : public base::SupportsUserData,
 
   // Traffic Light API
 #if defined(OS_MAC)
-  virtual void SetTrafficLightPosition(const gfx::Point& position) = 0;
-  virtual gfx::Point GetTrafficLightPosition() const = 0;
+  virtual void SetTrafficLightPosition(base::Optional<gfx::Point> position) = 0;
+  virtual base::Optional<gfx::Point> GetTrafficLightPosition() const = 0;
   virtual void RedrawTrafficLights() = 0;
 #endif
 

shell/browser/native_window_mac.h

@@ -159,8 +159,8 @@ class NativeWindowMac : public NativeWindow, public ui::NativeThemeObserver {
   // Custom traffic light positioning
   void RedrawTrafficLights() override;
   void SetExitingFullScreen(bool flag);
-  void SetTrafficLightPosition(const gfx::Point& position) override;
-  gfx::Point GetTrafficLightPosition() const override;
+  void SetTrafficLightPosition(base::Optional<gfx::Point> position) override;
+  base::Optional<gfx::Point> GetTrafficLightPosition() const override;
   void OnNativeThemeUpdated(ui::NativeTheme* observed_theme) override;
 
   enum class VisualEffectState {
@@ -220,7 +220,7 @@ class NativeWindowMac : public NativeWindow, public ui::NativeThemeObserver {
   bool fullscreen_window_title_ = false;
   bool resizable_ = true;
   bool exiting_fullscreen_ = false;
-  gfx::Point traffic_light_position_;
+  base::Optional<gfx::Point> traffic_light_position_;
 
   NSInteger attention_request_id_ = 0;  // identifier from requestUserAttention
 

shell/browser/native_window_mac.mm

@@ -124,13 +124,18 @@ - (void)viewDidMoveToSuperview {
 @interface CustomWindowButtonView : NSView {
  @private
   BOOL mouse_inside_;
+  gfx::Point margin_;
 }
+
+- (id)initWithMargin:(const base::Optional<gfx::Point>&)margin;
+- (void)setMargin:(const base::Optional<gfx::Point>&)margin;
 @end
 
 @implementation CustomWindowButtonView
 
-- (id)initWithFrame:(NSRect)frame {
-  self = [super initWithFrame:frame];
+- (id)initWithMargin:(const base::Optional<gfx::Point>&)margin {
+  self = [super initWithFrame:NSZeroRect];
+  [self setMargin:margin];
 
   NSButton* close_button =
       [NSWindow standardWindowButton:NSWindowCloseButton
@@ -163,18 +168,20 @@ - (id)initWithFrame:(NSRect)frame {
   return self;
 }
 
+- (void)setMargin:(const base::Optional<gfx::Point>&)margin {
+  margin_ = margin.value_or(gfx::Point(7, 3));
+}
+
 - (void)viewDidMoveToWindow {
   if (!self.window) {
     return;
   }
 
   // Stay in upper left corner.
-  const CGFloat top_margin = 3;
-  const CGFloat left_margin = 7;
   [self setAutoresizingMask:NSViewMaxXMargin | NSViewMinYMargin];
-  [self setFrameOrigin:NSMakePoint(left_margin, self.window.frame.size.height -
+  [self setFrameOrigin:NSMakePoint(margin_.x(), self.window.frame.size.height -
                                                     self.frame.size.height -
-                                                    top_margin)];
+                                                    margin_.y())];
 }
 
 - (BOOL)_mouseInGroup:(NSButton*)button {
@@ -365,7 +372,7 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) {
   options.Get(options::kZoomToPageWidth, &zoom_to_page_width_);
   options.Get(options::kFullscreenWindowTitle, &fullscreen_window_title_);
   options.Get(options::kSimpleFullScreen, &always_simple_fullscreen_);
-  options.Get(options::kTrafficLightPosition, &traffic_light_position_);
+  options.GetOptional(options::kTrafficLightPosition, &traffic_light_position_);
   options.Get(options::kVisualEffectState, &visual_effect_state_);
 
   bool minimizable = true;
@@ -543,9 +550,10 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) {
   // Ensure maximizable options retain pre-existing state.
   SetMaximizable(maximizable_);
 
-  if (!traffic_light_position_.x() && !traffic_light_position_.y()) {
+  // Changing system titlebar is only allowed for "hidden" titleBarStyle.
+  if (!traffic_light_position_ || title_bar_style_ != TitleBarStyle::kHidden)
     return;
-  }
+
   if (IsFullscreen())
     return;
 
@@ -571,7 +579,7 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) {
 
   [titleBarContainerView setHidden:NO];
   CGFloat buttonHeight = [close frame].size.height;
-  CGFloat titleBarFrameHeight = buttonHeight + traffic_light_position_.y();
+  CGFloat titleBarFrameHeight = buttonHeight + traffic_light_position_->y();
   CGRect titleBarRect = titleBarContainerView.frame;
   CGFloat titleBarWidth = NSWidth(titleBarRect);
   titleBarRect.size.height = titleBarFrameHeight;
@@ -589,10 +597,10 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) {
     if (isRTL) {
       CGFloat buttonWidth = NSWidth(rect);
       // origin is always top-left, even in RTL
-      rect.origin.x = titleBarWidth - traffic_light_position_.x() +
+      rect.origin.x = titleBarWidth - traffic_light_position_->x() +
                       (i * space_between) - buttonWidth;
     } else {
-      rect.origin.x = traffic_light_position_.x() + (i * space_between);
+      rect.origin.x = traffic_light_position_->x() + (i * space_between);
     }
     rect.origin.y = (titleBarFrameHeight - rect.size.height) / 2;
     [view setFrameOrigin:rect.origin];
@@ -1588,12 +1596,18 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) {
     [effect_view setMaterial:vibrancyType];
 }
 
-void NativeWindowMac::SetTrafficLightPosition(const gfx::Point& position) {
-  traffic_light_position_ = position;
-  RedrawTrafficLights();
+void NativeWindowMac::SetTrafficLightPosition(
+    base::Optional<gfx::Point> position) {
+  traffic_light_position_ = std::move(position);
+  if (title_bar_style_ == TitleBarStyle::kHidden) {
+    RedrawTrafficLights();
+  } else if (title_bar_style_ == TitleBarStyle::kCustomButtonsOnHover) {
+    [buttons_view_ setMargin:position];
+    [buttons_view_ viewDidMoveToWindow];
+  }
 }
 
-gfx::Point NativeWindowMac::GetTrafficLightPosition() const {
+base::Optional<gfx::Point> NativeWindowMac::GetTrafficLightPosition() const {
   return traffic_light_position_;
 }
 
@@ -1695,8 +1709,8 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) {
 
     // Create a custom window buttons view for kCustomButtonsOnHover.
     if (title_bar_style_ == TitleBarStyle::kCustomButtonsOnHover) {
-      buttons_view_.reset(
-          [[CustomWindowButtonView alloc] initWithFrame:NSZeroRect]);
+      buttons_view_.reset([[CustomWindowButtonView alloc]
+          initWithMargin:traffic_light_position_]);
 
       if (!minimizable)
         [[buttons_view_ viewWithTag:2] removeFromSuperview];

spec-main/api-browser-window-spec.ts

@@ -1536,28 +1536,36 @@ describe('BrowserWindow module', () => {
     });
   });
 
-  ifdescribe(process.platform === 'darwin')('BrowserWindow.getTrafficLightPosition(pos)', () => {
+  ifdescribe(process.platform === 'darwin')('trafficLightPosition', () => {
+    const pos = { x: 10, y: 10 };
     afterEach(closeAllWindows);
 
-    it('gets the set traffic light position property', () => {
-      const pos = { x: 10, y: 10 };
-      const w = new BrowserWindow({ show: false, titleBarStyle: 'hidden', trafficLightPosition: pos });
-      const currentPosition = w.getTrafficLightPosition();
+    describe('BrowserWindow.getTrafficLightPosition(pos)', () => {
+      it('gets position property for "hidden" titleBarStyle', () => {
+        const w = new BrowserWindow({ show: false, titleBarStyle: 'hidden', trafficLightPosition: pos });
+        expect(w.getTrafficLightPosition()).to.deep.equal(pos);
+      });
 
-      expect(currentPosition).to.deep.equal(pos);
+      it('gets position property for "customButtonsOnHover" titleBarStyle', () => {
+        const w = new BrowserWindow({ show: false, titleBarStyle: 'customButtonsOnHover', trafficLightPosition: pos });
+        expect(w.getTrafficLightPosition()).to.deep.equal(pos);
+      });
     });
-  });
 
-  ifdescribe(process.platform === 'darwin')('BrowserWindow.setTrafficLightPosition(pos)', () => {
-    afterEach(closeAllWindows);
-
-    it('can set the traffic light position property', () => {
-      const pos = { x: 10, y: 10 };
-      const w = new BrowserWindow({ show: false, titleBarStyle: 'hidden', trafficLightPosition: pos });
-      w.setTrafficLightPosition(pos);
-      const currentPosition = w.getTrafficLightPosition();
+    describe('BrowserWindow.setTrafficLightPosition(pos)', () => {
+      it('sets position property for "hidden" titleBarStyle', () => {
+        const w = new BrowserWindow({ show: false, titleBarStyle: 'hidden', trafficLightPosition: pos });
+        const newPos = { x: 20, y: 20 };
+        w.setTrafficLightPosition(newPos);
+        expect(w.getTrafficLightPosition()).to.deep.equal(newPos);
+      });
 
-      expect(currentPosition).to.deep.equal(pos);
+      it('sets position property for "customButtonsOnHover" titleBarStyle', () => {
+        const w = new BrowserWindow({ show: false, titleBarStyle: 'customButtonsOnHover', trafficLightPosition: pos });
+        const newPos = { x: 20, y: 20 };
+        w.setTrafficLightPosition(newPos);
+        expect(w.getTrafficLightPosition()).to.deep.equal(newPos);
+      });
     });
   });