Fix keyboard shortcut

src/components/ScreenWrapper.js

@@ -8,6 +8,7 @@ import styles, {getSafeAreaPadding} from '../styles/styles';
 import HeaderGap from './HeaderGap';
 import KeyboardShortcut from '../libs/KeyboardShortcut';
 import onScreenTransitionEnd from '../libs/onScreenTransitionEnd';
+import Navigation from '../libs/Navigation/Navigation';
 
 const propTypes = {
     /** Array of additional styles to add */
@@ -32,9 +33,6 @@ const propTypes = {
     navigation: PropTypes.shape({
         // Method to attach listener to Navigation state.
         addListener: PropTypes.func.isRequired,
-
-        // Returns to the previous navigation state e.g. if this is inside a Modal we will dismiss it
-        goBack: PropTypes.func,
     }),
 };
 
@@ -45,7 +43,6 @@ const defaultProps = {
     onTransitionEnd: () => {},
     navigation: {
         addListener: () => {},
-        goBack: () => {},
     },
 };
 
@@ -59,7 +56,7 @@ class ScreenWrapper extends React.Component {
 
     componentDidMount() {
         this.unsubscribeEscapeKey = KeyboardShortcut.subscribe('Escape', () => {
-            this.props.navigation.goBack();
+            Navigation.dismissModal();
         }, [], true);
 
         this.unsubscribeTransitionEnd = onScreenTransitionEnd(this.props.navigation, () => {

src/libs/KeyboardShortcut/index.js

@@ -5,16 +5,18 @@ const events = {};
 /**
  * Checks if an event for that key is configured and if so, runs it.
  * @param {Event} event
+ * @private
  */
 function bindHandlerToKeyupEvent(event) {
     if (events[event.keyCode] === undefined) {
         return;
     }
 
     const eventCallbacks = events[event.keyCode];
+    const reversedEventCallbacks = [...eventCallbacks].reverse();
 
     // Loop over all the callbacks
-    eventCallbacks.forEach((callback) => {
+    _.every(reversedEventCallbacks, (callback) => {
         const pressedModifiers = _.all(callback.modifiers, (modifier) => {
             if (modifier === 'shift' && !event.shiftKey) {
                 return false;
@@ -50,7 +52,7 @@ function bindHandlerToKeyupEvent(event) {
             return false;
         });
         if (!pressedModifiers || pressedExtraModifiers) {
-            return;
+            return true;
         }
 
         // If configured to do so, prevent input text control to trigger this event
@@ -59,20 +61,69 @@ function bindHandlerToKeyupEvent(event) {
             || event.target.nodeName === 'TEXTAREA'
             || event.target.contentEditable === 'true'
         )) {
-            return;
+            return true;
         }
 
         if (_.isFunction(callback.callback)) {
             callback.callback(event);
         }
         event.preventDefault();
+
+        // Short circuit the loop because the event is triggered
+        return false;
     });
 }
 
 // Make sure we don't add multiple listeners
 document.removeEventListener('keydown', bindHandlerToKeyupEvent, {capture: true});
 document.addEventListener('keydown', bindHandlerToKeyupEvent, {capture: true});
 
+/**
+ * Returns keyCode for a given key
+ * @param {String} key The key to watch, i.e. 'K' or 'Escape'
+ * @returns {Number} The key's keyCode, i.e. 75 or 27
+ * @private
+ */
+function getKeyCode(key) {
+    // For keys that have longer names we must catch and return the correct key key.charCodeAt(0) would return the
+    // key code for 'E' (the letter at index 0 in the string) not 'Escape'
+    switch (key) {
+        case 'Enter':
+            return 13;
+        case 'Escape':
+            return 27;
+        default:
+            return key.charCodeAt(0);
+    }
+}
+
+/**
+ * Unsubscribes to a keyboard event.
+ * @param {Number} key The key to stop watching
+ * @private
+ */
+function unsubscribe(key) {
+    const keyCode = getKeyCode(key);
+    events[keyCode].pop();
+}
+
+/**
+ * Subscribes to a keyboard event.
+ * @param {String} key The key to watch, i.e. 'K' or 'Escape'
+ * @param {Function} callback The callback to call
+ * @param {String|Array} modifiers Can either be shift or control
+ * @param {Boolean} captureOnInputs Should we capture the event on inputs too?
+ * @returns {Function} clean up method
+ */
+function subscribe(key, callback, modifiers = 'shift', captureOnInputs = false) {
+    const keyCode = getKeyCode(key);
+    if (events[keyCode] === undefined) {
+        events[keyCode] = [];
+    }
+    events[keyCode].push({callback, modifiers: _.isArray(modifiers) ? modifiers : [modifiers], captureOnInputs});
+    return () => unsubscribe(key);
+}
+
 /**
  * Module storing the different keyboard shortcut
  *
@@ -85,49 +136,7 @@ document.addEventListener('keydown', bindHandlerToKeyupEvent, {capture: true});
  * The "subClass" is used by pages to bind /unbind with no worries
  */
 const KeyboardShortcut = {
-    /**
-     * Returns keyCode for a given key
-     * @param {String} key The key to watch, i.e. 'K' or 'Escape'
-     * @returns {Number} The key's keyCode, i.e. 75 or 27
-     */
-    getKeyCode(key) {
-        // For keys that have longer names we must catch and return the correct key key.charCodeAt(0) would return the
-        // key code for 'E' (the letter at index 0 in the string) not 'Escape'
-        switch (key) {
-            case 'Enter':
-                return 13;
-            case 'Escape':
-                return 27;
-            default:
-                return key.charCodeAt(0);
-        }
-    },
-
-    /**
-     * Subscribes to a keyboard event.
-     * @param {String} key The key to watch, i.e. 'K' or 'Escape'
-     * @param {Function} callback The callback to call
-     * @param {String|Array} modifiers Can either be shift or control
-     * @param {Boolean} captureOnInputs Should we capture the event on inputs too?
-     * @returns {Function} clean up method
-     */
-    subscribe(key, callback, modifiers = 'shift', captureOnInputs = false) {
-        const keyCode = this.getKeyCode(key);
-        if (events[keyCode] === undefined) {
-            events[keyCode] = [];
-        }
-        events[keyCode].push({callback, modifiers: _.isArray(modifiers) ? modifiers : [modifiers], captureOnInputs});
-        return () => this.unsubscribe(key);
-    },
-
-    /**
-     * Unsubscribes to a keyboard event.
-     * @param {Number} key The key to stop watching
-     */
-    unsubscribe(key) {
-        const keyCode = this.getKeyCode(key);
-        delete events[keyCode];
-    },
+    subscribe,
 };
 
 export default KeyboardShortcut;

src/libs/Navigation/AppNavigator/AuthScreens.js

@@ -174,24 +174,23 @@ class AuthScreens extends React.Component {
 
         Timing.end(CONST.TIMING.HOMEPAGE_INITIAL_RENDER);
 
+        let searchShortcutModifiers = ['control'];
+        let groupShortcutModifiers = ['control', 'shift'];
+
+        if (getOperatingSystem() === CONST.OS.MAC_OS) {
+            searchShortcutModifiers = ['meta'];
+            groupShortcutModifiers = ['meta', 'shift'];
+        }
+
         // Listen for the key K being pressed so that focus can be given to
         // the chat switcher, or new group chat
         // based on the key modifiers pressed and the operating system
-        if (getOperatingSystem() === CONST.OS.MAC_OS) {
-            KeyboardShortcut.subscribe('K', () => {
-                Navigation.navigate(ROUTES.SEARCH);
-            }, ['meta'], true);
-            KeyboardShortcut.subscribe('K', () => {
-                Navigation.navigate(ROUTES.NEW_GROUP);
-            }, ['meta', 'shift'], true);
-        } else {
-            KeyboardShortcut.subscribe('K', () => {
-                Navigation.navigate(ROUTES.SEARCH);
-            }, ['control'], true);
-            KeyboardShortcut.subscribe('K', () => {
-                Navigation.navigate(ROUTES.NEW_GROUP);
-            }, ['control', 'shift'], true);
-        }
+        this.unsubscribeSearchShortcut = KeyboardShortcut.subscribe('K', () => {
+            Navigation.navigate(ROUTES.SEARCH);
+        }, searchShortcutModifiers, true);
+        this.unsubscribeGroupShortcut = KeyboardShortcut.subscribe('K', () => {
+            Navigation.navigate(ROUTES.NEW_GROUP);
+        }, groupShortcutModifiers, true);
     }
 
     shouldComponentUpdate(nextProps) {
@@ -211,7 +210,12 @@ class AuthScreens extends React.Component {
     }
 
     componentWillUnmount() {
-        KeyboardShortcut.unsubscribe('K');
+        if (this.unsubscribeSearchShortcut) {
+            this.unsubscribeSearchShortcut();
+        }
+        if (this.unsubscribeGroupShortcut) {
+            this.unsubscribeGroupShortcut();
+        }
         NetworkConnection.stopListeningForReconnect();
         clearInterval(this.interval);
         this.interval = null;