🐛 Fix #187: Add root prop to HotKeys to allow root HotKeys components…

README.md

@@ -131,6 +131,7 @@ export default MyNode;
     - [Other keyboard event listeners are no longer being triggered](#other-keyboard-event-listeners-are-no-longer-being-triggered)
     - [Actions aren't being triggered when using withHotKeys](#actions-arent-being-triggered-when-using-withhotkeys)
     - [Actions aren't being triggered for HotKeys](#actions-arent-being-triggered-for-hotkeys)
+    - [React Hotkeys thinks I'm holding down a key I've released](#react-hotkeys-thinks-im-holding-down-a-key-ive-released)
     - [Blue border appears around children of HotKeys](#blue-border-appears-around-children-of-hotkeys)
 - [Stability & Maintenance](#stability--maintenance)
 - [Contribute, please!](#contribute-please)
@@ -453,6 +454,12 @@ However, it [does require that its children be wrapped in a DOM-mounted node](#H
    * to get a reference to the node, so you can call .focus() on it
    */
   innerRef: {undefined}
+
+  /**
+   * Whether this is the root HotKeys node - this enables some special 
+   * behaviour
+   */
+  root={false}
   >
 
   /**
@@ -1220,6 +1227,16 @@ Also make sure your React application is not calling `stopPropagation()` on the
 
 Finally, make sure your key event are not coming from one of the [tags ignored by react-hotkeys](#Ignoring-events).
 
+#### React Hotkeys thinks I'm holding down a key I've released
+
+This can happen when you have an action handler that either unmounts the `<HotKeys>` component, or focuses an area of the application where there is no ancestor `<HotKeys>`. The solution is to add a `<HotKeys>` component with the `root` prop to the top of your application - or at least high enough to *not* be unmounted or unfocused by your action handler.
+
+```javascript
+<HotKeys root>
+  // The parts of your application that are re-rendered or unfocused here
+</HotKeys>
+```
+
 #### Blue border appears around children of HotKeys
 
 `react-hotkeys` adds a `<div />` around its children with a `tabindex="-1"` to allow them to be programmatically focused. This can result in browsers rendering a blue outline around them to visually indicate that they are the elements in the document that is currently in focus.

index.d.ts

@@ -53,6 +53,11 @@ export interface HotKeysEnabledProps extends GlobalHotKeysProps {
    * Function to call when this component loses focus in the browser
    */
   onBlur?: () => void;
+
+  /**
+   * Whether this is the root HotKeys node - this enables some special behaviour
+   */
+  root?: boolean;
 }
 
 /**

src/lib/strategies/FocusOnlyKeyEventStrategy.js

@@ -309,6 +309,14 @@ class FocusOnlyKeyEventStrategy extends AbstractKeyEventStrategy {
       KeyEventBitmapIndex.keydown
     );
 
+    /**
+     * We need to record the position of the component that is currently dealing with
+     * the event, in case the component defines a handler for that event that changes
+     * the focus or content in the render tree, causing the component to be de-registered
+     * and have its position lost
+     */
+    const componentPosition = this._getComponentPosition(componentId);
+
     if (responseAction === EventResponse.handled) {
       const keyInCurrentCombination = !!this._getCurrentKeyState(_key);
 
@@ -323,7 +331,7 @@ class FocusOnlyKeyEventStrategy extends AbstractKeyEventStrategy {
 
     this._simulateKeyPressesMissingFromBrowser(event, _key, focusTreeId, componentId, options);
 
-    this._updateEventPropagationHistory(componentId);
+    this._updateEventPropagationHistory(componentId, componentPosition);
 
     return false;
   }
@@ -417,6 +425,14 @@ class FocusOnlyKeyEventStrategy extends AbstractKeyEventStrategy {
       );
     }
 
+    /**
+     * We need to record the position of the component that is currently dealing with
+     * the event, in case the component defines a handler for that event that changes
+     * the focus or content in the render tree, causing the component to be de-registered
+     * and have its position lost
+     */
+    const componentPosition = this._getComponentPosition(componentId);
+
     /**
      * We attempt to find a handler of the event, only if it has not already
      * been handled and should not be ignored
@@ -431,7 +447,7 @@ class FocusOnlyKeyEventStrategy extends AbstractKeyEventStrategy {
       );
     }
 
-    this._updateEventPropagationHistory(componentId);
+    this._updateEventPropagationHistory(componentId, componentPosition);
 
     return shouldDiscardFocusTreeId;
   }
@@ -486,6 +502,14 @@ class FocusOnlyKeyEventStrategy extends AbstractKeyEventStrategy {
       );
     }
 
+    /**
+     * We need to record the position of the component that is currently dealing with
+     * the event, in case the component defines a handler for that event that changes
+     * the focus or content in the render tree, causing the component to be de-registered
+     * and have its position lost
+     */
+    const componentPosition = this._getComponentPosition(componentId);
+
     /**
      * We attempt to find a handler of the event, only if it has not already
      * been handled and should not be ignored
@@ -500,7 +524,7 @@ class FocusOnlyKeyEventStrategy extends AbstractKeyEventStrategy {
      */
     this._simulateKeyUpEventsHiddenByCmd(event, _key, focusTreeId, componentId, options);
 
-    this._updateEventPropagationHistory(componentId);
+    this._updateEventPropagationHistory(componentId, componentPosition);
 
     return shouldDiscardFocusId;
   }
@@ -544,10 +568,12 @@ class FocusOnlyKeyEventStrategy extends AbstractKeyEventStrategy {
   _ignoreEvent(event, componentId) {
     this.currentEvent.ignored = true;
 
+    const componentPosition = this._getComponentPosition(componentId);
+
     if(this._stopEventPropagationAfterIgnoringIfEnabled(event, componentId)) {
-      this._updateEventPropagationHistory(componentId, { forceReset: true });
+      this._updateEventPropagationHistory(componentId, componentPosition, { forceReset: true });
     } else {
-      this._updateEventPropagationHistory(componentId);
+      this._updateEventPropagationHistory(componentId, componentPosition);
     }
   }
 
@@ -577,11 +603,11 @@ class FocusOnlyKeyEventStrategy extends AbstractKeyEventStrategy {
     return previousComponentPosition === -1 || previousComponentPosition >= this._getComponentPosition(componentId);
   }
 
-  _updateEventPropagationHistory(componentId, options = { forceReset: false }) {
+  _updateEventPropagationHistory(componentId, componentPosition, options = { forceReset: false }) {
     if (options.forceReset || this._isFocusTreeRoot(componentId)) {
       this._clearEventPropagationState();
     } else {
-      this.eventPropagationState.previousComponentPosition = this._getComponentPosition(componentId);
+      this.eventPropagationState.previousComponentPosition = componentPosition;
     }
   }
 

src/withHotKeys.js

@@ -113,7 +113,12 @@ function withHotKeys(Component, hotKeysOptions = {}) {
        * component mounts. If false, changes to the keyMap and handlers
        * props will be ignored
        */
-      allowChanges: PropTypes.bool
+      allowChanges: PropTypes.bool,
+
+      /**
+       * Whether this is the root HotKeys node - this enables some special behaviour
+       */
+      root: PropTypes.bool
     };
 
     constructor(props) {
@@ -152,7 +157,7 @@ function withHotKeys(Component, hotKeysOptions = {}) {
          * Props used by HotKeys that should not be passed down to its focus trap
          * component
          */
-        keyMap, handlers, allowChanges,
+        keyMap, handlers, allowChanges, root,
 
         ...props
       } = this.props;
@@ -181,7 +186,7 @@ function withHotKeys(Component, hotKeysOptions = {}) {
     _shouldBindKeyListeners() {
       const keyMap = getKeyMap(this.props);
 
-      return !isEmpty(keyMap) || (
+      return !isEmpty(keyMap) || this.props.root || (
         Configuration.option('enableHardSequences') && this._handlersIncludeHardSequences(keyMap, getHandlers(this.props))
       );
     }

test/HotKeys/focus-only/RootProp.spec.js

@@ -0,0 +1,73 @@
+import React from 'react';
+import {mount} from 'enzyme';
+import {expect} from 'chai';
+import sinon from 'sinon';
+
+import FocusableElement from '../../support/FocusableElement';
+import KeyEventManager from '../../../src/lib/KeyEventManager';
+import KeyCode from '../../support/Key';
+
+import {HotKeys} from '../../../src/';
+
+describe('HotKeys root prop:', function () {
+  describe('when a HotKeys component has a root prop value of true', function () {
+    beforeEach(function () {
+      this.keyMap = {
+        'NEXT': 'a',
+      };
+
+      this.nextHandler = sinon.spy();
+
+      const handlers = {
+        'NEXT': () => {
+          this.secondElement.focus();
+
+          this.nextHandler();
+        }
+      };
+
+      this.wrapper = mount(
+        <HotKeys root>
+          <HotKeys keyMap={this.keyMap} handlers={handlers}>
+            <div className='firstChildElement' />
+          </HotKeys>
+
+          <div className='secondChildElement' />
+        </HotKeys>
+      );
+
+      this.firstElement = new FocusableElement(this.wrapper, '.firstChildElement');
+      this.secondElement = new FocusableElement(this.wrapper, '.secondChildElement');
+
+      this.firstElement.focus();
+
+      expect(this.firstElement.isFocused()).to.equal(true);
+    });
+
+    describe('and nested HotKeys component has a handler that changes focus to another element bound to keydown', function () {
+      it('then the root HotKeys still records the keyup event', function() {
+        this.firstElement.keyDown(KeyCode.A);
+
+        expect(this.nextHandler).to.have.been.calledOnce;
+
+        expect(this.secondElement.isFocused()).to.equal(true);
+
+        this.secondElement.keyPress(KeyCode.A);
+        this.secondElement.keyUp(KeyCode.A);
+
+        expect(KeyEventManager.getInstance()._focusOnlyEventStrategy.keyCombinationHistory).to.eql([
+          {
+            'keys': {
+              'a': [
+                [true, true, false],
+                [true, true, true]
+              ]
+            },
+            'ids': ['a'],
+            'keyAliases': {}
+          }
+        ]);
+      });
+    });
+  });
+});