How can I avoid shortcut working whit more than required keys?

Question

I'm using the following hook to use shortcuts in my app:
(I adapted it from this repo: https://github.com/arthurtyukayev/use-keyboard-shortcut)

 import { useEffect, useCallback, useReducer } from 'react';

const disabledEventPropagation = e => {
  if (e)
    if (e.stopPropagation) {
      e.stopPropagation();
    } else if (window.event) {
      window.event.cancelBubble = true;
    }
};

const blacklistedTargets = ['INPUT', 'TEXTAREA'];

const keysReducer = (state, action) => {
  switch (action.type) {
    case 'set-key-down':
      const keydownState = { ...state, [action.key]: true };
      return keydownState;
    case 'set-key-up':
      const keyUpState = { ...state, [action.key]: false };
      return keyUpState;
    case 'reset-keys':
      const resetState = { ...action.data };
      return resetState;
    default:
      return state;
  }
};

const useKeyboardShortcut = (shortcutKeys, callback, options) => {
  if (!Array.isArray(shortcutKeys))
    throw new Error('The first parameter to `useKeyboardShortcut` must be an ordered array of `KeyboardEvent.key` strings.');

  if (!shortcutKeys.length)
    throw new Error('The first parameter to `useKeyboardShortcut` must contain atleast one `KeyboardEvent.key` string.');

  if (!callback || typeof callback !== 'function')
    throw new Error('The second parameter to `useKeyboardShortcut` must be a function that will be envoked when the keys are pressed.');

  const { overrideSystem } = options || {};
  const initalKeyMapping = shortcutKeys.reduce((currentKeys, key) => {
    currentKeys[key.toLowerCase()] = false;
    return currentKeys;
  }, {});

  const [keys, setKeys] = useReducer(keysReducer, initalKeyMapping);

  const keydownListener = useCallback(
    assignedKey => keydownEvent => {
      const loweredKey = assignedKey.toLowerCase();

      if (keydownEvent.repeat) return;
      if (blacklistedTargets.includes(keydownEvent.target.tagName)) return;
      if (loweredKey !== keydownEvent.key.toLowerCase()) return;
      if (keys[loweredKey] === undefined) return;

      if (overrideSystem) {
        keydownEvent.preventDefault();
        disabledEventPropagation(keydownEvent);
      }

      setKeys({ type: 'set-key-down', key: loweredKey });
      return false;
    },
    [keys, overrideSystem],
  );

  const keyupListener = useCallback(
    assignedKey => keyupEvent => {
      const raisedKey = assignedKey.toLowerCase();

      if (blacklistedTargets.includes(keyupEvent.target.tagName)) return;
      if (keyupEvent.key.toLowerCase() !== raisedKey) return;
      if (keys[raisedKey] === undefined) return;

      if (overrideSystem) {
        keyupEvent.preventDefault();
        disabledEventPropagation(keyupEvent);
      }

      setKeys({ type: 'set-key-up', key: raisedKey });
      return false;
    },
    [keys, overrideSystem],
  );

  useEffect(() => {
    if (!Object.values(keys).filter(value => !value).length) {
      callback(keys);
      const newKeyMapping = initalKeyMapping;
      newKeyMapping.control = true;
      setKeys({ type: 'reset-keys', data: initalKeyMapping });
    } else setKeys({ type: null });
  }, [callback, keys]);

  useEffect(() => {
    shortcutKeys.forEach(k => window.addEventListener('keydown', keydownListener(k)));
    return () => shortcutKeys.forEach(k => window.removeEventListener('keydown', keydownListener(k)));
  }, []);

  useEffect(() => {
    shortcutKeys.forEach(k => window.addEventListener('keyup', keyupListener(k)));
    return () => shortcutKeys.forEach(k => window.removeEventListener('keyup', keyupListener(k)));
  }, []);
};

export default useKeyboardShortcut;

So, than I use the following lines to create and handle the shortcuts:

const shortcutUndo = ['Control', 'Z'];
useKeyboardShortcut(shortcutUndo, handleUndoShortcut);

The problem is wverytime I press Ctrl+Z+[any other key] the ctr+z shortcut still run, but it shouldn't be running. I can't think any way to prevent this, does anyone has a clue?

Answer 1

You can fix this issue by rethinking how shortcuts are triggered. More precisely, your hook can do the following:

let sequence: string[] = [];

function onKeyDown(key: string) {
    // Push key to sequence
    sequence.push(key);
}

function onKeyUp(key: string) {
    // Store the sequence before we pop from it
    const seq = [...sequence];
    const popped = sequence.pop(); // last pressed-down key

    if (popped === undefined) return; // Sequence was already empty (dropped)
    if (popped !== key) {
        // User didn't release the last key, drop the whole sequence
        sequence = [];
    }

    // Check whether the sequence matches
    if (arrayEquals(seq, shortcutKeys)) {
        // Shortcut got triggered, do whatever
    }
}
}

this is just for the key press logic, I omitted all React and hook-related things

The idea is that while the user keeps pressing down keys, you store the sequence of keys that were pressed. Then when the user releases the last pressed key, we check if the right sequence was pressed. If the user released a totally different key, we reset the whole sequence to be safe (although maybe you just want to remove the unpressed key).

This leads to the following happening when (un)pressing the following keys:

let sequence = [];

const shortcutKeys = ['Control', 'Z'];

function onKeyDown(key) {
    // Push key to sequence
    sequence.push(key);
    console.log('onKeyDown', key, '-> [' + sequence.join('+') + ']');
}

function onKeyUp(key) {
    console.log('onKeyUp', key);
    // Store the sequence before we pop from it
    const seq = [...sequence];
    const popped = sequence.pop(); // last pressed-down key

    if (popped === undefined) return; // Sequence was already empty (dropped)
    if (popped !== key) {
        // User didn't release the last key, drop the whole sequence
        sequence = [];
        console.log('\tSequence dropped');
        return;
    }

    console.log('\t->', '[' + sequence.join('+') + ']');

    // Check whether the sequence matches
    if (seq.join('+') === shortcutKeys.join('+')) {
        // Shortcut got triggered, do whatever
        console.log('Shortcut triggered!');
    }
}

console.log('== Example 1 ==');
onKeyDown('Control');
onKeyDown('Z');
onKeyUp('Z'); // Shortcut triggered
onKeyUp('Control');
// Shortcut triggered once

console.log('== Example 2 ==');
onKeyDown('Control');
onKeyDown('Shift');
onKeyUp('Shift');
onKeyDown('Z');
onKeyUp('Z'); // Shortcut triggered
onKeyUp('Control');

console.log('== Example 3 ==');
onKeyDown('Control');
onKeyDown('Z');
onKeyUp('Z'); // Shortcut triggered
onKeyDown('Z');
onKeyUp('Z'); // Shortcut triggered
onKeyDown('Z');
onKeyUp('Z'); // Shortcut triggered
onKeyUp('Control');

console.log('== Example 4 ==');
onKeyDown('Control');
onKeyDown('Z');
onKeyDown('S');
onKeyDown('S');
onKeyUp('Z');
onKeyUp('Control');
// Shortcut never triggered

console.log('== Example 5 ==');
onKeyDown('Control');
onKeyDown('Z');
onKeyUp('Control');
onKeyUp('Z');
// Shortcut never triggered

This solves your issue, while also allowing users to "spam" the shortcut without having to repeat the whole sequence.

A small change you can make is replacing the arrayEquals with something that only checks if the combination is present, but doesn't care about the order. E.g. this would make Ctrl+Shift+Z and Shift+Ctrl+Z (but also Ctrl+Z+Shift) all equal.