greena13 / react-hotkeys

Declarative hotkey and focus area management for React
https://github.com/greena13/react-hotkeys
ISC License
2.15k stars 160 forks source link

[BUG] Single key getting activated on multi-key sequence #234

Open mikeyyyzhao opened 5 years ago

mikeyyyzhao commented 5 years ago

Describe the bug I have hotkeys set to c and v, but when the user types command+c (to copy), or command+v (to paste), the c and v hotkeys I have set up get triggered as well.

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

Expected behavior The hot keys attached to c and v are only triggered on when the user types c and v, not when they are in combination with other keys.

Platform (please complete the following information): Web

martin-svk commented 5 years ago

We have the same problem, and interesting is that this looks to be a special command+something while i.e. ctrl+something works like expected, meaning it triggers the right shortcut.

mschaef-da commented 5 years ago

Same problem here, as well. This is an (admittedly heavy-handed) workaround, as long as you don't have any modified shortcuts:

import { configure } from 'react-hotkeys';

configure({
    ignoreEventsCondition: e => {
        return e.altKey || e.metaKey || e.ctrlKey;
    }
});
mikeyyyzhao commented 5 years ago

Unfortunately this won't work for us since we are using command + some letters to trigger events. Any other suggestions are more than welcome!!!

amcdnl commented 5 years ago

I have a similar situation where I have a hotkey for SHIFT+C, however, if you do CMD+SHIFT+C it still fires.

greena13 commented 5 years ago

Hi @mikeyyyzhao, @martin-svk, @mschaef-da and @amcdnl. Thank you all for sharing that you're having this issue.

This should not be the case as sub-matching is disabled by default.

The cmd key has been particularly difficult to accommodate, so that is likely the cause of this issue. Are any of you able to share verbose logs of pressing the key combination for the longer key combination and having the shorter one triggered? That would help me confirm some things about your project setups.

sctape commented 4 years ago

@greena13 I'm also dealing with this problem. I have a single handler mapped to d that is triggering when a user pressing cmd+d to bookmark the page. allowCombinationSubmatches is set to the default value of false. Here's the verbose logging from the interaction:

FocusOnlyKeyEventStrategy.js?98f4:153 HotKeys (F1๐Ÿ“—-C0๐Ÿ”บ-P0๐Ÿ”บ:) Focused. 

FocusOnlyKeyEventStrategy.js?98f4:156 HotKeys (F1๐Ÿ“—-C0๐Ÿ”บ-P0๐Ÿ”บ:) Component options:
 {
    "actions": {
        "NAVIGATE_DAY_RANGE": [
            {
                "prefix": "",
                "actionName": "NAVIGATE_DAY_RANGE",
                "sequenceLength": 1,
                "id": "d",
                "keyDictionary": {
                    "d": true
                },
                "keyEventType": 0,
                "size": 1
            }
        ],
        "NAVIGATE_WEEK_RANGE": [
            {
                "prefix": "",
                "actionName": "NAVIGATE_WEEK_RANGE",
                "sequenceLength": 1,
                "id": "w",
                "keyDictionary": {
                    "w": true
                },
                "keyEventType": 0,
                "size": 1
            }
        ],
        "NAVIGATE_MONTH_RANGE": [
            {
                "prefix": "",
                "actionName": "NAVIGATE_MONTH_RANGE",
                "sequenceLength": 1,
                "id": "m",
                "keyDictionary": {
                    "m": true
                },
                "keyEventType": 0,
                "size": 1
            }
        ]
    },
    "handlers": {
        "NAVIGATE_DAY_RANGE": "function (e) {\n        if (_this.props.currentView === _view.COVERAGE_VIEW && range === _view.DAY_VIEW) {\n          return;\n        }\n\n        _this.props.navigateToRange(range);\n      }",
        "NAVIGATE_WEEK_RANGE": "function (e) {\n        if (_this.props.currentView === _view.COVERAGE_VIEW && range === _view.DAY_VIEW) {\n          return;\n        }\n\n        _this.props.navigateToRange(range);\n      }",
        "NAVIGATE_MONTH_RANGE": "function (e) {\n        if (_this.props.currentView === _view.COVERAGE_VIEW && range === _view.DAY_VIEW) {\n          return;\n        }\n\n        _this.props.navigateToRange(range);\n      }"
    },
    "componentId": 0,
    "options": {
        "defaultKeyEvent": "keydown"
    }
}
FocusOnlyKeyEventStrategy.js?98f4:298 HotKeys (F1๐Ÿ“—-E1๐Ÿ’š-C0๐Ÿ”บ-P0๐Ÿ”บ:) New 'Meta' keydown event.
FocusOnlyKeyEventStrategy.js?98f4:533 HotKeys (F1๐Ÿ“—-E1๐Ÿ’š-C0๐Ÿ”บ-P0๐Ÿ”บ:) Added 'Meta' to current combination: 'Meta'.
FocusOnlyKeyEventStrategy.js?98f4:538 HotKeys (F1๐Ÿ“—-E1๐Ÿ’š-C0๐Ÿ”บ-P0๐Ÿ”บ:) Key history: [
    {
        "keys": {
            "Meta": [
                [
                    0,
                    0,
                    0
                ],
                [
                    1,
                    0,
                    0
                ]
            ]
        },
        "ids": [
            "Meta"
        ],
        "keyAliases": {}
    }
].
FocusOnlyKeyEventStrategy.js?98f4:648 HotKeys (F1๐Ÿ“—-E1๐Ÿ’š-C0๐Ÿ”บ-P0๐Ÿ”บ:) Attempting to find action matching 'Meta' keydown . . .
AbstractKeyEventStrategy.js?b56c:405 HotKeys (F1๐Ÿ“—-E1๐Ÿ’š-C0๐Ÿ”บ-P0๐Ÿ”บ:) Internal key mapping:
 {
    "": {
        "actionConfigs": {
            "d": {
                "prefix": "",
                "sequenceLength": 1,
                "id": "d",
                "keyDictionary": {
                    "d": true
                },
                "size": 1,
                "events": {
                    "0": {
                        "actionName": "NAVIGATE_DAY_RANGE",
                        "handler": "function (e) {\n        if (_this.props.currentView === _view.COVERAGE_VIEW && range === _view.DAY_VIEW) {\n          return;\n        }\n\n        _this.props.navigateToRange(range);\n      }"
                    }
                }
            },
            "w": {
                "prefix": "",
                "sequenceLength": 1,
                "id": "w",
                "keyDictionary": {
                    "w": true
                },
                "size": 1,
                "events": {
                    "0": {
                        "actionName": "NAVIGATE_WEEK_RANGE",
                        "handler": "function (e) {\n        if (_this.props.currentView === _view.COVERAGE_VIEW && range === _view.DAY_VIEW) {\n          return;\n        }\n\n        _this.props.navigateToRange(range);\n      }"
                    }
                }
            },
            "m": {
                "prefix": "",
                "sequenceLength": 1,
                "id": "m",
                "keyDictionary": {
                    "m": true
                },
                "size": 1,
                "events": {
                    "0": {
                        "actionName": "NAVIGATE_MONTH_RANGE",
                        "handler": "function (e) {\n        if (_this.props.currentView === _view.COVERAGE_VIEW && range === _view.DAY_VIEW) {\n          return;\n        }\n\n        _this.props.navigateToRange(range);\n      }"
                    }
                }
            }
        },
        "order": null
    }
}
AbstractKeyEventStrategy.js?b56c:429 HotKeys (F1๐Ÿ“—-E1๐Ÿ’š-C0๐Ÿ”บ-P0๐Ÿ”บ:) No matching actions found for 'Meta' keydown.
FocusOnlyKeyEventStrategy.js?98f4:298 HotKeys (F1๐Ÿ“—-E3๐Ÿ’›-C0๐Ÿ”บ-P0๐Ÿ”บ:) New (simulated) 'Meta' keypress event.
FocusOnlyKeyEventStrategy.js?98f4:538 HotKeys (F1๐Ÿ“—-E3๐Ÿ’›-C0๐Ÿ”บ-P0๐Ÿ”บ:) Key history: [
    {
        "keys": {
            "Meta": [
                [
                    1,
                    0,
                    0
                ],
                [
                    1,
                    2,
                    0
                ]
            ]
        },
        "ids": [
            "Meta"
        ],
        "keyAliases": {}
    }
].
FocusOnlyKeyEventStrategy.js?98f4:637 HotKeys (F1๐Ÿ“—-E3๐Ÿ’›-C0๐Ÿ”บ-P0๐Ÿ”บ:) Ignored 'Meta' keypress because it doesn't have any keypress handlers.
FocusOnlyKeyEventStrategy.js?98f4:298 HotKeys (F1๐Ÿ“—-E4๐Ÿ’œ-C0๐Ÿ”บ-P0๐Ÿ”บ:) New 'd' keydown event.
FocusOnlyKeyEventStrategy.js?98f4:533 HotKeys (F1๐Ÿ“—-E4๐Ÿ’œ-C0๐Ÿ”บ-P0๐Ÿ”บ:) Added 'd' to current combination: 'Meta+d'.
FocusOnlyKeyEventStrategy.js?98f4:538 HotKeys (F1๐Ÿ“—-E4๐Ÿ’œ-C0๐Ÿ”บ-P0๐Ÿ”บ:) Key history: [
    {
        "keys": {
            "Meta": [
                [
                    1,
                    0,
                    0
                ],
                [
                    1,
                    2,
                    0
                ]
            ],
            "d": [
                [
                    0,
                    0,
                    0
                ],
                [
                    1,
                    0,
                    0
                ]
            ]
        },
        "ids": [
            "Meta+d"
        ],
        "keyAliases": {}
    }
].
FocusOnlyKeyEventStrategy.js?98f4:648 HotKeys (F1๐Ÿ“—-E4๐Ÿ’œ-C0๐Ÿ”บ-P0๐Ÿ”บ:) Attempting to find action matching 'Meta+d' keydown . . .
AbstractKeyEventStrategy.js?b56c:405 HotKeys (F1๐Ÿ“—-E4๐Ÿ’œ-C0๐Ÿ”บ-P0๐Ÿ”บ:) Internal key mapping:
 {
    "": {
        "actionConfigs": {
            "d": {
                "prefix": "",
                "sequenceLength": 1,
                "id": "d",
                "keyDictionary": {
                    "d": true
                },
                "size": 1,
                "events": {
                    "0": {
                        "actionName": "NAVIGATE_DAY_RANGE",
                        "handler": "function (e) {\n        if (_this.props.currentView === _view.COVERAGE_VIEW && range === _view.DAY_VIEW) {\n          return;\n        }\n\n        _this.props.navigateToRange(range);\n      }"
                    }
                }
            },
            "w": {
                "prefix": "",
                "sequenceLength": 1,
                "id": "w",
                "keyDictionary": {
                    "w": true
                },
                "size": 1,
                "events": {
                    "0": {
                        "actionName": "NAVIGATE_WEEK_RANGE",
                        "handler": "function (e) {\n        if (_this.props.currentView === _view.COVERAGE_VIEW && range === _view.DAY_VIEW) {\n          return;\n        }\n\n        _this.props.navigateToRange(range);\n      }"
                    }
                }
            },
            "m": {
                "prefix": "",
                "sequenceLength": 1,
                "id": "m",
                "keyDictionary": {
                    "m": true
                },
                "size": 1,
                "events": {
                    "0": {
                        "actionName": "NAVIGATE_MONTH_RANGE",
                        "handler": "function (e) {\n        if (_this.props.currentView === _view.COVERAGE_VIEW && range === _view.DAY_VIEW) {\n          return;\n        }\n\n        _this.props.navigateToRange(range);\n      }"
                    }
                }
            }
        },
        "order": [
            "d",
            "w",
            "m"
        ]
    }
}
AbstractKeyEventStrategy.js?b56c:418 HotKeys (F1๐Ÿ“—-E4๐Ÿ’œ-C0๐Ÿ”บ-P0๐Ÿ”บ:) Found action that matches 'Meta+d': NAVIGATE_DAY_RANGE. Calling handler . . .
FocusOnlyKeyEventStrategy.js?98f4:508 HotKeys (F1๐Ÿ“—-E4๐Ÿ’œ-C0๐Ÿ”บ-P0๐Ÿ”บ:) Stopping further event propagation.
FocusOnlyKeyEventStrategy.js?98f4:298 HotKeys (F1๐Ÿ“—-E6โค๏ธ-C0๐Ÿ”บ-P0๐Ÿ”บ:) New (simulated) 'd' keypress event.
FocusOnlyKeyEventStrategy.js?98f4:538 HotKeys (F1๐Ÿ“—-E6โค๏ธ-C0๐Ÿ”บ-P0๐Ÿ”บ:) Key history: [
    {
        "keys": {
            "Meta": [
                [
                    1,
                    0,
                    0
                ],
                [
                    1,
                    2,
                    0
                ]
            ],
            "d": [
                [
                    1,
                    0,
                    0
                ],
                [
                    1,
                    2,
                    0
                ]
            ]
        },
        "ids": [
            "Meta+d"
        ],
        "keyAliases": {}
    }
].
FocusOnlyKeyEventStrategy.js?98f4:637 HotKeys (F1๐Ÿ“—-E6โค๏ธ-C0๐Ÿ”บ-P0๐Ÿ”บ:) Ignored 'Meta+d' keypress because it doesn't have any keypress handlers.
FocusOnlyKeyEventStrategy.js?98f4:298 HotKeys (F1๐Ÿ“—-E7๐Ÿ’š-C0๐Ÿ”บ-P0๐Ÿ”บ:) New 'Meta' keyup event.
FocusOnlyKeyEventStrategy.js?98f4:538 HotKeys (F1๐Ÿ“—-E7๐Ÿ’š-C0๐Ÿ”บ-P0๐Ÿ”บ:) Key history: [
    {
        "keys": {
            "Meta": [
                [
                    1,
                    2,
                    0
                ],
                [
                    1,
                    2,
                    1
                ]
            ],
            "d": [
                [
                    1,
                    0,
                    0
                ],
                [
                    1,
                    2,
                    0
                ]
            ]
        },
        "ids": [
            "Meta+d"
        ],
        "keyAliases": {}
    }
].
FocusOnlyKeyEventStrategy.js?98f4:637 HotKeys (F1๐Ÿ“—-E7๐Ÿ’š-C0๐Ÿ”บ-P0๐Ÿ”บ:) Ignored 'Meta+d' keyup because it doesn't have any keyup handlers.
FocusOnlyKeyEventStrategy.js?98f4:298 HotKeys (F1๐Ÿ“—-E9๐Ÿ’›-C0๐Ÿ”บ-P0๐Ÿ”บ:) New (simulated) 'd' keyup event.
FocusOnlyKeyEventStrategy.js?98f4:538 HotKeys (F1๐Ÿ“—-E9๐Ÿ’›-C0๐Ÿ”บ-P0๐Ÿ”บ:) Key history: [
    {
        "keys": {
            "Meta": [
                [
                    1,
                    2,
                    0
                ],
                [
                    1,
                    2,
                    1
                ]
            ],
            "d": [
                [
                    1,
                    2,
                    0
                ],
                [
                    1,
                    2,
                    2
                ]
            ]
        },
        "ids": [
            "Meta+d"
        ],
        "keyAliases": {}
    }
].
FocusOnlyKeyEventStrategy.js?98f4:637 HotKeys (F1๐Ÿ“—-E9๐Ÿ’›-C0๐Ÿ”บ-P0๐Ÿ”บ:) Ignored 'Meta+d' keyup because it doesn't have any keyup handlers.
FocusOnlyKeyEventStrategy.js?98f4:220 HotKeys (F1๐Ÿ“—-C0๐Ÿ”บ-P0๐Ÿ”บ:) Lost focus.
subalee commented 4 years ago

This is from our app. The config and log is below.

configure({
    ignoreRepeatedEventsWhenKeyHeldDown: false,
    logLevel: 'verbose',
});
17:36:44.943 rollbar.umd.min.js?8391:1 HotKeys (GLOBAL-E16๐Ÿ’œ): New 's' keydown event (that has NOT passed through React app).
17:36:44.943 rollbar.umd.min.js?8391:1 HotKeys (GLOBAL-E16๐Ÿ’œ): Added 's' to current combination: 's'.
17:36:44.943 rollbar.umd.min.js?8391:1 HotKeys (GLOBAL-E16๐Ÿ’œ): Key history: [
    {
        "keys": {
            "s": [
                [
                    0,
                    0,
                    0
                ],
                [
                    1,
                    0,
                    0
                ]
            ]
        },
        "ids": [
            "s"
        ],
        "keyAliases": {}
    }
].
17:36:44.943 rollbar.umd.min.js?8391:1 HotKeys (GLOBAL-E16๐Ÿ’œ): Attempting to find action matching 's' keydown . . .
17:36:44.943 rollbar.umd.min.js?8391:1 HotKeys (GLOBAL-E16๐Ÿ’œ-C0๐Ÿ”บ): Internal key mapping:
 {
    "": {
        "actionConfigs": {
            "s": {
                "prefix": "",
                "sequenceLength": 1,
                "id": "s",
                "keyDictionary": {
                    "s": true
                },
                "size": 1,
                "events": {
                    "0": {
                        "actionName": "SHOW_SNAPSHOT",
                        "handler": "function func() {\n            return null;\n          }"
                    }
                }
            },
            "r": {
                "prefix": "",
                "sequenceLength": 1,
                "id": "r",
                "keyDictionary": {
                    "r": true
                },
                "size": 1,
                "events": {
                    "0": {
                        "actionName": "SHOW_RESULTS",
                        "handler": "function func() {\n            return null;\n          }"
                    }
                }
            }
        },
        "order": [
            "s",
            "r"
        ]
    }
}
17:36:44.943 rollbar.umd.min.js?8391:1 HotKeys (GLOBAL-E16๐Ÿ’œ-C0๐Ÿ”บ): Found action that matches 's': SHOW_SNAPSHOT. Calling handler . . .
17:36:44.944 rollbar.umd.min.js?8391:1 HotKeys (GLOBAL-E16๐Ÿ’œ-C0๐Ÿ”บ): Stopping further event propagation.
17:36:44.944 rollbar.umd.min.js?8391:1 HotKeys (GLOBAL-E16๐Ÿ’œ): Searching no further, as handler has been found (and called).
17:36:44.944 rollbar.umd.min.js?8391:1 HotKeys (GLOBAL-E17๐Ÿงก): New 's' keypress event (that has NOT passed through React app).
17:36:44.944 rollbar.umd.min.js?8391:1 HotKeys (GLOBAL-E17๐Ÿงก): Key history: [
    {
        "keys": {
            "s": [
                [
                    1,
                    0,
                    0
                ],
                [
                    1,
                    1,
                    0
                ]
            ]
        },
        "ids": [
            "s"
        ],
        "keyAliases": {}
    }
].
17:36:44.944 rollbar.umd.min.js?8391:1 HotKeys (GLOBAL-E17๐Ÿงก): Ignored 's' keypress because it doesn't have any keypress handlers.
17:36:45.004 rollbar.umd.min.js?8391:1 HotKeys (GLOBAL-E18โค๏ธ): New 's' keyup event (that has NOT passed through React app).
17:36:45.005 rollbar.umd.min.js?8391:1 HotKeys (GLOBAL-E18โค๏ธ): Key history: [
    {
        "keys": {
            "s": [
                [
                    1,
                    1,
                    0
                ],
                [
                    1,
                    1,
                    1
                ]
            ]
        },
        "ids": [
            "s"
        ],
        "keyAliases": {}
    }
].
17:36:45.005 rollbar.umd.min.js?8391:1 HotKeys (GLOBAL-E18โค๏ธ): Ignored 's' keyup because it doesn't have any keyup handlers.
17:36:47.287 rollbar.umd.min.js?8391:1 HotKeys: Window focused - clearing key history
ogtfaber commented 4 years ago

I found a good workaround for this, define the keys "command+c" and just handle it with an empty function. Now the handling defaults to whatever the browser's default behavior is.

const keyMap = {
  IGNORE: ["command+c", "command+f"]
}

const handlers = {
  IGNORE: () => {}
}

You can still have a keymap for the invidivual "c" and "f" key.

subalee commented 4 years ago

@ogtfaber Thanks, i've tried the workaround approach and it works.

craigcarlyle commented 3 years ago

For anybody that's still experiencing this issue: I was able to solve it by defining the sequence in an array.

// Does NOT work as expected
TOGGLE_LEFT_MENU: {
    name: 'Toggle left menu',
    sequence: 'm',
},
// Works as expected
TOGGLE_LEFT_MENU: {
    name: 'Toggle left menu',
    sequences: ['m'],
},