hodgef / simple-keyboard

Javascript Virtual Keyboard - Customizable, responsive and lightweight
https://virtual-keyboard.js.org/
MIT License
2.18k stars 167 forks source link

Special keys like alt, ctrl, shift, etc. are not released when the virtual keyboard loses the focus #1841

Closed sskorol closed 1 year ago

sskorol commented 1 year ago

Simple-keyboard version: ^3.4.202 Browser: Chrome 109 OS: MacOS / Linux

Describe the bug

Expected: all the keys on the virtual keyboard are released. Actual: some keys (in most of the cases - the special ones like alt, ctrl, cmd, shift) are left "unreleased".

Note that it can be "fixed" by pressing the unreleased key once again.

Screenshots UX_BUG

hodgef commented 1 year ago

Hey @sskorol, I decided to make most buttons behave the same, meaning no action unless it's decided by the dev. If I change this now I might introduce a breaking change, therefore I would suggest to use e.preventDefault() when needed.

For example, in the following demo: https://codesandbox.io/s/n5mllkkmyl?file=/src/index.js

There is this code: image

/**
 * Physical Keyboard support
 * Whenever the input is changed with the keyboard, updating SimpleKeyboard's internal input
 */
document.addEventListener("keydown", (event) => {
  // Disabling keyboard input, as some keys (like F5) make the browser lose focus.
  // If you'd like to re-enable it, comment the next line and uncomment the following ones
  // event.preventDefault();
  if (event.key === "Alt") event.preventDefault();
  if (event.key === "F5") event.preventDefault();

  if (event.key === "ArrowUp") event.preventDefault();
  if (event.key === "ArrowDown") event.preventDefault();
  if (event.key === "ArrowLeft") event.preventDefault();
  if (event.key === "ArrowRight") event.preventDefault();
});
hodgef commented 1 year ago

Actually, now that I think of it - this will require you to roll out your own key press mechanism. I will provide a new option.

hodgef commented 1 year ago

Please feel free to pass the following option in the latest version of simple-keyboard:

physicalKeyboardHighlightPreventDefault: true

Thanks, Francisco Hodge

sskorol commented 1 year ago

@hodgef thanks, I've just tried the latest version, but facing the same issue.

Here's the component I use:

const Keyboard = observer(() => {
    const {rosStore: { handleKeyboardShortcuts }} = useStore()
    const [layoutName, setLayoutName] = useState('default')
    const { classes } = useStyles();

    const onKeyPress = (button) => {
        if (button === '{capslock}') {
            setLayoutName(layoutName === 'default' ? 'shift' : 'default')
        }
        const key = syntethicKeysMap[button] || button
        handleKeyboardShortcuts(key)
    }

    const theme = useTheme()

    return (
        <div className='keyboard-wrapper'>
            <SimpleKeyboard
                layoutName={layoutName}
                onKeyPress={onKeyPress}
                theme={`hg-theme-default ${classes.root}`}
                baseClass={'simple-keyboard-main'}
                physicalKeyboardHighlight={true}
                physicalKeyboardHighlightPress={true}
                physicalKeyboardHighlightPreventDefault={true}
                physicalKeyboardHighlightTextColor={theme.palette.common.white}
                physicalKeyboardHighlightBgColor={theme.palette.background.keyboardButtonBgPressed}
                syncInstanceInputs={true}
                mergeDisplay={true}
                layout={layout}
                display={display}
                buttonTheme={buttonTheme}
                buttonAttributes={buttonAttributes}
            />
        </div>
    )
})

export default Keyboard

I also tried to preventDefaults from your previous comment like this:

    const handleKeyDown = (event) => {
        event.preventDefault();
    }

    useEffect(() => {
        document.addEventListener('keydown', handleKeyDown);
        return () => document.removeEventListener('keydown', handleKeyDown);
    }, []);

But it only partially works. For example, alt + f works as expected, but alt + tab causes alt to remain highlighted. It's also reproducible in the sandbox you've shared above.

Any thoughts on what I'm doing wrong?

sskorol commented 1 year ago

I apologies for the confusion. Seems like I posted the issue to the wrong repo. I'm using react-simple-keyboard. And I see you released a simple-keyboard fix. Damn, it's quite late over here. Seems like my brain needs some rest. Just curious if there will be the same fix for the react keyboard?

hodgef commented 1 year ago

No problem, react-simple-keyboard normally pulls deps at 4:00 EST. But I've gone ahead and updated it.

Feel free to try the latest version of react-simple-keyboard (3.5.11) - the fix should be there.

Regards, Francisco Hodge

sskorol commented 1 year ago

@hodgef ok, I checked it. Now this property behaves the same way as I described above in a useEffect with preventDefault snippet: some hotkeys are prevented, the others - not. For example, alt + tab still holds the alt key active after physical releasing. Or I have a custom system hotkey to take screenshots -> alt + p. It also causes the alt key to hang highlighted after releasing.

P.S. I greatly appreciate such quick fixes!

hodgef commented 1 year ago

Hey @sskorol, I have edited the behavior per the commit above, feel free to update and give it a try. If you still have issues, try to repro in the test listener you've created. If you can't block the default behavior in your test, I likely won't be able either (due to browser limitation).

Note: there are combos like alt+tab that are not possible to override. Check out this answer for details: https://stackoverflow.com/a/40434403

Regards, Francisco Hodge

sskorol commented 1 year ago

@hodgef got it. Thanks for the link and a quick fix!

Just wonder if there's an API for releasing all the highlighted buttons at once. I can catch the window blur event which would signalize the focus switch. In this case, if there are some stuck buttons on the virtual keyboard, I'd call some method to release them. Does it make sense?

hodgef commented 1 year ago

Check out keyboard.recurseButtons() https://hodgef.com/simple-keyboard/documentation/methods/recursebuttons/

Most likely you can do something like this

keyboard.recurseButtons(buttonElement => {
  buttonElement.removeAttribute("style");
});
sskorol commented 1 year ago

@hodgef in this case I need to access keyboard ref within a functional component. I tried ref attribute, but it doesn't work. Current value is null. Is there a special property for this?

sskorol commented 1 year ago

Ok, seems like I found the way. Now it correctly handles alt + tab

const Keyboard = observer(() => {
    const keyboard = useRef(null);

    const assignRef = (kb) => {
        keyboard.current = kb
    }

    const handleBlur = (event) => {
        keyboard.current.recurseButtons(buttonElement => {
            buttonElement.removeAttribute("style");
        });
    }

    useEffect(() => {
        window.addEventListener('blur', handleBlur);
        return () => window.removeEventListener('blur', handleBlur);
    }, []);

    return (
        <div className='keyboard-wrapper'>
            <SimpleKeyboard
                onInit={assignRef}
// ...
hodgef commented 1 year ago

You got it! Per the hooks demo there's also keyboardRef:

image https://hodgef.com/simple-keyboard/editor/?d=simple-keyboard/react-simple-keyboard-hooks-demo/tree/master

sskorol commented 1 year ago

Ah, damn, I also tried keyboardRef (as I found it in a source code). But I used it in a wrong way by passing keyboard reference directly like this keyboardRef={keyboard} (haven't noticed it's a function)🤦🏻‍♂️ Now I see what's the correct way of using it. Thanks!

sskorol commented 1 year ago

@hodgef hm, I just noticed that physicalKeyboardHighlightPreventDefault={true} blocks all the inputs I have in the app. I can't type anything there. Moreover, they are in completely different components on the other screen. So I wonder, how does this option may affect the global inputs state. Note that I use @mui/material/InputBase, but never get onChange event fired.

hodgef commented 1 year ago

Well, the change is a preventDefault on keydown/keyup: https://github.com/hodgef/simple-keyboard/blame/master/src/lib/services/PhysicalKeyboard.ts#L31

I can see this issue happening if your inputs are reliant on these listeners.

What I can do is to only target the modifier keys instead of all keys, however it will still affect anything that relies on those. I will give that a try and update this thread.

Regards, Francisco Hodge

hodgef commented 1 year ago

I have limited the preventDefault to modifier keys only. If you want to access the isModifierKey method elsewhere in your logic, you can access it through keyboard.physicalKeyboard.isModifierKey. Regards

sskorol commented 1 year ago

It looks much better now! Thank you!

mujeebcpy commented 6 months ago

there is a typo in isModifierKey f instead of d