Skip to content

Fix for HotKey false activations with lower modifiers#307

Open
chris-beedie wants to merge 2 commits intomoses-palmer:masterfrom
chris-beedie:master
Open

Fix for HotKey false activations with lower modifiers#307
chris-beedie wants to merge 2 commits intomoses-palmer:masterfrom
chris-beedie:master

Conversation

@chris-beedie
Copy link
Copy Markdown

If you have multiple hotkeys defined with the same key, but different modifiers you can have false activations. e.g. if we define hotkeys:

<ctrl>+a
<shift>+a
<ctrl>+<shift>+a

All three will be triggered when pressing ctrl+shift+a. This is due to state being held locally in the hotkey and keys only added to the state if they exist as part of the hotkey combo.

I've removed the key in self._keys check before adding to state as a quick fix, but maintaining state globally may be a better solution.

@moses-palmer
Copy link
Copy Markdown
Owner

Thank you for your contribution!

However, I'm not entirely certain that the current behaviour is a bug. It is easy to reason about; a hot-key will be triggered once its keys are pressed. If I press and hold A, Shift, Ctrl or Ctrl, Shift, A, the hot-keys will be triggered, albeit in different order.

A possible modification of the current behaviour, that I think you would agree on, is to special-case modifiers; trigger hot-keys only when the non-modifier key is pressed, and only the registered modifiers are simultaneously pressed.

@chris-beedie
Copy link
Copy Markdown
Author

chris-beedie commented Sep 24, 2020

ok, I see your point. I had only really considered the hotkeys in the more traditional form of modifiers followed by action key. The current implementation does allow for something a bit more exotic. A special case to handle the more traditional use case would suit me, and I'd imagine the majority of use cases

@btonasse
Copy link
Copy Markdown

I was about to open a bug report for this when I noticed this pull request. I can't think of any application that allows for a hotkey like +a to activate when you press +a. It's counter-intuitive to have it work otherwise IMHO

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.

3 participants