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

Native tabs on macOS #9052

Merged
merged 8 commits into from Mar 30, 2017

Conversation

Projects
None yet
4 participants
@tonyganch
Member

tonyganch commented Mar 29, 2017

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, tabbingIdentifier option 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

@bpasero

This comment has been minimized.

Show comment
Hide comment
@bpasero

bpasero Mar 29, 2017

Contributor

@tonyganch very cool!

Contributor

bpasero commented Mar 29, 2017

@tonyganch very cool!

@kevinsawicki kevinsawicki self-assigned this Mar 29, 2017

@tonyganch

This comment has been minimized.

Show comment
Hide comment
@tonyganch

tonyganch Mar 30, 2017

Member

@kevinsawicki, thank you for helpful review!
I added declarations for the methods that I use plus added checks with respondsToSelector.
I checked the changes locally with older SDK and ran the build on Travis, it's green now:
https://travis-ci.org/electron/electron/jobs/216753155

Do you think I should leave base::mac::IsAtLeastOS10_12 check or it's not necessary anymore because of respondsToSelector and I'd better remove it?

Member

tonyganch commented Mar 30, 2017

@kevinsawicki, thank you for helpful review!
I added declarations for the methods that I use plus added checks with respondsToSelector.
I checked the changes locally with older SDK and ran the build on Travis, it's green now:
https://travis-ci.org/electron/electron/jobs/216753155

Do you think I should leave base::mac::IsAtLeastOS10_12 check or it's not necessary anymore because of respondsToSelector and I'd better remove it?

@tonyganch tonyganch closed this Mar 30, 2017

@tonyganch tonyganch reopened this Mar 30, 2017

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Mar 30, 2017

Contributor

Do you think I should leave base::mac::IsAtLeastOS10_12 check or it's not necessary anymore because of respondsToSelector and I'd better remove it?

I think we should be good without the IsAtLeastOS10_12 check.

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

Contributor

kevinsawicki commented Mar 30, 2017

Do you think I should leave base::mac::IsAtLeastOS10_12 check or it's not necessary anymore because of respondsToSelector and I'd better remove it?

I think we should be good without the IsAtLeastOS10_12 check.

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

@kevinsawicki kevinsawicki merged commit a2588c1 into electron:master Mar 30, 2017

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Mar 30, 2017

Contributor

Thanks for this @tonyganch, awesome work 👍 📑 🎆

Contributor

kevinsawicki commented Mar 30, 2017

Thanks for this @tonyganch, awesome work 👍 📑 🎆

@bpasero

This comment has been minimized.

Show comment
Hide comment
@bpasero

bpasero Mar 31, 2017

Contributor

@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

Contributor

bpasero commented Mar 31, 2017

@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

@russelldc

This comment has been minimized.

Show comment
Hide comment
@russelldc

russelldc May 4, 2017

@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.

russelldc commented May 4, 2017

@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.

@tsdorsey tsdorsey referenced this pull request Aug 9, 2017

Closed

[1.19.0] [macOS] Atom CLI doesn't open native tabs #15202

1 of 1 task complete

danielma added a commit to danielma/electron that referenced this pull request Sep 13, 2017

feat(NativeWindowMac): addTabbedWindow
Add support for the `NSWindow addTabbedWindow` method on MacOSX

This plays nicely with the changes from #9052 and #9725

Usage samples available in [this commit][c] in my fork of
`electron-quick-start`

[c]: danielma/electron-quick-start@79f0659

danielma added a commit to danielma/electron that referenced this pull request Sep 13, 2017

feat(NativeWindowMac): addTabbedWindow
Add support for the [`NSWindow addTabbedWindow`][nsw] method on MacOSX

This plays nicely with the changes from #9052 and #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

danielma added a commit to danielma/electron that referenced this pull request Sep 13, 2017

feat(NativeWindowMac): addTabbedWindow
Add support for the [`NSWindow addTabbedWindow`][nsw] method on MacOSX

This plays nicely with the changes from #9052 and #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

danielma added a commit to danielma/electron that referenced this pull request Sep 13, 2017

feat(NativeWindowMac): addTabbedWindow
Add support for the [`NSWindow addTabbedWindow`][nsw] method on MacOSX

This plays nicely with the changes from #9052 and #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

zcbenz added a commit to danielma/electron that referenced this pull request Oct 3, 2017

feat(NativeWindowMac): addTabbedWindow
Add support for the [`NSWindow addTabbedWindow`][nsw] method on MacOSX

This plays nicely with the changes from #9052 and #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

@bacongravy bacongravy referenced this pull request Jul 16, 2018

Closed

no tabs on linux #655

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