dddice
Bug: (Macos) pressing cmd+c or any shortcut that has cmd in it will break any registered shortcuts unless the window goes blur
Suppose we have a sample app like below. Assume <App /> is wrapped with <ShortcutProvider>
Sample application. Please click me!
import { useState, useEffect } from 'react'
import { useShortcut } from 'react-keybind'
import './App.css'
function App() {
const [show, setShow] = useState(false)
const { registerShortcut, unregisterShortcut } = useShortcut()
console.log('hello')
const handleShortcut = (e) => {
console.log(e)
setShow((prev) => !prev)
}
useEffect(() => {
registerShortcut(handleShortcut, ['k'], 'show', 'show')
return () => {
unregisterShortcut(['k'])
}
// eslint-disable-next-line
}, [])
return show ? (
<h1>Shown</h1>
) : (
<h1>Not shown</h1>
)
}
export default App
If I press the cmd key or press cmd+c, the registered shortcut 'k' stops working. This is not the case in Windows though.
Thanks for filing an issue.
Taking a quick look at the sample app attached, there's an issue that could possibly be related.
handleShortcut should be a useCallback and passed as a dependency to the useEffect
const handleShortcut = useCallback(() => {
setShow(prev => !prev);
}, []);
useEffect(() => {
registerShortcut(handleShortcut, ['k'], 'show', 'show')
return () => {
unregisterShortcut(['k'])
}
}, [handleShortcut]);
Past that, the test suite of this library should encompass any edge-cases and they currently pass. If there's an issue with unsetting shortcuts, you're welcome to submit a PR with a failing test case and fix.
I don't mean to ~~brag or~~ be rude but not wrapping it in useCallback and not putting it in the dependency array is only a bad practise but it definitely is not the cause of the issue. I am well aware that not putting the callback in the dep array can potentially cause issues (stale functional reference) but that's precisely why I am passing a callback to setShow instead of using show state value.
The problem might be related to this https://stackoverflow.com/questions/27380018/when-cmd-key-is-kept-pressed-keyup-is-not-triggered-for-any-other-key
To demonstrate this when I am pressing & holding the cmd key followed by pressing and holding the k key. So now I have both cmd & k on hold. The keydown fired for both. But if I now release k there won't be a keyup event for k since cmd is pressed. So to get the keyup event for k I need to release the cmd key before releasing k.
Looking at the code it seems that you are not clearing the keyDowns array since there is no keyup for k when cmd is being held. A simple solution would be to reset the keyDowns ref array after a slight delay irrespective of the keyup event was found or not.
If there's an issue with unsetting shortcuts, you're welcome to submit a PR with a failing test case and fix.
This can't be replicated through a testing library because you are firing the events yourself. While irl behaviour is out of the ordinary.
It's certainly not bragging to be wrong about how to use hooks properly. Regardless, as stated in the original reply, you're welcome to submit a PR with a new test case and fix to address the problem.
