From e01b1831d96d5d68f54af879b00c617358df5372 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 16 Dec 2020 14:30:39 +0900 Subject: [PATCH] feat: make trafficLightPosition work for customButtonOnHover (#26789) --- docs/api/browser-window.md | 11 ++-- shell/browser/api/electron_api_base_window.cc | 9 +++- shell/browser/native_window.h | 5 +- shell/browser/native_window_mac.h | 6 +-- shell/browser/native_window_mac.mm | 50 ++++++++++++------- spec-main/api-browser-window-spec.ts | 40 +++++++++------ 6 files changed, 76 insertions(+), 45 deletions(-) diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index aa37bd2fdf59..7131d590e3a8 100644 --- a/docs/api/browser-window.md +++ b/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_ diff --git a/shell/browser/api/electron_api_base_window.cc b/shell/browser/api/electron_api_base_window.cc index 1314fd2bf1d4..40a33f210a6e 100644 --- a/shell/browser/api/electron_api_base_window.cc +++ b/shell/browser/api/electron_api_base_window.cc @@ -842,11 +842,16 @@ void BaseWindow::SetVibrancy(v8::Isolate* isolate, v8::Local 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 diff --git a/shell/browser/native_window.h b/shell/browser/native_window.h index ad5ccc35a4d9..7931936c29fa 100644 --- a/shell/browser/native_window.h +++ b/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 position) = 0; + virtual base::Optional GetTrafficLightPosition() const = 0; virtual void RedrawTrafficLights() = 0; #endif diff --git a/shell/browser/native_window_mac.h b/shell/browser/native_window_mac.h index 25435b1d9bab..7dde3d7e3529 100644 --- a/shell/browser/native_window_mac.h +++ b/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 position) override; + base::Optional 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 traffic_light_position_; NSInteger attention_request_id_ = 0; // identifier from requestUserAttention diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm index bc5bc2a0b231..211792133181 100644 --- a/shell/browser/native_window_mac.mm +++ b/shell/browser/native_window_mac.mm @@ -124,13 +124,18 @@ @interface CustomWindowButtonView : NSView { @private BOOL mouse_inside_; + gfx::Point margin_; } + +- (id)initWithMargin:(const base::Optional&)margin; +- (void)setMargin:(const base::Optional&)margin; @end @implementation CustomWindowButtonView -- (id)initWithFrame:(NSRect)frame { - self = [super initWithFrame:frame]; +- (id)initWithMargin:(const base::Optional&)margin { + self = [super initWithFrame:NSZeroRect]; + [self setMargin:margin]; NSButton* close_button = [NSWindow standardWindowButton:NSWindowCloseButton @@ -163,18 +168,20 @@ return self; } +- (void)setMargin:(const base::Optional&)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 @@ NativeWindowMac::NativeWindowMac(const gin_helper::Dictionary& options, 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 NativeWindowMac::RedrawTrafficLights() { // 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 NativeWindowMac::RedrawTrafficLights() { [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 NativeWindowMac::RedrawTrafficLights() { 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 NativeWindowMac::SetVibrancy(const std::string& type) { [effect_view setMaterial:vibrancyType]; } -void NativeWindowMac::SetTrafficLightPosition(const gfx::Point& position) { - traffic_light_position_ = position; - RedrawTrafficLights(); +void NativeWindowMac::SetTrafficLightPosition( + base::Optional 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 NativeWindowMac::GetTrafficLightPosition() const { return traffic_light_position_; } @@ -1695,8 +1709,8 @@ void NativeWindowMac::AddContentViewLayers(bool minimizable, bool closable) { // 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]; diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index 89902c7ed808..7c6d58c310e0 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/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); + 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); + }); - 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(); - - 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); + }); }); });