-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
macos: fix window.titleBarStyle
on macOS
#11584
Conversation
@msujew I'd like to know your thoughts since I believe you made the initial change. Should we apply the extra macOS handling at the preference level (and also benefit from not having to do a reload) or change the following line: theia/packages/core/src/electron-main/electron-main-application.ts Lines 502 to 503 in d27ebe7
to this.useNativeWindowFrame = isOSX || titleBarStyle === 'native'; |
We should also think of validating preference against theia/packages/core/src/electron-browser/window/electron-window-preferences.ts Lines 43 to 51 in d27ebe7
|
@vince-fugnitto I believe we should also perform the proposed change in the |
1a7c132
to
2d3a80d
Compare
@msujew thank you for the feedback, I made the update and now handle it in both places 👍 |
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.
I don't have a Mac at hand, but the changes looks reasonable to me 👍
We should probably fix the included
preference issue sometime.
The commit fixes the `window.titleBarStyle` handling on macOS which would otherwise cause the menu to be completely removed and difficult to recover for users. Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
2d3a80d
to
fe69456
Compare
@vince-fugnitto The latest change look better to me. I believe that we should be rather safe than sorry in this regard. Checking for |
What it does
Fixes: #11583
The commit fixes the handling when setting
window.titleBarStyle
tocustom
on macOS (which does not apply) as it has some negative side effects.How to test
titleBarStyle
and change fromnative
tocustom
Review checklist
Reminder for reviewers
Signed-off-by: vince-fugnitto vincent.fugnitto@ericsson.com