Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Physical Keyboard Input Listener doesn't work without input #1807

Closed
AndrewBudziszek opened this issue Mar 7, 2022 · 11 comments
Closed

Physical Keyboard Input Listener doesn't work without input #1807

AndrewBudziszek opened this issue Mar 7, 2022 · 11 comments
Assignees
Labels
📦 Use case A question regarding implementation

Comments

@AndrewBudziszek
Copy link

AndrewBudziszek commented Mar 7, 2022

Is your feature request related to a problem? Please describe.
When using a physical keyboard, input does not go through the onChange/onKeyPress.

Debug shows that it is hearing the key events but the events are not running through the proper functions.
Caret position reset due to "keyup" event

Digital keyboard works as expected, even without input target.

Describe the solution you'd like
When I type on my keyboard, I expect my input to run thought RSK's built in onChange, onKeyPress, etc functions - even if I don't have a specified input target.

Describe alternatives you've considered
Other solutions would be to implement key listeners outside of RSK.

Additional context
More context can be added based on questions about the requested feature.

@AndrewBudziszek
Copy link
Author

Misunderstood docs

@AndrewBudziszek
Copy link
Author

I do not have an actual input to target. It looks like my keyboard input is picked up by RSK, however, it's not running the input through onChange or onKeyPress. When hitting my keyboard I get this message:
Caret position reset due to "keyup" event but the input does not go through onChange or onKeyPress. Clicking on the digital keyboard gets input through onChange and onKeyPress. I'd expect that the physical keyboard input would always be run though onChange/onKeyPress.

@AndrewBudziszek AndrewBudziszek changed the title Physical Keyboard Input Listener Physical Keyboard Input Listener doesn't work without input Mar 7, 2022
@hodgef hodgef self-assigned this Mar 7, 2022
@hodgef
Copy link
Owner

hodgef commented Mar 7, 2022

Hello @SonnyBrooks, the caret position reset is an internal construct to prevent edge cases. Essentially, when you interact outside of the keyboard, the caret position is set back to null.

What you're asking for might be provided by the following options, but I don't know what's your current implementation so this may vary:

https://hodgef.com/simple-keyboard/documentation/options/physicalkeyboardhighlight/
https://hodgef.com/simple-keyboard/documentation/options/physicalkeyboardhighlightpress/

Regards,
Francisco Hodge

@hodgef hodgef closed this as completed Mar 7, 2022
@hodgef hodgef added the 📦 Use case A question regarding implementation label Mar 7, 2022
@AndrewBudziszek
Copy link
Author

AndrewBudziszek commented Mar 7, 2022

Those options are just visual aren't they?

Here's my implementation:

<Keyboard
  keyboardRef={r => (keyboard.current = r)}
  theme={"hg-theme-default w-custom-theme"}
  layout={layout}
  display={display}
  mergeDisplay={true}
  onChange={e => onChange(e)}
  onKeyPress={e => onKeyPress(e)}
  physicalKeyboardHighlightPress={true}
  physicalKeyboardHighlight={true}
  buttonAttributes={
    [
      {
        attribute: "disabled",
        value: "true",
        buttons: "{space}"
      }
    ]
  }
/>

function onChange(input) {
   if(input.length > 5) 
    keyboard.current.setInput(input.substr(0, 5));
    setCoolAttr(input);
  } else {
    keyboard.current.setInput(input.trim())
 }
}

When I press my physical keyboard, onChange receives no events.

@hodgef
Copy link
Owner

hodgef commented Mar 7, 2022

Check out this demo:
https://codesandbox.io/s/musing-leaf-y5r1d6

When you click the preview space and hit your keyboard keys, are simple-keyboard buttons triggered? Normally that should be the case. The docs links above have a brief explanation for each option. Let me know if it still doesn't work - thanks!

@AndrewBudziszek
Copy link
Author

AndrewBudziszek commented Mar 7, 2022

Yes, the keys work as expected in the demo. I tried to translate them to my implementation:

import Keyboard from 'react-simple-keyboard';

const onChange = (input) => {
  console.log(input);
}

const onKeyPress = (input) => {
  console.log(input)
}

return (
<div className="max-w-screen-sm	m-auto">
  <Keyboard
    keyboardRef={r => (keyboard.current = r)}
    theme={"hg-theme-default myTheme1"}
    layout={layout}
    display={display}
    mergeDisplay={true}
    onChange={onChange}
    onKeyPress={onKeyPress}
    physicalKeyboardHighlightPress={true}
    physicalKeyboardHighlight={true}
    buttonAttributes={
      [
        {
          attribute: "disabled",
          value: "true",
          buttons: "{space}"
        }
      ]
    }
  />
</div>
)

When clicking on the digital keyboard, I get the logs as expected. When I hit my physical keyboard, no logs are recorded.

@hodgef
Copy link
Owner

hodgef commented Mar 8, 2022

Sorry, I still cannot repro the issue that you're experiencing. Could you provide a full sandbox link or test git repository showing the issue?

I will reopen the ticket once I get that. Thanks!

@hodgef hodgef added 🔴 Missing Repro This issue requires a reproducible (runnable & complete) example of the problem being described and removed 📦 Use case A question regarding implementation labels Mar 8, 2022
@AndrewBudziszek
Copy link
Author

AndrewBudziszek commented Mar 8, 2022

Here's my repro - https://codesandbox.io/s/divine-feather-zq2lij?file=/src/components/Game/GameKeyboard.jsx

This issue is being experienced on Chrome, Brave, and Edge.

@hodgef
Copy link
Owner

hodgef commented Mar 8, 2022

It seems to work for me. I added an extra "q" so it matches when I have CapsLock set to off and it triggers it. Then I set CapsLock to on and it does trigger the uppercase Q.

test

Tested on Chrome, Edge on Windows. If you're using Linux please let me know. Thanks

@hodgef hodgef added 📦 Use case A question regarding implementation and removed 🔴 Missing Repro This issue requires a reproducible (runnable & complete) example of the problem being described labels Mar 8, 2022
@AndrewBudziszek
Copy link
Author

I see what my issue is.

On my keyboard, I'm not using SHIFT to type Cap Q. I can add a text-transform on those buttons to make them uppercase and switch my display back to all lowercase.

I don't know if an Ignore Case setting would be helpful or if my use case is too narrow.

Regardless, thanks for looking into this, much appreciated.

@hodgef hodgef added this to Proposal in ⚡️ Enhancements Mar 8, 2022
@hodgef
Copy link
Owner

hodgef commented Mar 8, 2022

Yes that would be helpful indeed, although a bit tricky to roll out due to the current logic. I added this as a Proposal so I can pick it up in the future. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 Use case A question regarding implementation
Projects
⚡️ Enhancements
  
Proposal
Development

No branches or pull requests

2 participants