-
Notifications
You must be signed in to change notification settings - Fork 15.3k
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: add option to not transform processes on win.SetVisibleOnAllWorkspaces #27200
feat: add option to not transform processes on win.SetVisibleOnAllWorkspaces #27200
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. |
09519a5
to
e8cf0c7
Compare
This PR requires approval from @electron/wg-api since it adds a new option. |
Sweet, thank you! Let me know if there's anything I can do to help. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding this option looks good to me, but the documentation needs to be clearer as to the functionality it provides
docs/api/browser-window.md
Outdated
@@ -1626,6 +1626,8 @@ Returns `Boolean` - Whether the menu bar is visible. | |||
* `options` Object (optional) | |||
* `visibleOnFullScreen` Boolean (optional) _macOS_ - Sets whether | |||
the window should be visible above fullscreen windows | |||
* `skipTransformProcessType` Boolean (optional) _macOS_ - Sets whether | |||
the process type should be transformed. Default is `false` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While functionally true, this description doesn't seem really helpful to developers. Can you reword to provide better insight to developers as to why you would want to set this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! How does the updated description sound?
84f4a3d
to
eb44963
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CyrusRoshan the updated description is a little more helpful but it is still unclear. I'm still not 100% clear why we are changing the process type and I think that's the part that needs clarifying. Maybe including more context from #27101 would be helpful. I'm trying to think of developers who don't have the context that you do if that makes sense.
docs/api/browser-window.md
Outdated
* `skipTransformProcessType` Boolean (optional) _macOS_ - Transforming | ||
the process type must hide the window and dock for a short time. This | ||
can be skipped if the process type has already been transformed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* `skipTransformProcessType` Boolean (optional) _macOS_ - Transforming | |
the process type must hide the window and dock for a short time. This | |
can be skipped if the process type has already been transformed. | |
* `skipTransformProcessType` Boolean (optional) _macOS_ - By default on macOS, setVisibleOnAllWorkspaces transforms the process type in order to hide the window and dock for a short time. If this is undesirable, setting this value to true will override that behavior. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CyrusRoshan thanks for updating the description. It is much clearer now what this option does.
…on macOS, making it backwards-compatible with v9.2.1 (electron#27101)
…on macOS, making it backwards-compatible with v9.2.1 (electron#27101)
a7dddeb
to
f4c2e84
Compare
This PR has got enough approvals from members of wg-api, it is a bug of bot that |
Congrats on merging your first pull request! 🎉🎉🎉 |
Release Notes Persisted
|
Description of Change
Allows calls to win.SetVisibleOnAllWorkspaces to skip transforming the process type. This allows us to use the same behavior found in Electron versions 9.2.1 and under, which is useful for "pinning" an app to a workspace/desktop, without causing other issues related to process transformation.
This PR adds optional backwards-compatibility with 9.2.1 and under, while maintaining the same default behavior.
Fixes: (#27101)
Process transformation introduced in this PR: PR #24956
Stakeholders:
@codebytere
@zcbenz
@MarshallOfSound
Checklist
npm test
passesRelease Notes
Notes: Allowed skipping process type transformation in win.SetVisibleOnAllWorkspaces on macOS.