Skip to content

Commit

Permalink
🐛 Fix #201: Always allow submatches for combinations involving command
Browse files Browse the repository at this point in the history
  • Loading branch information
greena13 committed Jul 11, 2019
1 parent 9a5c5d9 commit d8e83cf
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,17 @@ import Configuration from '../../lib/Configuration';
* in the key event.
* @param {ActionConfiguration} combinationMatcher Matcher to compare to the
* key state
* @param {boolean} keyupIsHiddenByCmd Whether current combination involves the
* cmd key and keys for which it hides their keyup event
* @returns {boolean} True if the key state has the right amount of keys for a
* match with combinationMatcher to be possible
* @private
*/
function isMatchPossibleBasedOnNumberOfKeys(keyState, combinationMatcher) {
function isMatchPossibleBasedOnNumberOfKeys(keyState, combinationMatcher, keyupIsHiddenByCmd) {
const keyStateKeysNo = Object.keys(keyState.keys).length;
const combinationKeysNo = Object.keys(combinationMatcher.keyDictionary).length;

if (Configuration.option('allowCombinationSubmatches')) {
if (keyupIsHiddenByCmd || Configuration.option('allowCombinationSubmatches')) {
return keyStateKeysNo >= combinationKeysNo;
} else {
/**
Expand Down
6 changes: 6 additions & 0 deletions src/lib/Configuration.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,12 @@ const _defaultConfiguration = {
* Whether to allow combination submatches - e.g. if there is an action bound to
* cmd, pressing shift+cmd will *not* trigger that action when
* allowCombinationSubmatches is false.
*
* @note This option is ignored for combinations involving command (Meta) and
* submatches are <i>always</i> allowed because Meta hides keyup events
* of other keys, so until Command is released, it's impossible to know
* if one of the keys that has also been pressed has been released.
* @see https://github.com/greena13/react-hotkeys/pull/207
* @type {Boolean}
*/
allowCombinationSubmatches: false,
Expand Down
14 changes: 10 additions & 4 deletions src/lib/strategies/AbstractKeyEventStrategy.js
Original file line number Diff line number Diff line change
Expand Up @@ -789,14 +789,13 @@ class AbstractKeyEventStrategy {
return !keyHasNativeKeypress || (keyHasNativeKeypress && this._keyIsCurrentlyDown('Meta'));
} else if (eventType === KeyEventRecordIndex.keyup) {
return (keyupIsHiddenByCmd(keyName) && keyIsCurrentlyTriggeringEvent(
this._getCurrentKeyState('Meta'),
KeyEventRecordIndex.keyup)
this._getCurrentKeyState('Meta'),
KeyEventRecordIndex.keyup)
);
}

return false
}

_cloneAndMergeEvent(event, extra) {
const eventAttributes = Object.keys(ModifierFlagsDictionary).reduce((memo, eventAttribute) => {
memo[eventAttribute] = event[eventAttribute];
Expand Down Expand Up @@ -1069,7 +1068,14 @@ class AbstractKeyEventStrategy {
const combinationId = combinationOrder[combinationIndex];
const combinationMatcher = matchingSequence.combinations[combinationId];

if (isMatchPossibleBasedOnNumberOfKeys(currentKeyState, combinationMatcher)) {
const cmdKeyIsPressed =
this._getCurrentKeyState('Meta') && !keyIsCurrentlyTriggeringEvent(this._getCurrentKeyState('Meta'), KeyEventRecordIndex.keyup);

const keyupIsHidden = cmdKeyIsPressed && Object.keys(combinationMatcher.keyDictionary).some((keyName) => {
return keyupIsHiddenByCmd(keyName);
});

if (isMatchPossibleBasedOnNumberOfKeys(currentKeyState, combinationMatcher, keyupIsHidden)) {
if (this._combinationMatchesKeys(normalizedKeyName, currentKeyState, combinationMatcher, eventRecordIndex)) {

if (Configuration.option('allowCombinationSubmatches')) {
Expand Down
70 changes: 70 additions & 0 deletions test/GlobalHotKeys/HoldingDownKeySharedByActions.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import React from 'react';
import {mount} from 'enzyme';
import {expect} from 'chai';
import sinon from 'sinon';
import simulant from 'simulant';

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

describe('Holding down key shared by actions:', function () {
after(function() {
configure({allowCombinationSubmatches: false });
});

[true, false].forEach((allowCombinationSubmatches) => {
describe(`when allowCombinationSubmatches is ${allowCombinationSubmatches}`, () => {
before(function(){
configure({allowCombinationSubmatches: allowCombinationSubmatches });
});

describe('and there are two actions with combinations that involve cmd (cmd+a and cmd+b) (https://github.com/greena13/react-hotkeys/issues/201)', function () {
beforeEach(function () {
this.keyMap = {
'ACTION1': 'cmd+a',
'ACTION2': 'cmd+b',
};

this.handler1 = sinon.spy();
this.handler2 = sinon.spy();

const handlers = {
'ACTION1': this.handler1,
'ACTION2': this.handler2
};

this.reactDiv = document.createElement('div');
document.body.appendChild(this.reactDiv);

this.wrapper = mount(
<GlobalHotKeys keyMap={ this.keyMap } handlers={ handlers }>
<div className="childElement" />
</GlobalHotKeys>,
{ attachTo: this.reactDiv }
);
});

afterEach(function() {
document.body.removeChild(this.reactDiv);
});

describe('and cmd and \'a\' are pressed, and \'a\' is released and \'b\' is pressed instead', function () {
it('then both actions are triggered', function() {
simulant.fire(this.reactDiv, 'keydown', { key: KeyCode.COMMAND });

simulant.fire(this.reactDiv, 'keydown', { key: KeyCode.A });

expect(this.handler1).to.have.been.calledOnce;

simulant.fire(this.reactDiv, 'keydown', { key: KeyCode.B });
simulant.fire(this.reactDiv, 'keypress', { key: KeyCode.B });

simulant.fire(this.reactDiv, 'keyup', { key: KeyCode.COMMAND });

expect(this.handler2).to.have.been.calledOnce;
});
});
})
});
});
});

0 comments on commit d8e83cf

Please sign in to comment.