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

UI: Fix keyboard shortcuts for other keyboard layouts #224

Merged
merged 1 commit into from Nov 26, 2022

Conversation

d0sboots
Copy link
Collaborator

A prior change changed from event.key to event.code. I'm not sure why this commit was made, since there's no further explanation in the commit message. But using event.code breaks non-QWERTY keyboard layouts, so reverting improves my (Dvorak) experience greatly.
Also, it's strongly warned against at MDN:
https://developer.mozilla.org/en-US/docs/Web/API/Element/keydown_event#event_properties

This also fixes ScriptEditorRoot.tsx, where some code snuck in that wasn't using any of the constants.

This reverts commit 016a9a8.

Testing

Tried the various hotkeys, they work correctly for Dvorak now. In particular, Ctrl-X (which is physically Ctrl-B) now cuts text instead of exiting to the terminal in the script editor.

Switching to Qwerty, the hotkeys match their "usual" behavior. I didn't detect any weird cases, and I tried a bunch in both layouts.

I'm not sure why this commit was made, since there's no further
explanation in the commit message. But using event.code breaks
non-QWERTY keyboard layouts, so reverting improves my (Dvorak)
experience greatly. Also, it's strongly warned against at MDN:
https://developer.mozilla.org/en-US/docs/Web/API/Element/keydown_event#keyboardevent.code

This also fixes ScriptEditorRoot.tsx, where some code snuck in that
wasn't using *any* of the constants.

This reverts commit 016a9a8.
@d0sboots
Copy link
Collaborator Author

d0sboots commented Nov 23, 2022

Thanks to @TheMas3212, I discovered danielyxie/bitburner#3592 which was the original motivation for the change. I don't have a mac to test with, but danielyxie/bitburner#3605 claims that "Windows/Mac/Linux with US-intl/US/en_GB/NL keyboard layouts (these were all affected in the same way)"

I have tested specifically with Windows US-intl. The behavior I see is that currently (on the live version of the game), if I use right-Alt+S, I get ß(eszett). This is because on intl (and presumably the others), right-Alt is mapped to AltGr. It does not show up as event.altKey (nor any other flags), so there is no way to trap for it - you will always get ß.

In contrast, left-Alt is normal alt. It works properly, both currently and with my revert (and presumably, when the original bug was filed).

TL;DR: I don't think the "fix" ever actually fixed anything.

@d0sboots
Copy link
Collaborator Author

I'm not sure what's up with the checks. These files are not generated, as far as I'm aware, and lint was clean when I ran it myself.

@Snarling
Copy link
Collaborator

Yeah I have no idea what's up with these check failures either, I think this is fine for merging.

@Snarling Snarling merged commit 70fadde into bitburner-official:dev Nov 26, 2022
@d0sboots d0sboots deleted the keyboard branch November 29, 2022 03:49
d0sboots added a commit to d0sboots/bitburner-src that referenced this pull request Dec 3, 2022
…places" (bitburner-official#224)"

The change did in fact reintroduce danielyxie/bitburner#3592
The issue was very Mac-specific, which is why my testing didn't/couldn't
catch it.

Plan is to bring back the use of event.key eventually (to fix the issue
with different keyboard layouts), but only with a way to configure
hotkeys so that users can work around their platform-specific issues.

This reverts commit 70fadde.
Snarling pushed a commit that referenced this pull request Dec 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants