useKeyPress doesn't track repeated combination keypresses until the entire combination is released

[adwilk]

Describe the Bug

When using useKeyPress to track combinations of keys, for example Meta+z, repeated keypresses are not captured if you continue to hold down the Meta key.

Your Example Website or App

https://codesandbox.io/s/recursing-driscoll-stvwou?file=/src/App.js

Steps to Reproduce the Bug or Issue

When const comboPressed = useKeyPress("Meta+z");:

• Press and hold Meta
• Press and hold z
• comboPressed is true
• Release z
• comboPressed is still true and so additional presses of z can't be detected
• Release Meta
• comboPressed is now false

The inverse does not have the same behaviour:

• Press and hold z
• Press and hold Meta
• comboPressed is true
• Release Meta
• comboPressed is now false and so additional presses of z can be detected

Expected behavior

As a user I would expect that after I press Meta and z, and then continue to hold Meta but release z that the pressed state of useKeyPress("Meta+z") is false.

Screenshots or Videos

No response

Platform

• OS: macOS
• Browser: Chrome
• Version: 103.0.5060.53

Additional context

No response

[joeyballentine]

Can confirm. Had to not use this because using useKeyPress made repeatedly undo-ing unbearable.

[moklick]

It seems that on macOs, the keyup event doesn't get fired in combination with 'Meta'. Need to look deeper into this and see how we can fix this.

[joeyballentine]

It seems that on macOs, the keyup event doesn't get fired in combination with 'Meta'. Need to look deeper into this and see how we can fix this.

It's not just macos. It happens to me on windows as well

[moklick]

Ok, thanks. We will need to check how other libs handle this

[allenhwkim]

IMO, it should use keydown event instead of keypress event. The following code is what I have tried without useKeyPress API, and it seems working

Note. this example is using state management library

import * as React from 'react';
import {KeyboardEvent} from 'react';
import ReactFlow from 'reactflow';
import 'reactflow/dist/style.css';

import useStore from './store';

function Flow() {
  const { nodes, edges, onNodesChange, onEdgesChange, onConnect } = useStore();

  const onKeyDown = (event : KeyboardEvent) => {
    const ctrl = event.ctrlKey ? 'Control-' : '';
    const alt = event.altKey ? 'Alt-' : '';
    const meta = event.metaKey ? 'Meta-' : '';
    const shift = event.shiftKey ? 'Shift-' : '';
    const key = `${ctrl}${alt}${shift}${meta}${event.key}`;
    if (!['Alt', 'Shift', 'Meta', 'Control'].includes(event.key)) {
      console.log(`${key} pressed`);
    }
  }

  return (
    <ReactFlow 
      tabIndex={0}
      onKeyDown={(e) => onKeyDown(e)}
      nodes={nodes}
      edges={edges}
      onNodesChange={onNodesChange}
      onEdgesChange={onEdgesChange}
      onConnect={onConnect}
    ></ReactFlow>
  );
}

export default Flow;

[Max-ChenFei]

For example, we usekeypress hook const copy = useKeyPress('Control+c');

From the code https://github.com/wbkd/react-flow/blob/f79620f4c0c1ab96eb01986988e50eb5274f9d54/packages/core/src/hooks/useKeyPress.ts#L70-L72 Here even if we just release one key (for example c here) for combination keys, all pressedkeys (control, c) will be clear so that another held key (Control) is not memorized, so if we pressed again, the pressedkeys will be only c, so it will not match Control+c.

A possible solution I tested is just to delete the key that was released. pressedKeys.current.delete(event[keyOrCode]); at line 72.

Hi @moklick any ideas?

[Vochsel]

Just chiming back in here to say @Max-ChenFei 's answer seems to work.

if (isMatchingKey(keyCodes, pressedKeys.current, true)) {
    setKeyPressed(false);
}

pressedKeys.current.delete(event[keyOrCode]);

in file https://github.com/wbkd/react-flow/blob/993a778b80cc1e80a47983ed75407b579313a73c/packages/core/src/hooks/useKeyPress.ts#L70-L75

Moving out the delete keyCode and just setting key pressed to false seems to work.

[Max-ChenFei]

Hi, @Vochsel just a temp solution, I add onKeyDown event to the parent of the ReatFlow and use the default keydown event.

 <div
        ref={sceneDomRef}
        tabIndex={0}
        onKeyDown={(e) => {
          if (
            e.target === sceneDomRef.current ||
            (e.target as HTMLElement).classList.contains('react-flow__node')
          )
            onKeyDown(e, sceneState ?? undefined, sceneDomRef);
        }}
        style={{ outline: 'none' }}
      >

        <ReactFlow
....

in onKeyDown function, something like these if (e.code === 'KeyC' && e.ctrlKey)

[moklick]

thanks, I will check it.

[Vochsel]

Chiming back in here to say I'm sometimes hitting an edge cases where no key press works till the blur reset handler is refired...

[moklick]

Yep, unfortunately this doesn't work. For the explained case it works better, but it introduces a new bug :/ I would love to use a third party lib for this, but I couldn't find a good one.

[joeyballentine]

@moklick in chaiNNer we use this package: https://www.npmjs.com/package/react-hotkeys-hook and it works quite well

[Iaotle]

@joeyballentine this fixed my issue as well. @moklick I suggest a move to this handler, as it is more feature-rich and does not interfere unlike the one in react-flow. Changing useShortcut from the copy-paste example to:

function useShortcut(keyCode, callback: Function, active: boolean | undefined): void {
    useHotkeys(keyCode, () => callback(), { enabled: active }, [active, callback]);
}

solved my issue where copy-paste commands that were not meant to be intercepted at all would trigger only on 2nd or 3rd attempt.