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

fix: Explicitally set "setTitlebarAppearsTransparent" #11164

Merged
merged 2 commits into from Nov 21, 2017

Conversation

Projects
None yet
5 participants
@felixrieseberg
Member

felixrieseberg commented Nov 19, 2017

I think this fixes #11150, but I know that it doesn't break anything.

@felixrieseberg felixrieseberg requested a review from electron/reviewers as a code owner Nov 19, 2017

@codebytere

Looks good to me!

@ckerr

This comment has been minimized.

Member

ckerr commented Nov 20, 2017

Looks harmless, but can we get a macOS tester to confirm the fix before we merge and close #11150?

@felixrieseberg

This comment has been minimized.

Member

felixrieseberg commented Nov 20, 2017

@ckerr I have a High Sierra machine at home, I'll verify it there later tonight.

@felixrieseberg

This comment has been minimized.

Member

felixrieseberg commented Nov 21, 2017

Can confirm: No more error message 👍

@ckerr

This comment has been minimized.

Member

ckerr commented Nov 21, 2017

Thank you for confirming!

@ckerr ckerr merged commit dba9181 into master Nov 21, 2017

8 checks passed

ci/circleci: electron-linux-arm Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm64 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-ia32 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-x64 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/jenkins/branch This commit looks good
Details
continuous-integration/jenkins/pr-head This commit looks good
Details

@ckerr ckerr deleted the nstitlebar-hidden branch Nov 21, 2017

// Hide the title bar.
if (title_bar_style_ == HIDDEN_INSET || title_bar_style_ == HIDDEN) {
// Hide the title bar background
if (title_bar_style_ != NORMAL) {
if (base::mac::IsAtLeastOS10_10()) {
[window_ setTitlebarAppearsTransparent:YES];

This comment has been minimized.

@singhshashi

singhshashi Dec 30, 2017

Should this be titleBarAppearsTransparent instead of setTitleBarAppearsTransparent? NSWindow has titleBarAppearsTransparent https://developer.apple.com/documentation/appkit/nswindow/1419167-titlebarappearstransparent?language=objc. I could not find anything in the Apple documentation for the term setTitleBarAppearsTransparent.

This comment has been minimized.

@MarshallOfSound

MarshallOfSound Dec 30, 2017

Member

The set prefix is because we aren't addressing the window itself rather a wrapper.

base::scoped_nsobject<AtomNSWindow> window_;

This comment has been minimized.

@singhshashi

singhshashi Dec 30, 2017

Ah! Now that I think of it, the code would not have compiled if an undefined method was used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment