From 5145e1f58789990324f34387abf7b473ba107147 Mon Sep 17 00:00:00 2001
From: Aleck Greenham <greenhama13@gmail.com>
Date: Sat, 18 May 2019 17:17:05 +0100
Subject: [PATCH] =?UTF-8?q?=E2=9A=94=20Fix=20#173:=20Ignore=20when=20a=20c?=
 =?UTF-8?q?omponent=20enables=20itself=20twice=20without=20switching=20foc?=
 =?UTF-8?q?us=20tree?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 .../strategies/FocusOnlyKeyEventStrategy.js   | 17 +++++--
 src/withHotKeys.js                            | 13 ++++-
 ...oringEnablingDuplicateComponentIds.spec.js | 50 +++++++++++++++++++
 3 files changed, 76 insertions(+), 4 deletions(-)
 create mode 100644 test/lib/FocusOnlyKeyEventStrategy/IgnoringEnablingDuplicateComponentIds.spec.js

diff --git a/src/lib/strategies/FocusOnlyKeyEventStrategy.js b/src/lib/strategies/FocusOnlyKeyEventStrategy.js
index fac2d45a..e1f03a4d 100644
--- a/src/lib/strategies/FocusOnlyKeyEventStrategy.js
+++ b/src/lib/strategies/FocusOnlyKeyEventStrategy.js
@@ -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) {
@@ -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,
diff --git a/src/withHotKeys.js b/src/withHotKeys.js
index 7f135eae..2b0e6119 100644
--- a/src/withHotKeys.js
+++ b/src/withHotKeys.js
@@ -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
@@ -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;
     }
diff --git a/test/lib/FocusOnlyKeyEventStrategy/IgnoringEnablingDuplicateComponentIds.spec.js b/test/lib/FocusOnlyKeyEventStrategy/IgnoringEnablingDuplicateComponentIds.spec.js
new file mode 100644
index 00000000..cb274534
--- /dev/null
+++ b/test/lib/FocusOnlyKeyEventStrategy/IgnoringEnablingDuplicateComponentIds.spec.js
@@ -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;
+      });
+    });
+  });
+});