Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: enable window controls overlay on macOS #29253

Merged
merged 12 commits into from Jul 1, 2021
Merged

Conversation

jkleinsc
Copy link
Contributor

@jkleinsc jkleinsc commented May 20, 2021

Description of Change

This PR adds a new property titleBarOverlay to BrowserWindow on macOS to support Window Controls Overlay. On macOS, when using a frameless window in conjunction with win.setWindowButtonVisibility(true) or using a titleBarStyle so that the traffic lights are visible, this property enables the Window Controls Overlay JavaScript APIs and CSS Environment Variables will be available for developers to use to better style custom titlebars.

There will be another PR forthcoming that will enable this capability on Windows. Windows needs NativeWindow::SetWindowControlsOverlayRect(const gfx::Rect& overlay_rect), but this newly added method is not used on macOS.

Checklist

Release Notes

Notes: Added support for Windows Control Overlay on macOS.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label May 20, 2021
@jkleinsc jkleinsc added target/13-x-y semver/minor backwards-compatible functionality labels May 20, 2021
Copy link
Member

@zcbenz zcbenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NotifyLayoutWindowControlsOverlay should also be called when SetWindowButtonVisibility and SetTrafficLightPosition are called.

shell/browser/native_window.cc Show resolved Hide resolved
shell/browser/native_window_mac.mm Outdated Show resolved Hide resolved
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label May 27, 2021
@jkleinsc jkleinsc requested a review from zcbenz May 27, 2021 18:30
shell/browser/native_window_observer.h Outdated Show resolved Hide resolved
docs/api/browser-window.md Outdated Show resolved Hide resolved
shell/browser/native_window_mac.mm Outdated Show resolved Hide resolved
shell/browser/native_window_mac.mm Outdated Show resolved Hide resolved
shell/browser/native_window_mac.mm Outdated Show resolved Hide resolved
shell/browser/native_window_observer.h Outdated Show resolved Hide resolved
docs/api/frameless-window.md Outdated Show resolved Hide resolved
@jkleinsc jkleinsc requested a review from zcbenz June 9, 2021 17:55
@MarshallOfSound MarshallOfSound changed the title feat: enable windows control overlay on macOS feat: enable window controls overlay on macOS Jun 9, 2021
@zcbenz
Copy link
Member

zcbenz commented Jun 10, 2021

I'm not sure whether this needs API approval, since it implements an experimental feature of Chromium, but the API looks good to me anyway.

@electron/wg-api

Copy link
Member

@MarshallOfSound MarshallOfSound left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes just so this doesn't land in the current state, we had some offline discussions mostly revolving around the large quantity of copied code in the windows PR but both PRs should be updated to only take effect when a new "overlay" tile bar style is defined. This should not affect any existing apps.

@nornagon
Copy link
Member

I think the only wg-api-relevant consideration here is whether this has the potential to break existing use cases. Eventually the JS/CSS parts of this feature will be available without the blink feature flag, so it's worth considering whether a developer would want to have a hidden titlebar style without allowing the JS to set titlebar properties. I can't really imagine a compelling use case for that, so I'm 👍 from an API perspective.

shell/browser/native_window_mac.mm Outdated Show resolved Hide resolved
jkleinsc and others added 3 commits June 30, 2021 10:14
Co-authored-by: Samuel Attard <sam@electronjs.org>
Co-authored-by: Samuel Attard <sam@electronjs.org>
@jkleinsc jkleinsc merged commit 1f8a46c into main Jul 1, 2021
@jkleinsc jkleinsc deleted the title-overlay-macos branch July 1, 2021 19:25
@release-clerk
Copy link

release-clerk bot commented Jul 1, 2021

Release Notes Persisted

Added support for Windows Control Overlay on macOS.

@trop
Copy link
Contributor

trop bot commented Jul 1, 2021

I was unable to backport this PR to "13-x-y" cleanly;
you will need to perform this backport manually.

@trop
Copy link
Contributor

trop bot commented Jul 1, 2021

I have automatically backported this PR to "14-x-y", please check out #29986

jkleinsc added a commit that referenced this pull request Aug 24, 2021
* feat: enable windows control overlay on macOS

* address review feedback

* chore: address review feedback

* Address review feedback

* update doc per review

* only enable WCO when titleBarStyle is overlay

* Revert "only enable WCO when titleBarStyle is overlay"

This reverts commit 1b58b5b.

* Add new titleBarOverlay property to manage feature

* spelling fix

* Update docs/api/frameless-window.md

Co-authored-by: Samuel Attard <sam@electronjs.org>

* Update shell/browser/api/electron_api_browser_window.cc

Co-authored-by: Samuel Attard <sam@electronjs.org>

* update per review feedback

Co-authored-by: Samuel Attard <sam@electronjs.org>
(cherry picked from commit 1f8a46c)
@trop
Copy link
Contributor

trop bot commented Aug 25, 2021

@jkleinsc has manually backported this PR to "13-x-y", please check out #30698

BlackHole1 pushed a commit to BlackHole1/electron that referenced this pull request Aug 30, 2021
* feat: enable windows control overlay on macOS

* address review feedback

* chore: address review feedback

* Address review feedback

* update doc per review

* only enable WCO when titleBarStyle is overlay

* Revert "only enable WCO when titleBarStyle is overlay"

This reverts commit 1b58b5b.

* Add new titleBarOverlay property to manage feature

* spelling fix

* Update docs/api/frameless-window.md

Co-authored-by: Samuel Attard <sam@electronjs.org>

* Update shell/browser/api/electron_api_browser_window.cc

Co-authored-by: Samuel Attard <sam@electronjs.org>

* update per review feedback

Co-authored-by: Samuel Attard <sam@electronjs.org>
mlaurencin pushed a commit that referenced this pull request Sep 15, 2021
* feat: enable windows control overlay on macOS

* address review feedback

* chore: address review feedback

* Address review feedback

* update doc per review

* only enable WCO when titleBarStyle is overlay

* Revert "only enable WCO when titleBarStyle is overlay"

This reverts commit 1b58b5b.

* Add new titleBarOverlay property to manage feature

* spelling fix

* Update docs/api/frameless-window.md

Co-authored-by: Samuel Attard <sam@electronjs.org>

* Update shell/browser/api/electron_api_browser_window.cc

Co-authored-by: Samuel Attard <sam@electronjs.org>

* update per review feedback

Co-authored-by: Samuel Attard <sam@electronjs.org>
(cherry picked from commit 1f8a46c)
zcbenz pushed a commit that referenced this pull request Sep 16, 2021
* feat: enable windows control overlay on macOS

* address review feedback

* chore: address review feedback

* Address review feedback

* update doc per review

* only enable WCO when titleBarStyle is overlay

* Revert "only enable WCO when titleBarStyle is overlay"

This reverts commit 1b58b5b.

* Add new titleBarOverlay property to manage feature

* spelling fix

* Update docs/api/frameless-window.md

Co-authored-by: Samuel Attard <sam@electronjs.org>

* Update shell/browser/api/electron_api_browser_window.cc

Co-authored-by: Samuel Attard <sam@electronjs.org>

* update per review feedback

Co-authored-by: Samuel Attard <sam@electronjs.org>
(cherry picked from commit 1f8a46c)
deepak1556 pushed a commit that referenced this pull request Sep 23, 2021
* feat: enable window controls overlay on macOS (#29253)

* feat: enable windows control overlay on macOS

* address review feedback

* chore: address review feedback

* Address review feedback

* update doc per review

* only enable WCO when titleBarStyle is overlay

* Revert "only enable WCO when titleBarStyle is overlay"

This reverts commit 1b58b5b.

* Add new titleBarOverlay property to manage feature

* spelling fix

* Update docs/api/frameless-window.md

Co-authored-by: Samuel Attard <sam@electronjs.org>

* Update shell/browser/api/electron_api_browser_window.cc

Co-authored-by: Samuel Attard <sam@electronjs.org>

* update per review feedback

Co-authored-by: Samuel Attard <sam@electronjs.org>
(cherry picked from commit 1f8a46c)

* feat: enable windows control overlay on Windows (#30678)

cherry-picked from 41646d1

Co-Authored-By: Michaela Laurencin <35157522+mlaurencin@users.noreply.github.com>

Co-authored-by: Michaela Laurencin <35157522+mlaurencin@users.noreply.github.com>

* modify included header files and update patches

* kick off missed ci

* fix lint error

* chore: update patches

* chore: update patches

* remove version control marker

* correct `resizeable_` backport

Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Co-authored-by: Cheng Zhao <zcbenz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-review/approved ✅ semver/minor backwards-compatible functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants