react-hotkeys icon indicating copy to clipboard operation
react-hotkeys copied to clipboard

[BUG] getApplicationKeyMap() does not provide all bound keys

Open jcready opened this issue 5 years ago • 11 comments

Describe the bug Using the provided example in the docs for displaying a list of active hotkeys which uses getApplicationKeyMap() I am unable to see all hotkeys.

How are you using react hotkeys components? (HotKeys, GlobalHotKeys, IgnoreKeys etc) GlobalHotKeys

Example ~~https://codesandbox.io/s/peaceful-borg-t5b39~~
https://codesandbox.io/s/new-water-4pii9

Expected behavior In the example above I should see "a", "b", "c", and "d" listed under the "Keyboard shortcuts" header

Actual behavior I only see "c" and "d" listed under the "Keyboard shortcuts" header.

Platform (please complete the following information):

  • Version of react-hotkeys: 2.0.0
  • Browser: Chrome
  • OS: Mac OS X

jcready avatar Sep 05 '19 18:09 jcready

Strange, @jcready, can I just confirm all components that you expect to see the keyboard shortcuts for are mounted in the DOM at the time that you attempt to display them?

greena13 avatar Oct 20 '19 13:10 greena13

Yes, see the code sandbox link provided.

jcready avatar Oct 20 '19 14:10 jcready

Sorry, for some reason my example link wasn't the correct one. You should be able to see the issue if you go to this sandbox https://codesandbox.io/s/new-water-4pii9

Open the console tab at the bottom and then when the main window is in focus you should be able to press the following keys: "a", "b", "c", and "d" and see the following output to the console:

action 1
action 2
action 3
action 4

The problem is that in the Shortcuts component I'm rendering it doesn't show "a" or "b".

jcready avatar Oct 21 '19 15:10 jcready

Hi, just reporting that I am also having this issue. I am also using multiple GlobalHotKeys components. Like the code sandbox provided by @jcready, I only see the keymap for the last GlobalHotKeys that I render. In my case, I would ideally like to consolidate all of my handlers into one GlobalHotKeys component, but I am unable to as it breaks the shortcuts (see #219).

I realize your time is limited for resolving these issues @greena13; just want to say that I appreciate the work you've put into this library!

dobrynin avatar Oct 21 '19 22:10 dobrynin

I am also having this issue, and I am only using GlobalHotKeys. I have 5 GlobalHotKeys set up and I only see 1 when I call getApplicationKeyMap().

nora-white avatar Nov 27 '19 19:11 nora-white

I'm seeing the same issue. Only the last global hot keys is impacted.

This project needs a new maintainer so we can get these items fixed.

burtonator avatar May 26 '20 00:05 burtonator

Looking at the code:

globalEventStrategy

https://github.com/greena13/react-hotkeys/blob/master/src/getApplicationKeyMap.js https://github.com/greena13/react-hotkeys/blob/master/src/lib/KeyEventManager.js

This is the offending code I think.

  get applicationKeyMap() {
    return [this.globalEventStrategy, this.focusOnlyEventStrategy].reduce((memo, strategy) => {
      const builder = new ApplicationKeyMapBuilder(strategy.componentTree);
      const keyMap = builder.build();

      return { ...memo, ...keyMap };
    }, {});
  }

burtonator avatar May 26 '20 00:05 burtonator

Looking at this a bit more... the keyMap DOES get registered by each GlobalHotKeys but the result doesn't have all the bindings for some reason.

burtonator avatar May 26 '20 00:05 burtonator

Hi everyone, yesterday faced this issue and accidentally found a workaround.

I have 2 GlobalHotKeys components usages and getApplicationKeyMap returned hotkeys of only one. One of GlobalHotKeys is used in root component of my app, so I tried to wrap whole tree in it(instead of self-closing <GlobalHotKeys />), including second GlobalHotKeys many levels deep. Now getApplicationKeyMap returns all hotkeys to me.

Hope this finding might help to someone.

svekl avatar May 27 '20 08:05 svekl

@svekl's answer worked for me! I have GlobalHotKeys used in two distant parts of the tree, but only one was being returned by getApplicationKeyMap (the first one rendered on the page, as defined in the docs). I wrapped them in another GlobalHotKeys and it works!

Before:

// In Component Foo
// These show up in getApplicationKeyMap() because Foo renders before Bar
<Foo>
  <GlobalHotKeys keyMap={...} />
</Foo>

// In Component Bar
// These don't show up in getApplicationKeyMap()
<Bar>
  <GlobalHotKeys keyMap={...}> />
</Bar>

// In my component GlobalHotkeysDialog
// Only returns keymaps from Foo
const globalKeyMappings = getApplicationKeyMap()

After:

// Root, in my case _app.js
<GlobalHotKeys>
  <Foo />  // contains its own GlobalHotKeys
  <Bar />// contains its own GlobalHotKeys
  <GlobalHotkeysDialog /> // Now displays hotkeys from Foo and Bar
</GlobalHotKeys>

devinhalladay avatar Jan 04 '21 06:01 devinhalladay

Though the above works, it strikes me (per the docs) that this is suboptimal for performance reasons. Is it an intentional feature that we need to wrap all GlobalHotkey usages in another GlobalHotKey, or is this a bug?

If it's a feature, I think this could use better documentation…if not, I'm not quite qualified to write a PR but would love to help if anyone else is.

devinhalladay avatar Jan 04 '21 23:01 devinhalladay