Skip to content

Commit

Permalink
Fix #173: Ignore when a component enables itself twice without swit…
Browse files Browse the repository at this point in the history
…ching focus tree
  • Loading branch information
greena13 committed May 18, 2019
1 parent 69a933f commit 5145e1f
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 4 deletions.
17 changes: 14 additions & 3 deletions src/lib/strategies/FocusOnlyKeyEventStrategy.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,9 @@ class FocusOnlyKeyEventStrategy extends AbstractKeyEventStrategy {
* @param {HandlersMap} actionNameToHandlersMap - Map of actions to handler functions
* @param {Object} options Hash of options that configure how the actions
* and handlers are associated and called.
* @returns {[FocusTreeId, ComponentId]} The current focus tree's ID and a unique
* component ID to assign to the focused HotKeys component and passed back
* when handling a key event
* @returns {FocusTreeId|undefined} The current focus tree's ID or undefined if the
* the <tt>componentId</tt> has already been registered (shouldn't normally
* occur).
*/
enableHotKeys(componentId, actionNameToKeyMap = {}, actionNameToHandlersMap = {}, options) {
if (this.resetOnNextFocus || this.keyMaps) {
Expand All @@ -161,6 +161,17 @@ class FocusOnlyKeyEventStrategy extends AbstractKeyEventStrategy {
this.resetOnNextFocus = false;
}

if (this._getComponent(componentId)) {
/**
* The <tt>componentId</tt> has already been registered - this occurs when the
* same component has somehow managed to be focused twice, without being blurred
* in between.
*
* @see https://github.com/greena13/react-hotkeys/issues/173
*/
return undefined;
}

this._addComponentToList(
componentId,
actionNameToKeyMap,
Expand Down
13 changes: 12 additions & 1 deletion src/withHotKeys.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import KeyEventManager from './lib/KeyEventManager';
import isEmpty from './utils/collection/isEmpty';
import KeyCombinationSerializer from './lib/KeyCombinationSerializer';
import backwardsCompatibleContext from './utils/backwardsCompatibleContext';
import isUndefined from './utils/isUndefined';

/**
* Wraps a React component in a HotKeysEnabled component, which passes down the
Expand Down Expand Up @@ -269,7 +270,17 @@ function withHotKeys(Component, hotKeysOptions = {}) {
this._getComponentOptions()
);

this._focusTreeIdsPush(focusTreeId);
if (!isUndefined(focusTreeId)) {
/**
* focusTreeId should never normally be undefined, but this return state is
* used to indicate that a component with the same componentId has already
* registered as focused/enabled (again, a condition that should not normally
* occur, but apparently can for as-yet unknown reasons).
*
* @see https://github.com/greena13/react-hotkeys/issues/173
*/
this._focusTreeIdsPush(focusTreeId);
}

this._focused = true;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { expect } from 'chai';
import sinon from 'sinon';

import KeyEventManager from '../../../src/lib/KeyEventManager';

describe('Ignoring enabling duplicate component ids:', function () {
context('when the FocusOnlyKeyEventStrategy registers a component ID', () => {
beforeEach(function () {
this.keyEventManager = new KeyEventManager();
this.eventStrategy = this.keyEventManager._focusOnlyEventStrategy;

this.handler = sinon.spy();

this.componentId = this.eventStrategy.registerKeyMap({});
});

context('and it has already registered that component ID', () => {
beforeEach(function () {
this.keyMap = {ACTION1: { sequence: 'a', action: 'keydown' } };

this.eventStrategy.enableHotKeys(
this.componentId,
this.keyMap,
{ACTION1: this.handler},
{},
{}
);

expect(this.eventStrategy.componentList[0].componentId).to.eql(this.componentId);
expect(this.eventStrategy.componentList.length).to.eql(1);
});

it('then returns undefined and does not add the component again \
(https://github.com/greena13/react-hotkeys/issues/173)', function () {
const result = this.eventStrategy.enableHotKeys(
this.componentId,
this.keyMap,
{ACTION1: this.handler},
{},
{}
);

expect(this.eventStrategy.componentList[0].componentId).to.eql(this.componentId);
expect(this.eventStrategy.componentList.length).to.eql(1);

expect(result).to.be.undefined;
});
});
});
});

0 comments on commit 5145e1f

Please sign in to comment.