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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add TouchBar Support #8095

Merged
merged 69 commits into from Mar 3, 2017

Conversation

Projects
None yet
@MarshallOfSound
Member

MarshallOfSound commented Nov 29, 2016

So here goes 馃憤

I've still got to add docs for all this but basically this PR adds support for

  • window.setTouchBar(bar)
  • new TouchBar([...])
  • new TouchBarButton(...)
  • new TouchBarColorPicker(...)
  • new TouchBarGroup(...)
  • new TouchBarLabel(...)
  • new TouchBarPopover(...)
  • new TouchBarSpacer(...)
  • new TouchBarSlider(...)

Still to do

  • Add support for the "customizable" attribute (pretty major thing missing but it requires some good docs and some magical techniques)
  • Move item classes to their own files instead of being static classes on TouchBar
  • Document all the things
  • Implement a NSCustomTouchBarItem with an NSScrubber as the view (this is quite a big task, not sure if it is even possible to declare at runtime)
  • Implement a NSCustomTouchBarItem with an NSSegmentedControl as the view!
  • Test? Somehow? Idk?
  • Ensure this doesn't break on older macOS versions. (Test on El Capitan)
  • API Feedback / changes
  • Remove modifications to default_app
  • Fix linting 馃槅
  • Add methods to manually set the slider value
  • Add a way to create the "spacer" touchbar items

Closes #7781

@MarshallOfSound

This comment has been minimized.

Show comment
Hide comment
@MarshallOfSound

MarshallOfSound Nov 29, 2016

Member

@zcbenz / @kevinsawicki / @zeke Given this compiles without any issue on my laptop I think the OSX build machine needs to have it's XCode updated?

Member

MarshallOfSound commented Nov 29, 2016

@zcbenz / @kevinsawicki / @zeke Given this compiles without any issue on my laptop I think the OSX build machine needs to have it's XCode updated?

@MarshallOfSound MarshallOfSound referenced this pull request in MarshallOfSound/Google-Play-Music-Desktop-Player-UNOFFICIAL- Nov 29, 2016

Closed

Touchbar support for macOS Sierra #1965

@bengotow

This comment has been minimized.

Show comment
Hide comment
@bengotow

bengotow Nov 29, 2016

Contributor

This looks great! Will try it today with N1. You're correct - this will need to be built with the target SDK set to MacOS 10.12.1 with the most recent version of Xcode. I wonder how often Travis updates Xcode...

Contributor

bengotow commented Nov 29, 2016

This looks great! Will try it today with N1. You're correct - this will need to be built with the target SDK set to MacOS 10.12.1 with the most recent version of Xcode. I wonder how often Travis updates Xcode...

@MarshallOfSound

This comment has been minimized.

Show comment
Hide comment
@MarshallOfSound

MarshallOfSound Nov 29, 2016

Member

@bengotow It looks like travis-ci already has support for the required version of X-Code

https://docs.travis-ci.com/user/languages/objective-c/ 馃槃

Member

MarshallOfSound commented Nov 29, 2016

@bengotow It looks like travis-ci already has support for the required version of X-Code

https://docs.travis-ci.com/user/languages/objective-c/ 馃槃

@krisnarocks

This comment has been minimized.

Show comment
Hide comment
@krisnarocks

krisnarocks Dec 10, 2016

Would like to point this cool app,
https://red-sweater.com/touche/

It emulates TouchBar on all mac (as it claims). So, if you need to do some testing on a mac without TouchBar, you could do it now.

Edit: Or, you could use the latest xcode with the latest sierra and open the TouchBar emulator window

krisnarocks commented Dec 10, 2016

Would like to point this cool app,
https://red-sweater.com/touche/

It emulates TouchBar on all mac (as it claims). So, if you need to do some testing on a mac without TouchBar, you could do it now.

Edit: Or, you could use the latest xcode with the latest sierra and open the TouchBar emulator window

@MarshallOfSound

This comment has been minimized.

Show comment
Hide comment
@MarshallOfSound

MarshallOfSound Dec 16, 2016

Member

In order to reduce API overhead (keep methods minimal) I added a method to all TouchBarItem instances called updateConfig (probably going to rename to updateOptions) that will update the relevant NSTouchBarItem instances with the new config without redrawing the entire TouchBar. Seems to work nicely 馃憤

Member

MarshallOfSound commented Dec 16, 2016

In order to reduce API overhead (keep methods minimal) I added a method to all TouchBarItem instances called updateConfig (probably going to rename to updateOptions) that will update the relevant NSTouchBarItem instances with the new config without redrawing the entire TouchBar. Seems to work nicely 馃憤

@HaNdTriX HaNdTriX referenced this pull request in atom/atom Dec 25, 2016

Closed

Support MacBook Touch Bar #13224

@hansemannn hansemannn referenced this pull request in yomybaby/atom-titanium Dec 27, 2016

Open

Support Macbook Pro Touch Bar #53

@MarshallOfSound MarshallOfSound referenced this pull request in MarshallOfSound/Google-Play-Music-Desktop-Player-UNOFFICIAL- Jan 5, 2017

Closed

Feature request: Touch bar Song Title #2077

@bcoe

This comment has been minimized.

Show comment
Hide comment
@bcoe

bcoe Jan 13, 2017

@MarshallOfSound hey, I'm attempting to take this branch for a spin and am running into some issues.

When I start up the project, electron . in the project folder; focusing on the window doesn't seem to modify the touch bar ... I'm fairly new to electron development, so please excuse my ignorance if I'm missing something simple.

bcoe commented Jan 13, 2017

@MarshallOfSound hey, I'm attempting to take this branch for a spin and am running into some issues.

When I start up the project, electron . in the project folder; focusing on the window doesn't seem to modify the touch bar ... I'm fairly new to electron development, so please excuse my ignorance if I'm missing something simple.

@zeke

This comment has been minimized.

Show comment
Hide comment
@zeke

zeke Jan 13, 2017

Member

Hi @bcoe!

Member

zeke commented Jan 13, 2017

Hi @bcoe!

@MarshallOfSound

This comment has been minimized.

Show comment
Hide comment
@MarshallOfSound

MarshallOfSound Jan 14, 2017

Member

@bcoe This part is probably where you went wrong

built it and installed it globally (npm i -g).

There isn't currently an easy way (that I know of) to link the version of Electron you built with an the electron command in another module. Whenever I test I just copy the output of the electron build to node_modules/electron/dist in whatever project I want to use it.

If you launch the sample application (that is generated during the build) do you get the touch bar buttons and such?

Member

MarshallOfSound commented Jan 14, 2017

@bcoe This part is probably where you went wrong

built it and installed it globally (npm i -g).

There isn't currently an easy way (that I know of) to link the version of Electron you built with an the electron command in another module. Whenever I test I just copy the output of the electron build to node_modules/electron/dist in whatever project I want to use it.

If you launch the sample application (that is generated during the build) do you get the touch bar buttons and such?

@ktiedt

This comment has been minimized.

Show comment
Hide comment
@ktiedt

ktiedt Jan 25, 2017

This is amazing progress :D

Any chance the popover can allow for options (if it doesnt already - I didn't see) to mimic the volume and or brightness popovers? Or the animated popover for the default function keys?

ktiedt commented Jan 25, 2017

This is amazing progress :D

Any chance the popover can allow for options (if it doesnt already - I didn't see) to mimic the volume and or brightness popovers? Or the animated popover for the default function keys?

@MarshallOfSound

This comment has been minimized.

Show comment
Hide comment
@MarshallOfSound

MarshallOfSound Jan 25, 2017

Member

@ktiedt I briefly looked into mirroring that kind of functionality and the TLDR is.

  • I couldn't find a way to do the animation (maybe private API?, or maybe I'm blind and missed the docs?) 馃槅
  • You can replicate the brightness and volume sliders with some craftily placed spacers in the popover touchbar
Member

MarshallOfSound commented Jan 25, 2017

@ktiedt I briefly looked into mirroring that kind of functionality and the TLDR is.

  • I couldn't find a way to do the animation (maybe private API?, or maybe I'm blind and missed the docs?) 馃槅
  • You can replicate the brightness and volume sliders with some craftily placed spacers in the popover touchbar
@tvanier

This comment has been minimized.

Show comment
Hide comment
@tvanier

tvanier Feb 6, 2017

Thanks @MarshallOfSound for the awesome TouchBar support! Was wondering if someone tried a button with an image? I rebuilt the default app with the button below (in default_app.js), but it won't show up in the touch bar. Maybe some special image format/size is required? I could just find "2x with a maximum height of 30 points (corresponding to 60 pixels)" in these docs.

const imageButton = new (TouchBar.Button)({
    image: path.join(__dirname, 'myimage@2x.png'),
    click: () => {
        console.log('my image clicked');
    }
});

tvanier commented Feb 6, 2017

Thanks @MarshallOfSound for the awesome TouchBar support! Was wondering if someone tried a button with an image? I rebuilt the default app with the button below (in default_app.js), but it won't show up in the touch bar. Maybe some special image format/size is required? I could just find "2x with a maximum height of 30 points (corresponding to 60 pixels)" in these docs.

const imageButton = new (TouchBar.Button)({
    image: path.join(__dirname, 'myimage@2x.png'),
    click: () => {
        console.log('my image clicked');
    }
});
@MarshallOfSound

This comment has been minimized.

Show comment
Hide comment
@MarshallOfSound

MarshallOfSound Feb 6, 2017

Member

@tvanier I did while I was building out the API. Try removing the "@2x" from the file name

Member

MarshallOfSound commented Feb 6, 2017

@tvanier I did while I was building out the API. Try removing the "@2x" from the file name

@bcoe

This comment has been minimized.

Show comment
Hide comment
@bcoe

bcoe Feb 12, 2017

@MarshallOfSound thanks for your help, you're right I was not using the correct version of electron to test. My testing went pretty well; I got things running with hyper-term for a proof of concept:

https://twitter.com/BenjaminCoe/status/830607339577225218

some feedback for you:

  • I found I was able to update the label on existing buttons (which was how I implemented the feature shown above).
  • I was having trouble updating a label element; updating the label did not cause a redraw -- and you'd end up with draw artifacts.
  • in general I'm curious if it's possible to expose more control over text layout, my goal is to:
    • show git and build information in the touchbar -- so fairly complex text output (this would rock!).

amazing work! can't wait for this to eventually land.

bcoe commented Feb 12, 2017

@MarshallOfSound thanks for your help, you're right I was not using the correct version of electron to test. My testing went pretty well; I got things running with hyper-term for a proof of concept:

https://twitter.com/BenjaminCoe/status/830607339577225218

some feedback for you:

  • I found I was able to update the label on existing buttons (which was how I implemented the feature shown above).
  • I was having trouble updating a label element; updating the label did not cause a redraw -- and you'd end up with draw artifacts.
  • in general I'm curious if it's possible to expose more control over text layout, my goal is to:
    • show git and build information in the touchbar -- so fairly complex text output (this would rock!).

amazing work! can't wait for this to eventually land.

@MarshallOfSound

This comment has been minimized.

Show comment
Hide comment
@MarshallOfSound

MarshallOfSound Feb 12, 2017

Member

I was having trouble updating a label element; updating the label did not cause a redraw -- and you'd end up with draw artifacts.

Will experiment with that, I have an example locally which implements a clock in a label element (one of the local testing things I did) and it appears to update nicely.

in general I'm curious if it's possible to expose more control over text layout, my goal is to:

The best I can do really is expose the core API's of the touch bar, anything super advanced requires custom views in native code and can't really be generalized well enough.

show git and build information in the touchbar -- so fairly complex text output (this would rock!).

First off, that would be aswesome, just having like Webpack Status: Building or something in the touchbar would save a few alt tabs to watch it rebuild.

Secondly, what exactly do you need in order to achieve what you want to do? Can't this kind of text output be done with labels?

Member

MarshallOfSound commented Feb 12, 2017

I was having trouble updating a label element; updating the label did not cause a redraw -- and you'd end up with draw artifacts.

Will experiment with that, I have an example locally which implements a clock in a label element (one of the local testing things I did) and it appears to update nicely.

in general I'm curious if it's possible to expose more control over text layout, my goal is to:

The best I can do really is expose the core API's of the touch bar, anything super advanced requires custom views in native code and can't really be generalized well enough.

show git and build information in the touchbar -- so fairly complex text output (this would rock!).

First off, that would be aswesome, just having like Webpack Status: Building or something in the touchbar would save a few alt tabs to watch it rebuild.

Secondly, what exactly do you need in order to achieve what you want to do? Can't this kind of text output be done with labels?

@matthewdias

This comment has been minimized.

Show comment
Hide comment
@matthewdias

matthewdias Feb 12, 2017

According to the spec, the touchbar isn't supposed to be used for displaying information. It's strictly for controls.

According to the spec, the touchbar isn't supposed to be used for displaying information. It's strictly for controls.

@NTag

This comment has been minimized.

Show comment
Hide comment
@NTag

NTag Feb 12, 2017

@matthewdias However, when using QuickTime for a screencast, the touchbar displays the length and the size of the recording, which is pure information.

screenshot-touchbar-quicktime

NTag commented Feb 12, 2017

@matthewdias However, when using QuickTime for a screencast, the touchbar displays the length and the size of the recording, which is pure information.

screenshot-touchbar-quicktime

@sindresorhus sindresorhus referenced this pull request in wulkano/kap Feb 12, 2017

Closed

Touch Bar support #143

@jhen0409

This comment has been minimized.

Show comment
Hide comment
@jhen0409

jhen0409 Feb 14, 2017

Member

Just found a way to add button to Control Strip, but it's private API: https://github.com/a2/touch-baer

Probably only these applications are using it: Xcode, TouchSwitcher, Rocket

Currently it's I feeling the best experience for use TouchBar, but I think it will break the experience of MPRemoteCommandCenter, this may be the reason Apple didn't public it? 馃

Member

jhen0409 commented Feb 14, 2017

Just found a way to add button to Control Strip, but it's private API: https://github.com/a2/touch-baer

Probably only these applications are using it: Xcode, TouchSwitcher, Rocket

Currently it's I feeling the best experience for use TouchBar, but I think it will break the experience of MPRemoteCommandCenter, this may be the reason Apple didn't public it? 馃

@Cryrivers

This comment has been minimized.

Show comment
Hide comment
@Cryrivers

Cryrivers Feb 22, 2017

Any updates for this PR?

Any updates for this PR?

@PeterTeng

This comment has been minimized.

Show comment
Hide comment
@PeterTeng

PeterTeng Feb 22, 2017

The last update was 2 months ago, any one approach to this feature too?

The last update was 2 months ago, any one approach to this feature too?

@MarshallOfSound

This comment has been minimized.

Show comment
Hide comment
@MarshallOfSound

MarshallOfSound Feb 22, 2017

Member

@Cryrivers @PeterTeng This PR is currently blocked by the inability to build Chromium with the required target SDK.

See #8280 (comment)

Member

MarshallOfSound commented Feb 22, 2017

@Cryrivers @PeterTeng This PR is currently blocked by the inability to build Chromium with the required target SDK.

See #8280 (comment)

@zeke zeke added the blocked label Feb 23, 2017

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment

@Cryrivers Cryrivers referenced this pull request in Cryrivers/ember-electron-touchbar Feb 23, 2017

Open

Release an alpha version of this addon #1

@MarshallOfSound

This comment has been minimized.

Show comment
Hide comment
@MarshallOfSound

MarshallOfSound Feb 23, 2017

Member

@kevinsawicki Will look into next week some time (unless someone plays with it first). Will have to downgrade my SDK and see if it still compiles 馃槅 What exact combinations of SDK / xCode do the build machines use again?

Member

MarshallOfSound commented Feb 23, 2017

@kevinsawicki Will look into next week some time (unless someone plays with it first). Will have to downgrade my SDK and see if it still compiles 馃槅 What exact combinations of SDK / xCode do the build machines use again?

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Feb 23, 2017

Contributor

What exact combinations of SDK / xCode do the build machines use again?

Internal CI has XCode 8.2.1 and 10.10 SDK

Contributor

kevinsawicki commented Feb 23, 2017

What exact combinations of SDK / xCode do the build machines use again?

Internal CI has XCode 8.2.1 and 10.10 SDK

@kevinsawicki kevinsawicki removed the blocked label Feb 28, 2017

@kevinsawicki kevinsawicki self-assigned this Feb 28, 2017

@galli-leo

This comment has been minimized.

Show comment
Hide comment
@galli-leo

galli-leo Feb 28, 2017

Would it be possible to include TouchID functionality as well? I really like https://hyper.is as terminal client and it would be very cool to implement TouchID for sudo.

Would it be possible to include TouchID functionality as well? I really like https://hyper.is as terminal client and it would be very cool to implement TouchID for sudo.

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Feb 28, 2017

Contributor

Would it be possible to include TouchID functionality as well?

That is outside the scope of this pull request, please open a separate issue for it, thanks.

Contributor

kevinsawicki commented Feb 28, 2017

Would it be possible to include TouchID functionality as well?

That is outside the scope of this pull request, please open a separate issue for it, thanks.

+
+ _addLiveProperty (name, initialValue) {
+ const privateName = `_${name}`
+ this[privateName] = initialValue

This comment has been minimized.

@MarshallOfSound

MarshallOfSound Mar 2, 2017

Member

Can't this just be

let liveValue = initialValue
Object.defineProperty(this, name, {
  get: function () {
    return liveValue
  },
  set: function (value) {
    liveValue = value
    this.emit('change')
  },
})

Then we aren't exposing the private variable on the this scope

@MarshallOfSound

MarshallOfSound Mar 2, 2017

Member

Can't this just be

let liveValue = initialValue
Object.defineProperty(this, name, {
  get: function () {
    return liveValue
  },
  set: function (value) {
    liveValue = value
    this.emit('change')
  },
})

Then we aren't exposing the private variable on the this scope

This comment has been minimized.

@kevinsawicki

kevinsawicki Mar 2, 2017

Contributor

The underscore version is used other places to set the property directly in response to an interaction event without emitting a change event so the UI isn't updated in response to an interaction.

@kevinsawicki

kevinsawicki Mar 2, 2017

Contributor

The underscore version is used other places to set the property directly in response to an interaction event without emitting a change event so the UI isn't updated in response to an interaction.

This comment has been minimized.

@MarshallOfSound

MarshallOfSound Mar 2, 2017

Member

Ah, nice 馃槃

@MarshallOfSound

MarshallOfSound Mar 2, 2017

Member

Ah, nice 馃槃

@kevinsawicki kevinsawicki changed the title from [WIP] Initial TouchBar Support to [WIP] Add TouchBar Support Mar 3, 2017

@zeke

zeke approved these changes Mar 3, 2017

Docs look great. 馃憤

+}
+
+const finishSpin = () => {
+ const uniqueValues = new Set([reel1.label, reel2.label, reel3.label]).size

This comment has been minimized.

@kevinsawicki kevinsawicki changed the title from [WIP] Add TouchBar Support to Add TouchBar Support Mar 3, 2017

Initial TouchBar Magic
* Make the AtomNSWindow also a NSTouchbarDelegate
* Implement basic makeTouchBar and makeItemForIdentifier methods
* Initial sending of touch / update events through IPC to BrowserWindowObjects

TODO:
* JS API
* JS Object Converters
* Generalize methods so that popovers can work

@kevinsawicki kevinsawicki merged commit 0098822 into master Mar 3, 2017

8 of 9 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
electron-linux-arm Build #5737875 succeeded in 75s
Details
electron-linux-ia32 Build #5737876 succeeded in 68s
Details
electron-linux-x64 Build #5737877 succeeded in 147s
Details
electron-mas-x64 Build #3576 succeeded in 8 min 7 sec
Details
electron-osx-x64 Build #3581 succeeded in 8 min 35 sec
Details
electron-win-ia32 Build #2589 succeeded in 8 min 0 sec
Details
electron-win-x64 Build #2565 succeeded in 8 min 7 sec
Details
@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Mar 3, 2017

Contributor

I've tested this on El Capitan to verify it works on older OS versions and also machine without a touch bar.

Thanks @MarshallOfSound for your awesome work on this 馃憤 馃帀 馃

It will be included in the next Electron beta release, 1.6.3.

slot-machine

Contributor

kevinsawicki commented Mar 3, 2017

I've tested this on El Capitan to verify it works on older OS versions and also machine without a touch bar.

Thanks @MarshallOfSound for your awesome work on this 馃憤 馃帀 馃

It will be included in the next Electron beta release, 1.6.3.

slot-machine

@kevinsawicki kevinsawicki deleted the touchbar branch Mar 3, 2017

@tvanier

This comment has been minimized.

Show comment
Hide comment
@tvanier

tvanier Mar 3, 2017

Nice job @MarshallOfSound and @kevinsawicki ! I will retry a button with an image file, this did not work for me a few weeks back, likely because of my png. Maybe add an example with a working image?

const imageButton = new (TouchBar.Button)({
    image: path.join(__dirname, 'my-icon.png'),
    click: () => {
        console.log('my image clicked');
    }
});

tvanier commented Mar 3, 2017

Nice job @MarshallOfSound and @kevinsawicki ! I will retry a button with an image file, this did not work for me a few weeks back, likely because of my png. Maybe add an example with a working image?

const imageButton = new (TouchBar.Button)({
    image: path.join(__dirname, 'my-icon.png'),
    click: () => {
        console.log('my image clicked');
    }
});
@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Mar 3, 2017

Contributor

image: path.join(__dirname, 'my-icon.png'),

The property is icon now to match the MenuItem API.

Contributor

kevinsawicki commented Mar 3, 2017

image: path.join(__dirname, 'my-icon.png'),

The property is icon now to match the MenuItem API.

@tvanier

This comment has been minimized.

Show comment
Hide comment
@tvanier

tvanier Mar 3, 2017

Ah ok, although NSButton uses image I think. I guess these icon specs should work?

If you use an image in a button or other control in the Touch Bar, take care to employ a template image. Template images in the Touch Bar respond automatically to system white-point changes, and automatically react to user interactions.
Use Retina-resolution image assets, designated as @2x in your asset catalog and with a maximum height of 30 points (corresponding to 60 pixels).

https://developer.apple.com/reference/appkit/nstouchbar#2647104

tvanier commented Mar 3, 2017

Ah ok, although NSButton uses image I think. I guess these icon specs should work?

If you use an image in a button or other control in the Touch Bar, take care to employ a template image. Template images in the Touch Bar respond automatically to system white-point changes, and automatically react to user interactions.
Use Retina-resolution image assets, designated as @2x in your asset catalog and with a maximum height of 30 points (corresponding to 60 pixels).

https://developer.apple.com/reference/appkit/nstouchbar#2647104

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Mar 3, 2017

Contributor

I guess these icon specs should work?

Yup, they should, I've tried with this icon locally called electron@2x.png and it worked for me:
electron 2x

Contributor

kevinsawicki commented Mar 3, 2017

I guess these icon specs should work?

Yup, they should, I've tried with this icon locally called electron@2x.png and it worked for me:
electron 2x

@PeterTeng

This comment has been minimized.

Show comment
Hide comment

PeterTeng commented Mar 4, 2017

Nice job @MarshallOfSound and @kevinsawicki !!!!

@rhysd

This comment has been minimized.

Show comment
Hide comment
@rhysd

rhysd Mar 5, 2017

Contributor

I tried master branch and this really works nice! Thank you.

Contributor

rhysd commented Mar 5, 2017

I tried master branch and this really works nice! Thank you.

@sindresorhus sindresorhus referenced this pull request in sindresorhus/caprine Mar 8, 2017

Open

Touch Bar support #170

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