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

Add keyboard shortcuts to the pref buttons - fixes #527 #785

Merged
merged 11 commits into from Apr 26, 2012

Conversation

pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Mar 31, 2012

This adds ⌘I to the info button, ⌘R to the 'refresh' buttons (where they exist), ⌘- to the 'remove' buttons (where they exist) and ⌘+ to the 'add' menu button (where they exist)

The ⌘+ button brings up the 'add' menu in the catalog and triggers pref panes.

Since I was connecting buttons to iVars, I thought I may as well validate when the buttons are enabled as well.
The 'info' button is only enabled when a single item is clicked in all views. The remove and refresh buttons are disabled when no items are clicked.

For some reason it was not possible to set the key equivalents straight off in Interface Builder, they didn't seem to register, so it had to be done programmatically.

@skurfer
Copy link
Member

@skurfer skurfer commented Apr 5, 2012

Haven’t found any issues with this. Has anyone else tried it? Should we assign a shortcut for the “Open Plug-In Help” menu item? Not sure what the convention is for that. ⌘? maybe?

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 5, 2012

Haven’t found any issues with this. Has anyone else tried it? Should we
assign a shortcut for the “Open Plug-In Help” menu item? Not sure what the
convention is for that. ⌘? maybe?

I've added ⌘?. Only opens the help if it actually exists (same as whether
or not the button is enabled)

I've also widened the search bars a tad. The plugins one can't be widened
any more as there's the text "Installing x plugins" and the status bar next
to it (see them when downloading a plugin)
They're about 15px wider now though, which is always a help

On 5 April 2012 21:02, Rob McBroom <
reply@reply.github.com

wrote:

Haven’t found any issues with this. Has anyone else tried it? Should we
assign a shortcut for the “Open Plug-In Help” menu item? Not sure what the
convention is for that. ⌘? maybe?


Reply to this email directly or view it on GitHub:
#785 (comment)

@HenningJ
Copy link
Contributor

@HenningJ HenningJ commented Apr 7, 2012

I noticed a couple things:

  • ⌘? opens the "Guide" section of the preferences, when the plugin has no help.
  • When I opened plugin help and press ⌘? again, it also takes me to the guide section. It should close the plugin help window instead.
  • When I'm in the catalog section and have selected on of the entries, then press ⌘R to rescan, the list of catalog entries looses focus. After that, I can't use the arrow keys to navigate the catalog entries. It works when pressing ⌘I

Apart from that, I really like this.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 7, 2012

⌘? opens the "Guide" section of the preferences, when the plugin has no help.

Interesting. I see the shortcut is set in the .xib for ⌘?, but it doesn't actually work on my keyboard, and perhaps this is also true for @skurfer since he originally suggested this shortcut. Something to look into, but it might be a problem with Lion...
I tried the German keyboard layout and it still didn't work.

When I'm in the catalog section and have selected on of the entries, then press ⌘R to rescan, the list of catalog entries looses focus. After that, I can't use the arrow keys to navigate the catalog entries. It works when pressing ⌘I

I looked into this for a long time, but couldn't figure out why this was and how to fix it. I hoped nobody would pick up on it, but it is something that should be fixed really

@HenningJ
Copy link
Contributor

@HenningJ HenningJ commented Apr 8, 2012

I looked into this for a long time, but couldn't figure out why this was and how to fix it. I hoped nobody would pick up on it, but it is something that should be fixed really

I've tracked it down: Remove the line

[[itemTable window] makeFirstResponder:[itemTable window]];

from the method QSCatalogPrefPane rescanCurrentItem:.

I found another inconsistency between the pref panes: I can dismiss every pref pane with ⎋, except the catalog pref pane. Not really important, but I just noticed it.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 8, 2012

Awesome, thanks Henning!

I found another inconsistency between the pref panes: I can dismiss every pref pane with ⎋, except the catalog pref pane. Not really important, but I just noticed it.

Thanks a known issue #718. I'll look into it on this pull request.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 8, 2012

I've looked into #718 and can't for the life of me figure out why it doesn't work. QSWindow.m:sendEvent gets called, but clearly the event doesn't get sent to the NSWindow object (the whole prefs pane) to close it.

@HenningJ
Copy link
Contributor

@HenningJ HenningJ commented Apr 9, 2012

I've looked into #718 and can't for the life of me figure out why it doesn't work. QSWindow.m:sendEvent gets called, but clearly the event doesn't get sent to the NSWindow object (the whole prefs pane) to close it.

I've looked into it as well.
It took me quite a while, but I did figure it out. I figured somewhere inside QSCatalogPrefPane.m or QSCatalog.xib, something must hijack the ⎋ and overwrite the default behavior. And I found it:
Inside the QSCatalog.xib, there are two buttons, each setting their "KeyEquivalent" to ⎋. They are both outside the visible area and I think they are just used to do exactly that: hijack the ⎋.

The buttons are at Window -> Content View -> Square Button (X) and at source options -> Square Button (X).

If you remove both of them (or at least clear their KeyEquivalent), it works.

@skurfer
Copy link
Member

@skurfer skurfer commented Apr 9, 2012

Interesting. I see the shortcut is set in the .xib for ⌘?, but it doesn't actually work on my keyboard, and perhaps this is also true for @skurfer since he originally suggested this shortcut.

True. ⌘? does nothing unless I have a plug-in selected.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 9, 2012

Once again, good research Henning! I'll add the changes to this pull
request :)

On 9 April 2012 13:50, Henning Jungkurth <
reply@reply.github.com

wrote:

I've looked into #718 and can't for the life of me figure out why it
doesn't work. QSWindow.m:sendEvent gets called, but clearly the event
doesn't get sent to the NSWindow object (the whole prefs pane) to close it.

I've looked into it as well.
It took me quite a while, but I did figure it out. I figured somewhere
inside QSCatalogPrefPane.m or QSCatalog.xib, something must hijack the
⎋ and overwrite the default behavior. And I found it:
Inside the QSCatalog.xib, there are two buttons, each setting their
"KeyEquivalent" to ⎋. They are both outside the visible area and I think
they are just used to do exactly that: hijack the ⎋.

The buttons are at Window -> Content View -> Square Button (X) and at
source options -> Square Button (X).

If you remove both of them (or at least clear their KeyEquivalent), it
works.


Reply to this email directly or view it on GitHub:
#785 (comment)

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 9, 2012

I've added a commit that removes the two key equivs. I think this pull request is complete now :)

@skurfer
Copy link
Member

@skurfer skurfer commented Apr 9, 2012

I've added a commit that removes the two key equivs.

Looks good. ⎋ dismisses the preferences on the Catalog pane now.

I think this pull request is complete now

What about the weirdness surrounding ⌘??

On a related note, I suggested adding that shortcut to the “Open Plug-In Help” menu item so users might see it and know it exists. Thoughts?

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 24, 2012

What about the weirdness surrounding ⌘??

I can't seem to figure it out. I've added a commit that effectively reverts the original commit.
Since this has lots of fiddling with .xibs, I'm worried that we'll get a merge conflict soonish. If it's good with you I think it'd be better if this is merged in then I can be more timely (like, when I don't have exams...) about this.

skurfer added a commit that referenced this issue Apr 26, 2012
Add keyboard shortcuts to the pref buttons - fixes #527
@skurfer skurfer merged commit d976092 into quicksilver:master Apr 26, 2012
@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 29, 2012

@skurfer - I've implemented the ⌘⌥? keyboard shortcut, but it appears the 'About Quicksilver' option in the main menu also has this shortcut. I don't see the point of this, so I think that shortcut should be removed.

I don't want to make a pull request as it will cause a merge conflict with #681

On a side note: it appears the reason ⌘? doesn't work for us to bring up the plugins prefs is because this is the default shortcut to open the 'help' menu in the menu bar. You only see it if you have QS's dock icon enables (which neither of us do) but it eats up the shortcut so ⌘? doesn't take you to the plugins prefs. Not a big problem, just thought I'd tell you my findings.

@skurfer
Copy link
Member

@skurfer skurfer commented Apr 30, 2012

I've implemented the ⌘⌥? keyboard shortcut, but it appears the 'About Quicksilver' option in the main menu also has this shortcut. I don't see the point of this, so I think that shortcut should be removed.

I agree. That isn’t something we need a convenient way to run on a regular basis.

it appears the reason ⌘? doesn't work for us to bring up the plugins prefs is because this is the default shortcut to open the 'help' menu in the menu bar.

Ah, OK. Thanks for the info.

@lovequicksilver
Copy link

@lovequicksilver lovequicksilver commented May 9, 2012

These all work for me. :)

A side issue - in Plug-ins, selecting a plugin and tabbing to the 'Vers.' column allows text entry. If the text is changed, the field can't be escaped, requiring a QS relaunch to fix.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented May 9, 2012

Cool, thanks for letting us know Phil. I've fixed that :)

On 9 May 2012 22:21, lovequicksilver <
reply@reply.github.com

wrote:

These all work for me. :)

A side issue - in Plug-ins, selecting a plugin and tabbing to the 'Vers.'
column allows text entry. If the text is changed, the field can't be
escaped, requiring a QS relaunch to fix.


Reply to this email directly or view it on GitHub:
#785 (comment)

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

4 participants