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 initial hooks for keyboard-related events. #57

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

samhocevar
Copy link

This is the first step to implement keyboard interaction using the standard Win+B, Ctrl+F10, Enter, etc. shortcuts.

Going further will require more work:

  • something similar to Popup placement and offsets added #32 so that WM_CONTEXTMENU can open the context menu next to the icon rather than under the mouse cursor
  • handle NIN_KEYSELECT being received twice in a row (when Enter is used), maybe using a timer similar to TaskBarIcon.singleClickTimer

@@ -762,6 +794,8 @@ private void ShowContextMenu(Point cursorPosition)
// fallback, the context menu can't receive keyboard events - should not happen though
WinApi.SetForegroundWindow(handle);

ContextMenu.Focus();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd like to understand why you have added a focus, can you add some comment here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I still would like to know why you think this is needed, was there some kind of issue without it?

Copy link

Choose a reason for hiding this comment

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

Without this line, the context menu doesn't have keyboard focus after being displayed (at least on Windows 11), so navigation with the arrow keys doesn't work, and you can't even close the context menu with Escape. This line should definitely be merged into the main branch, even without the rest of the patch.

@Lakritzator
Copy link
Collaborator

Looks like a very good PR, much nicer as I could have made as I am not the best routed event etc WPF programmer.

@Lakritzator Lakritzator changed the base branch from master to develop November 29, 2021 21:23
@Lakritzator
Copy link
Collaborator

The only thing which surprised me was that by default the keyboard does nothing. Wouldn't it make sense to wire up the defaults?

@Lakritzator
Copy link
Collaborator

I tried to merge the most recent changes, but couldn't modify it, so I created a PR for it. It was a lot of manual work, so I hope I didn't screw up...

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.

4 participants