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

[BUG] Pressing modifier keys while other keys is held triggers other key, and prevents other keys #1115

Open
psychedelicious opened this issue Jan 9, 2024 · 3 comments
Labels
enhancement New feature or request Version 5.x These tickets will be solved in Version 5.x

Comments

@psychedelicious
Copy link

Describe the bug

When a key is held down and a modifier key is pressed, the first key triggers.

To Reproduce

https://codesandbox.io/p/sandbox/long-waterfall-r3v76q?file=%2Fsrc%2FApp.tsx%3A21%2C3

Listeners are attached to window for "space" and also for the hook, logging to console on call

  1. Press and release space. Logs as expected from both window listener and hook
  2. Press and hold space, then press and release a modifier. The window listener does not fire (expected), but the hotkeys hook fires (unexpected)
  3. Press and hold a modifier, then press and release space. The window listener fires (expected) but the hook does not (unexpected)

Expected behavior

The behaviour should be the same as the window listener. Callbacks should not be called when a modifier is pressed.

I tried ignoreModifiers but that didn't change anything - I don't think we'd expect it to anyways base on my understanding of that option

Desktop (please complete the following information):

  • OS: macOS 14.0 (23A344)
  • Browser: chrome 120.0.6099.199, FF 122.0b6

Additional context

I tried versions 4.4.0, 4.4.1, 4.4.2 and 4.4.3 and they all seem to have the same issue.

@JohannesKlauss
Copy link
Owner

Yes, this is currently "by design". The hook listens to every keystroke and saves all currently held keys. If another key is pressed it looks at all held down keys and triggers each callback that matches. Otherwise combinations like a+s are not possible.
But, I agree that this is somewhat counter intuitive. Maybe it makes sense to only check for held down keys on combinations where it is necessary, something like a+s, because other combos like space or meta+s don't need that. All information is present in the event.

Thanks for raising this.

@JohannesKlauss JohannesKlauss added enhancement New feature or request Version 5.x These tickets will be solved in Version 5.x labels Jan 9, 2024
@psychedelicious
Copy link
Author

psychedelicious commented Jan 9, 2024

Thanks for the explanation. I should explain a bit more why this is a problem with a real-world example.

Our project has some modal hotkeys and this behaviour interferes.

For example, while shift is held, interactions with numbers use finer steps than normal. If you are dragging an object around, it normally snaps to a grid, but while you hold shift it doesn't snap.

While space is held, the move tool is temporarily activated (it's a different tool normally).

A common use pattern is to activate the move tool with space, move an object around a bit, then also hold shift to get finer control of it.

Unfortunately this doesn't work with the current behaviour, because the hook reports a space keyup/down when you keyup/down on shift, causing the tool to change when it shouldn't. I couldn't figure out how to derive the actual state of the space key when this occurs other than to store additional state outside the callback.

I suppose I could use e.g. isHotkeyPressed within the keydown callback to double-check that space is held down, but that feels like a lot of fanagling. There's also the second arg to the callback but it seems to not have accurate information in it.

@JohannesKlauss
Copy link
Owner

Yes I agree. This hook wasn't intended to be used in such complicated scenarios, haha, but I'll gladly improve it to get to work for this example. But it is not that simple to implement and would be a breaking change. So this needs to go into version 5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Version 5.x These tickets will be solved in Version 5.x
Projects
None yet
Development

No branches or pull requests

2 participants