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

🐛 Fix #201: Always allow submatches for combinations involving command #207

Merged
merged 1 commit into from
Jul 11, 2019

Conversation

greena13
Copy link
Owner

Context

When the Cmd (Meta) key is pressed down with other keys, it hides their keyup events, and only the Meta key keyup event is triggered. react-hotkeys accounts for this and simulates the keyup events for all the keys Meta was pressed down with, when the its keyup event is detected.

However, if you hold down Meta with another key, release the other key and press a third key while Meta is still down, react-hotkeys does not know the first key has been released until you also release Meta. So for example, if you press Meta down, then a, then release a and press b - react-hotkeys thinks you're still holding Meta+a+b until you release Meta. This is undesirable, but (so far as I can come up with) unavoidable, and generally does not cause problems.

When it does cause a problem is if you have actions for both Meta+a and Meta+b: the second action is never triggered. This used to work through submatching: Meta+a+b would still match an action bound to Meta+b. However, v2.0.0-pre7 introduced a new configuration option allowCombinationSubmatches (which is false by default) to address the multiple issues submatches were causing (#161, #181, #175), that disables this. And so we arrive at #201.

This pull request

  • Ignores the allowCombinationSubmatches when Command is currently pressed

import KeyCode from '../support/Key';
import { configure, GlobalHotKeys } from '../../src';

describe('Holding down key shared by actions:', function () {
Copy link
Owner Author

Choose a reason for hiding this comment

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

⬜️ Make this test specific to Cmd

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.

1 participant