search

tiny-pilot / tinypilot

Use your Raspberry Pi as a browser-based KVM.
https://tinypilotkvm.com
MIT License
3.05k stars 255 forks source link

Space key broken on virtual keyboard #729

Closed jotaen closed 3 years ago

jotaen commented 3 years ago

When I press the “space” key on the virtual (on-screen) keyboard, it sends over keycode 32 which the backend claims to be unrecognized. When pressing the space key on my real keyboard, it sends code Space.

They are also displayed differently then in the key history. I’m not sure, though, which one I like better there, but I tend to favour the latter (Space).

I’ve tested the entire virtual keyboard again, the space key seems to be the only one broken.

Screenshot 2021-06-22 at 20 23 20 Screenshot 2021-06-22 at 20 26 16
mtlynch commented 3 years ago

Ah good catch. This is a simple fix:

https://github.com/tiny-pilot/tinypilot/blob/00d98c56d3b49fcb4846936884eeee950f9fd903/app/templates/custom-elements/on-screen-keyboard.html#L231

I think it should just be code="Space".

We used to send codes based on numeric keycodes rather than string keycodes, and it looks like I missed the space bar key when I switched over:

https://github.com/tiny-pilot/tinypilot/pull/355/files#diff-e0b9f192d24c17d8c778fda31f7c194db8f00b3ab6ec5424d0e38d07ce75895eL236

jotaen commented 3 years ago

Ah terrific! What’s your preference regarding the history tile? Space or ?

mtlynch commented 3 years ago

Oh sorry, Space is my preference for the key history.

jotaen4tinypilot commented 3 years ago

(I’ll take this, but waiting for https://github.com/tiny-pilot/tinypilot/pull/731 to avoid conflicts.)

jotaen commented 3 years ago

On a scale from 1 to 10, how important is the Space vs. ` for us? The keystroke history [relies onKeyboardEvent.keyfor the display value, not onKeyboardEvent.code](https://github.com/tiny-pilot/tinypilot/blob/d3d15d5cbb631ace321a399e50314a14fd975b33/app/static/js/app.js#L96) (which we have the canonicalize-function for). So unfortunately we’d need to introduce a dedicated check for it somewhere. It’s not super bad of course (probably like 3 lines of code for anif (evt.key === " ") return "Space";`) but for me it wouldn’t be worth it.

mtlynch commented 3 years ago

Yeah, like a 3/10 importance. We can punt on it.

jotaen4tinypilot commented 3 years ago

👍 I think so too. We can still do it later, but I initially thought it was an easy win which we can make happen without new special cases.