feat: add panel support for BrowserWindow #34388
Conversation
|
💖 Thanks for opening this pull request! 💖 We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix. Examples of commit messages with semantic prefixes:
Things that will help get your PR across the finish line:
We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can. |
There was a problem hiding this comment.
Hi! Thanks for opening this PR - however, all new PRs, especially semver-minor or major PRs, must target main. Only then, when it's reviewed and merged, can it be backported to version branches.
Could you also talk a bit more in your PR body about the impetus for this PR? What use cases are you trying to address?
|
I really like this implementation of panels overall, as it's similar to third-party solutions but safer and cleaner. Pros
Cons
Non-activating panels are such a useful and desirable part of modern apps (floating palettes are everywhere!) and I think the risk is manageable. Still, it would be great to work toward replacing this implementation with a real NSPanel in the long term. If done right it would be a transparent change to users who adopt this API in its current form. |
c2a5559 to
6ef9d10
Compare
6ef9d10 to
64e1e71
Compare
|
Hey @codebytere I changed the target branch to "main" as suggested and changed the wording in browser-window to make it more clear what this new type achieves i.e makes the window float on top of full-screened apps. As per @mitchchn's comment, this pull request addresses a feature which has been previously requested. This is still obviously not the ideal solution, but a workaround. |
Co-authored-by: Cheng Zhao <github@zcbenz.com>
Co-authored-by: Cheng Zhao <github@zcbenz.com>
|
Hey all, We started using this patch internally and found a weird bug. If we toggle the "resizable" property on BrowserWindow with type = "panel" the height of the window increases by 28 points (which is height of titlebar on Mac). I confirmed in Xcode that this is not a bug on the native implementation, so something is going wrong when SetCanResize and then Widget->OnSizeConstraintChanged() is called in chromium. I'm still looking into this, but wanted to share here in case anyone has experience with this and can pinpoint the issue more easily. Screen recording showing the issue - google drive link Fiddle code to repro: const mainWindow = new BrowserWindow({ let resizable = true; |
|
@isailaandrei Can you try placing |
|
Congrats on merging your first pull request! 🎉🎉🎉 |
|
Release Notes Persisted
|
|
/trop run backport-to 20-x-y |
|
The backport process for this PR has been manually initiated - sending your PR to |
|
I have automatically backported this PR to "20-x-y", please check out #34665 |
|
Hey folks! A question: this implementation indeed makes the window/panel appear without taking focus. But it now also can't react to hover events anymore... 🤔 Would you know which additional properties would do this? Best, |
* feat: add NSPanel support for BrowserWindow * change header guard to satisfy linter * change panel wording in browser-window * Revert "change panel wording in browser-window" This reverts commit 6f3f80f. * change wording in browser-window * Update shell/browser/ui/cocoa/electron_native_widget_mac.mm Co-authored-by: Cheng Zhao <github@zcbenz.com> * Update shell/browser/ui/cocoa/electron_native_widget_mac.h Co-authored-by: Cheng Zhao <github@zcbenz.com> * Changed ScopedDisableResize class to allow for nesting Co-authored-by: andreiisaila <andreiisaila@microsoft.com> Co-authored-by: Cheng Zhao <github@zcbenz.com>
|
NSWindow does not support nonactivating panel styleMask 0x80 |
1 similar comment
|
NSWindow does not support nonactivating panel styleMask 0x80 |
|
Hello, Is it normal that the official doc has no information about type: "panel" ? I spent lot of time figuring how to do that :-) |
Agree. It seems it's documented in https://www.electronjs.org/docs/latest/api/structures/base-window-options and even then it's hard to find, since one wouldn't know up-front this kind of window is a "panel". Maybe adding a corresponding section within https://www.electronjs.org/docs/latest/tutorial/window-customization would help with discoverability here? |
Improve discoverability of panel feature impl in electron#34388
|
Also @isailaandrei what's the expected behavior of this |
|
On close inspection this does not seem to be working for me (I was confused because I still had other options that made the window look like a panel ON). I'm seeing this when running a toy example: Any hints? I'm on Sonoma 14.5 |
|
We also have this message and it works fine, it's just informative. |
|
After some days using this, it seems it generally works although still it has some rough edges. @isailaandrei are there any ideas in the horizon from initial discussions on how to achieve a more panel-like identity...? |
Description of Change
Created a new ElectronNSPanel window which inherits from ElectronNSWindow. This window mimics the behavior of NSPanel - visible on top of full screened apps, joins all spaces. To achieve this it adds the "NSWindowStyleMaskNonactivatingPanel" style mask, normally reserved for NSPanel, at runtime.
This is a workaround to avoid patching chromium and duplicating lots of window code.
Checklist
npm testpassesRelease Notes
notes: Added support for panel-like behavior. Window can float over full-screened apps.