Implements #2734 “New API to configure BrowserWindow title bar on Mac” #2776
Conversation
|
💃 |
|
|
||
| ```javascript | ||
| var BrowserWindow = require('browser-window'); | ||
| var win = new BrowserWindow({ width: 800, height: 600, title-bar-style: "hidden" }); |
|
About the failing tests… I don’t understand why the Linux tests fail, or rather, why are the two new tests run on Linux? I have a condition to bail out from those tests when the OS is not Mac—why is that not respected? The Mac tests fail because I think Travis OSX runtime is 10.9. Should I make the tests conditional on 10.9 so they are not run (how?), or is there a way to make Travis use 10.10 runtime? |
| std::string titleBarStyle = "default"; | ||
| options.Get(switches::kTitleBarStyle, &titleBarStyle); | ||
|
|
||
| if (floor(NSAppKitVersionNumber) >= 1343) { |
There was a problem hiding this comment.
For checking OS X version, you can use base::mac::IsOSYosemiteOrLater() instead.
You can use |
| ### Alternatives on Mac | ||
|
|
||
| On Mac OS X 10.10 Yosemite and newer, there’s an alternative way to specify a chromeless window. Instead of setting `frame` to `false` which disables both the titlebar and window controls, you may want to have the title bar hidden and your content extend to the full window size, yet still preserve the window controls (“traffic lights”) for standard window actions. You can do so by specifying the new `title-bar-style` option: | ||
|
|
There was a problem hiding this comment.
Break lines as the rest of the file mostly <= 80
|
OK, tests don’t fail any more (they are not run on Travis but are run correctly if you run them locally on a Yosemite machine), I addressed all the comments, and made a squash commit. |
| if (titleBarStyle == "hidden-inset") { | ||
| NSToolbar *toolbar = [[NSToolbar alloc] initWithIdentifier:@"titlebarStylingToolbar"]; | ||
| toolbar.showsBaselineSeparator = NO; | ||
| [window_ setToolbar:toolbar]; |
There was a problem hiding this comment.
Looks like there is a memory leak here.
From apple's API document, the parameter of setToolbar is descripted as strong property, so we need to release toolbar here.
There was a problem hiding this comment.
My bad. Thanks for pointing this out. I assumed ARC, but apparently not, been a while since I worked on a non-ARC codebase. :)
New API supported on Yosemite 10.10 and newer.
|
Awesome, thanks! ✨ |
Implements #2734 “New API to configure BrowserWindow title bar on Mac”
|
Ooh, snazzy |
|
Now we just have to wait for the release :) |
| // Configure title bar look on Yosemite or newer | ||
| if (base::mac::IsOSYosemiteOrLater()) { | ||
| if ((titleBarStyle == "hidden") || (titleBarStyle == "hidden-inset")) { | ||
| [window_ setTitlebarAppearsTransparent:YES]; |
There was a problem hiding this comment.
This throws on OS X Mavericks with Electron 0.32.3 when titleBarStyle == "hidden-inset". I thought the base::mac::IsOSYosemiteOrLater() would guard against that?
There was a problem hiding this comment.
Can you create a new issue for this? I'm wondering if it is caused by me setting MACOSX_DEPLOYMENT_TARGET to 10.10.
New API supported on Yosemite 10.10 and newer.
Two new title bar styles: