Native tabs on macOS #9052
Native tabs on macOS #9052
Conversation
|
@tonyganch very cool! |
atom/browser/native_window_mac.mm
Outdated
| @@ -656,7 +656,8 @@ static bool FromV8(v8::Isolate* isolate, v8::Handle<v8::Value> val, | |||
| was_fullscreen_(false), | |||
| zoom_to_page_width_(false), | |||
| attention_request_id_(0), | |||
| title_bar_style_(NORMAL) { | |||
| title_bar_style_(NORMAL), | |||
| tabbing_identifier_(""){ | |||
There was a problem hiding this comment.
Space needed before {, perhaps we don't even need the tabbing_identifier_ initializer here since it will be an empty string by default.
atom/browser/native_window_mac.mm
Outdated
| // Create a tab only if tabbing identifier is specified and window has | ||
| // a native title bar. | ||
| if (tabbing_identifier_.empty() || transparent() || !has_frame()) { | ||
| [window_ setTabbingMode:NSWindowTabbingModeDisallowed]; |
There was a problem hiding this comment.
This was added in 10.12 and so this does not compile against the 10.10 SDK which Electron builds against.
Will probably need forward declarations and/or some respondsToSelector checks.
https://developer.apple.com/reference/appkit/nswindow/1644729-tabbingmode?language=objc
https://travis-ci.org/electron/electron/jobs/216322601
../../atom/browser/native_window_mac.mm:764:16: error: instance method '-setTabbingMode:' not found (return type defaults to 'id') [-Werror,-Wobjc-method-access]
[window_ setTabbingMode:NSWindowTabbingModeDisallowed];
^~~~~~~~~~~~~~
../../atom/browser/native_window_mac.mm:339:12: note: receiver is instance of class declared here
@interface AtomNSWindow : EventDispatchingWindow<QLPreviewPanelDataSource, QLPreviewPanelDelegate, NSTouchBarDelegate> {
^
../../atom/browser/native_window_mac.mm:764:31: error: use of undeclared identifier 'NSWindowTabbingModeDisallowed'
[window_ setTabbingMode:NSWindowTabbingModeDisallowed];
^
../../atom/browser/native_window_mac.mm:766:16: error: instance method '-setTabbingIdentifier:' not found (return type defaults to 'id') [-Werror,-Wobjc-method-access]
[window_ setTabbingIdentifier:base::SysUTF8ToNSString(tabbing_identifier_)];
atom/browser/native_window_mac.mm
Outdated
| if (tabbing_identifier_.empty() || transparent() || !has_frame()) { | ||
| [window_ setTabbingMode:NSWindowTabbingModeDisallowed]; | ||
| } else { | ||
| [window_ setTabbingIdentifier:base::SysUTF8ToNSString(tabbing_identifier_)]; |
There was a problem hiding this comment.
Same with this one, it is 10.12, https://developer.apple.com/reference/appkit/nswindow/1644704-tabbingidentifier?language=objc
|
@kevinsawicki, thank you for helpful review! Do you think I should leave |
I think we should be good without the I'll add a test that exercises it and remove the check and we'll see if it passes on CI which is pre-10.12 |
atom/browser/native_window_mac.mm
Outdated
| @@ -682,6 +690,8 @@ static bool FromV8(v8::Isolate* isolate, v8::Handle<v8::Value> val, | |||
|
|
|||
| options.Get(options::kTitleBarStyle, &title_bar_style_); | |||
|
|
|||
| options.Get(options::kTabbingIdentifier, &tabbing_identifier_); | |||
There was a problem hiding this comment.
Looks like tabbing_identifier_ is only used in this method so it could be local variable here instead of an instance variable.
docs/api/browser-window.md
Outdated
| @@ -264,6 +264,9 @@ It creates a new `BrowserWindow` with native properties as set by the `options`. | |||
| canvas features. Default is `false`. | |||
| * `scrollBounce` Boolean (optional) - Enables scroll bounce (rubber banding) effect on | |||
| macOS. Default is `false`. | |||
| * `tabbingIdentifier` String (optional) - Tab group name, allows opening the | |||
There was a problem hiding this comment.
This looks like it is under the webPreferences outer list but it isn't a web preference.
Perhaps it should go after zoomToPageWidth instead.
|
Thanks for this @tonyganch, awesome work 👍 📑 🎆 |
|
@tonyganch seems to work nicely! One thing I am wondering is: if you have a window with tab bar to the top and you open a new window, it will open as separate window. But if you are in fullscreen mode, that new window will be attached as tab. Would it make sense to have API to force a window to open into an existing tab group or not? This would make it possible to give the user the choice of wether a window should open as tab or not independent from being in fullscreen mode or not. Not even sure such API exists on macOS though. Update: extracted this into #9094 |
|
@tonyganch The video looks great, but when I try your sample locally (with Electron 1.6.6) on MacOS 10.12.4, all of the buttons open new windows rather than tabs. Edit: I tried downgrading to Electron 1.6.3 and matching the Node version in your video, but it's still not working. |
Add support for the `NSWindow addTabbedWindow` method on MacOSX This plays nicely with the changes from electron#9052 and electron#9725 Usage samples available in [this commit][c] in my fork of `electron-quick-start` [c]: danielma/electron-quick-start@79f0659
Add support for the [`NSWindow addTabbedWindow`][nsw] method on MacOSX This plays nicely with the changes from electron#9052 and electron#9725 Usage samples available in [this commit][c] in my fork of `electron-quick-start` [nsw]: https://developer.apple.com/documentation/appkit/nswindow/1855947-addtabbedwindow [c]: danielma/electron-quick-start@79f0659
Add support for the [`NSWindow addTabbedWindow`][nsw] method on MacOSX This plays nicely with the changes from electron#9052 and electron#9725 Usage samples available in [this commit][c] in my fork of `electron-quick-start` [nsw]: https://developer.apple.com/documentation/appkit/nswindow/1855947-addtabbedwindow [c]: danielma/electron-quick-start@79f0659
Add support for the [`NSWindow addTabbedWindow`][nsw] method on MacOSX This plays nicely with the changes from electron#9052 and electron#9725 Usage samples available in [this commit][c] in my fork of `electron-quick-start` [nsw]: https://developer.apple.com/documentation/appkit/nswindow/1855947-addtabbedwindow [c]: danielma/electron-quick-start@79f0659
Add support for the [`NSWindow addTabbedWindow`][nsw] method on MacOSX This plays nicely with the changes from electron#9052 and electron#9725 Usage samples available in [this commit][c] in my fork of `electron-quick-start` [nsw]: https://developer.apple.com/documentation/appkit/nswindow/1855947-addtabbedwindow [c]: danielma/electron-quick-start@79f0659
Should fix #6124.
Testing app: https://github.com/tonyganch/electron-quick-start/tree/native-tabs
Video: https://yadi.sk/i/vzAhnDkd3GTC5b
The idea is to make windows not tabbable by default.
That will allow existing applications to save user experience as they have it right now.
In order to open a window as a native tab,
tabbingIdentifieroption should be passed, like:new BrowserWindow({tabbingIdentifier: 'mainGroup'}).If tabbing identifier is missing/empty, or a window has custom/no title bar (is frameless/transparent), such window will not be tabbed.
/cc @alespergl @bpasero