Skip to content

Commit

Permalink
🐛 Fix #166: Fix over-zealous modifier flag discrepency correction
Browse files Browse the repository at this point in the history
  • Loading branch information
greena13 committed May 30, 2019
1 parent 1978a90 commit ded2bff
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 5 deletions.
15 changes: 13 additions & 2 deletions src/lib/strategies/AbstractKeyEventStrategy.js
Original file line number Diff line number Diff line change
Expand Up @@ -1111,17 +1111,28 @@ class AbstractKeyEventStrategy {
* Synchronises the key combination history to match the modifier key flag attributes
* on new key events
* @param {KeyboardEvent} event - Event to check the modifier flags for
* @param {String} key - Name of key that events relates to
* @param {KeyEventBitmapIndex} keyEventBitmapIndex - Index of event type
* @protected
*/
_checkForModifierFlagDiscrepancies(event) {
_checkForModifierFlagDiscrepancies(event, key, keyEventBitmapIndex) {
/**
* If a new key event is received with modifier key flags that contradict the
* key combination history we are maintaining, we can surmise that some keyup events
* for those modifier keys have been lost (possibly because the window lost focus).
* We update the key combination to match the modifier flags
*/
Object.keys(ModifierFlagsDictionary).forEach((modifierKey) => {
const modifierStillPressed = this._keyIsCurrentlyDown(modifierKey);
/**
* When a modifier key is being released (keyup), it sets its own modifier flag
* to false. (e.g. On the keyup event for Command, the metaKey attribute is false).
* If this the case, we want to handle it using the main algorithm.
*/
if (key === modifierKey && keyEventBitmapIndex === KeyEventBitmapIndex.keyup) {
return;
}

const modifierStillPressed = this._keyIsCurrentlyDown(modifierKey);

ModifierFlagsDictionary[modifierKey].forEach((attributeName) => {
if (event[attributeName] === false && modifierStillPressed) {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/strategies/FocusOnlyKeyEventStrategy.js
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ class FocusOnlyKeyEventStrategy extends AbstractKeyEventStrategy {
`New ${describeKeyEvent(event, key, keyEventBitmapIndex)} event.`
);

this._checkForModifierFlagDiscrepancies(event);
this._checkForModifierFlagDiscrepancies(event, key, keyEventBitmapIndex);
}

return EventResponse.handled;
Expand Down
4 changes: 2 additions & 2 deletions src/lib/strategies/GlobalKeyEventStrategy.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,10 @@ class GlobalKeyEventStrategy extends AbstractKeyEventStrategy {
* @param {KeyboardEvent} event - Event containing the key name and state
*/
handleKeydown(event) {
this._checkForModifierFlagDiscrepancies(event);

const _key = normalizeKeyName(getEventKey(event));

this._checkForModifierFlagDiscrepancies(event, _key, KeyEventBitmapIndex.keydown);

const reactAppResponse = this._howReactAppRespondedTo(
event,
_key,
Expand Down
51 changes: 51 additions & 0 deletions test/HotKeys/BindingToKeysWithSimulatedKeypressEvents.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import React from 'react';
import { expect } from 'chai';
import {mount} from 'enzyme';
import sinon from 'sinon';

import Key from '../support/Key';
import KeyEventManager from '../../src/lib/KeyEventManager';
import { HotKeys } from '../../src';
import FocusableElement from '../support/FocusableElement';

describe('Binding to keys with simulated keypress events:', function () {
context('when HotKeys has actions for a the keydown and keyup event', () => {
beforeEach(function () {
this.keyMap = {
'ACTION1': {sequence: 'command', action: 'keydown'},
'ACTION2': {sequence: 'command', action: 'keyup'},
};

this.action1Handler = sinon.spy();
this.action2Handler = sinon.spy();

this.handlers = {
'ACTION1': this.action1Handler,
'ACTION2': this.action2Handler,
};

this.wrapper = mount(
<HotKeys keyMap={this.keyMap} handlers={this.handlers}>
<div className="childElement" />
</HotKeys>
);

this.keyEventManager = KeyEventManager.getInstance();

this.targetElement = new FocusableElement(this.wrapper, '.childElement');
this.targetElement.focus();
});

it('then simulates the cmd keypress event and the non-modifier key\'s keypress event (https://github.com/greena13/react-hotkeys/issues/166)', function () {
this.targetElement.keyDown(Key.COMMAND, { metaKey: true });

expect(this.action1Handler).to.have.been.calledOnce;
expect(this.action2Handler).to.not.have.been.called;

this.targetElement.keyUp(Key.COMMAND, { metaKey: false });

expect(this.action1Handler).to.have.been.calledOnce;
expect(this.action2Handler).to.have.been.calledOnce;
});
});
});

0 comments on commit ded2bff

Please sign in to comment.