Skip to content
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

Touch Bar Support #2366

Merged
merged 5 commits into from Sep 17, 2018
Merged

Touch Bar Support #2366

merged 5 commits into from Sep 17, 2018

Conversation

skurfer
Copy link
Member

@skurfer skurfer commented Jun 8, 2017

A working demo for anyone that wants to try it out. To customize the touch bar, you’ll need to make the Dock icon visible, then choose “Customize Touch Bar” from the menu.

Available buttons:

  • History back/forward
  • Grab current selection
  • Clear the interface
  • Quick Look
  • Reveal Proxy
  • Show Task Viewer

And for dealing with collections:

  • Back/forward in collection
  • Explode collection
  • Remove Last Item

I don’t know if the touch bar will be the primary way to do any of these things. I mainly see it as a way to expose the fact that they’re possible. I’m guessing a lot of people don’t know about the functionality listed above, even though it’s all been here for years.

The main thing preventing this from being ready, in my opinion, is the icons. We need some better ones for things like grab selection.

I’d like to hear ideas for additional buttons.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Sep 20, 2017

Cool, I'm still dubious as to the use of the touchbar, but as a way of exposing hidden features this is great. Unfortunately I have no way of testing it. Can you send a screenshot/picture of the icons?

My guess is they are 'adequate' for now. I've looked through the code and it all seems good to me.
I've made a couple of small comments directly on the code

@@ -870,7 +870,9 @@ - (void)startQuicksilver:(id)sender {
[QSLibrarian sharedInstance];
[QSExecutor sharedInstance];
[QSTaskController sharedInstance];
[NSApp setAutomaticCustomizeTouchBarMenuItemEnabled:YES];
if ([NSApplication isSierra]) {
Copy link
Member

@pjrobertson pjrobertson Sep 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked if there was a way to do if(mac_has_touchbar) but apparently Apple doesn't want you to do that, so this seems fine to me

Copy link
Member Author

@skurfer skurfer Sep 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had it in there unconditionally at first because the documentation implies that the SDK will work it out for you. That was not the case when I tried (unsuccessfully) to run it under 10.10.

- (nullable NSTouchBarItem *)touchBar:(NSTouchBar *)touchBar makeItemForIdentifier:(NSTouchBarItemIdentifier)identifier
{
BOOL somethingIsSelected = ([self objectValue] != nil);
if ([identifier isEqualToString:QSHistoryItemIdentifier]) {
Copy link
Member

@pjrobertson pjrobertson Sep 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the Swift sample code (ToolbarSample/WindowController.swift) it uses a switch identifier instead of doing ifs. I think this is just a fancy part of Swift that means it can do switches on not just ints, but strings as well (?).

@skurfer
Copy link
Member Author

@skurfer skurfer commented Sep 20, 2017

Unfortunately I have no way of testing it. Can you send a screenshot/picture of the icons?

You can use Touché to check it out. There’s also a way to simulate in Xcode, but I never bothered to figure that out.

The main thing preventing this from being ready, in my opinion, is the icons. We need some better ones for things like grab selection.

I’m going to talk to a designer I know about this to see what it would cost. (She’s family, so probably cheap.) Any objections?

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Sep 22, 2017

@skurfer
Copy link
Member Author

@skurfer skurfer commented Jun 9, 2018

OK, I think this is ready. Anyone able to take a look?

@skurfer skurfer merged commit d6609c8 into master Sep 17, 2018
2 checks passed
skurfer added a commit that referenced this issue Sep 17, 2018
@skurfer skurfer deleted the touchbar branch Sep 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants