Intenational key binding fixes #234

MrStashley · 2024-01-17T00:27:12Z

This PR creates a new function in the global model for checking key presses, and it creates a special key press syntax

An example of this syntax: if you want to check for ctrl + shift + e, you would write "ctrl:shift:e"

We also now use event.key instead of event.code for checking key presses
There are a lot of considerations here
Event.key changes depending on modifiers - if user presses shift + e, event.key = "E" and if user pre sses option + e, on Mac event.key = "´"

I have handled the shift case, basically any of the above work - "Shift:e" == "Shift:E" == "E"
The option case however is still around, not sure how to go about handling that one
For now option keys aren't really going to work
I guess the way is to go by event code rather than key, but then people have to write their option commands with different syntax, ie "Alt:KeyR" rather than "Alt:r"
We could also do some string conversions on the event code, like we can map "KeyR" to "r", and a lot of the non character keys like "Escape" or "Enter" or "ArrowDown" are the same both key and code. There will be a few special cases though, like "[" = "LeftBracket"

or we can just roll with the punches and make users do "Alt:®" for Alt + r (this is how it is working right now), or maybe there's a tool out there for finding the un optioned version of an option key

src/app/workspace/cmdinput/aichat.tsx

MrStashley · 2024-01-17T00:32:48Z

src/app/workspace/cmdinput/textareainput.tsx

@@ -232,59 +232,59 @@ class TextAreaInput extends React.Component<{ screen: Screen; onHeightChange: ()
                }
                inputModel.closeAIAssistantChat();
                return;
-            }
-            if (e.code == "KeyE" && e.getModifierState("Meta")) {
+            } 


should I call this meta instead of command?

src/model/model.ts

sawka · 2024-01-17T02:09:49Z

I think your CopyFile code got merged into this PR somehow?

MrStashley · 2024-01-23T20:05:57Z

src/app/workspace/cmdinput/textareainput.tsx

            e.preventDefault();
            inputModel.resetInput();
            return;
        }
        if (
-            e.code == "KeyR" &&
-            (e.getModifierState("Meta") || e.getModifierState("Control")) &&


some or key bindings here
I feel like we should make these either Ctrl or Cmd, rather than both

I think it should just be control?
Looking at this though I understand why or is used here since it's a really important command

Adds job status indicators that will show any updates to running commands while you are focused away from a tab. These will show up as status icons in the tab view. These indicators will reset for a given tab when you focus back to it. I've updated the inner formatting of the tab to use flexboxes, allowing the title to display more text when there are no icons to display. Also includes some miscellaneous for-loop pattern improvements in model.ts and removing of unused variables, etc. --------- Co-authored-by: sawka <[email protected]>

MrStashley · 2024-01-23T23:40:05Z

src/app/history/history.tsx

@@ -95,29 +96,33 @@ function formatSessionName(snames: Record<string, string>, sessionId: string): s
 }

 @mobxReact.observer
-class HistoryCheckbox extends React.Component<{ checked: boolean, partialCheck?: boolean, onClick?: () => void }, {}> {
+class HistoryCheckbox extends React.Component<{ checked: boolean; partialCheck?: boolean; onClick?: () => void }, {}> {


prettier related changes, not rebase artifacts I'm pretty sure

MrStashley · 2024-01-23T23:40:56Z

src/app/workspace/cmdinput/aichat.tsx

                e.preventDefault();
                e.stopPropagation();
                inputModel.closeAIAssistantChat();
            }
-            if (e.code == "KeyL" && e.getModifierState("Control")) {
+


maybe remove space here
Idk what is your thought on spacing?
Apple was super strict on spacing inside of functions for some reason, so I'm hardwired to get rid of them

MrStashley · 2024-01-23T23:43:46Z

src/util/keyutil.ts

+
+function parseKeyDescription(keyDescription: string): KeyPressDecl {
+    let rtn = { key: "", mods: {} } as KeyPressDecl;
+    rtn.mods.Cmd = false;


is it ugly to init this way? wasn't sure how to properly init a record type
seems I have to set each of the values otherwise they end up as none

MrStashley · 2024-01-23T23:45:22Z

src/util/keyutil.ts

+    }
+    let eventKey = event.key;
+    let descKey = keyPress.key;
+    if (eventKey.length == 1 && /[A-Z]/.test(eventKey.charAt(0))) {


So this accepts shift:e, shift:E or E all as the same thing
it also accepts "Space" or " " to mean space
Thoughts?

Also I'm wondering if I should move this logic to the adapt keyboard event functions so that WaveKeyboardEvent has a standardized key value in these cases

we should add unicode checks in here. instead of checking for [A-Z] you can use \p{Lu} (unicode uppercase characters). https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Regular_expressions/Unicode_character_class_escape . I think you also need to set the v flag on the regexp.

see also https://unicode.org/reports/tr18/#General_Category_Property

sawka · 2024-01-25T18:25:37Z

For event.key, I think space bar actually comes back as " " and not "Space" . https://developer.mozilla.org/en-US/docs/Web/API/UI_Events/Keyboard_event_key_values#whitespace_keys

sawka · 2024-01-25T18:42:48Z

src/util/keyutil.ts

+
+type ModKeyStrs = "Cmd" | "Option" | "Shift" | "Ctrl";
+
+interface WaveKeyboardEvent {


maybe cut down the number of lines of comments here. since they are single line, just use //

sawka · 2024-01-25T18:46:01Z

I think for WaveKeyboardEvent, it shouldn't have cmd. You should just set alt and meta from the keyboard events so WaveKeyboardEvent always matches the native event.

put the special cmd logic in the check function. also better since that logic will then only live in once place instead of multiple.

MrStashley commented Jan 17, 2024

View reviewed changes

src/app/workspace/cmdinput/aichat.tsx Outdated Show resolved Hide resolved

MrStashley commented Jan 17, 2024

View reviewed changes

src/model/model.ts Outdated Show resolved Hide resolved

MrStashley commented Jan 17, 2024

View reviewed changes

src/model/model.ts Outdated Show resolved Hide resolved

MrStashley marked this pull request as ready for review January 17, 2024 01:10

MrStashley changed the title ~~Mr stashley add check key press function~~ Intenational key binding fixes Jan 17, 2024

MrStashley commented Jan 23, 2024

View reviewed changes

MrStashley and others added 15 commits January 23, 2024 15:02

first pass of copy file

8996f68

first pass fixing up function

b0307da

first pass fixing key press

22db0c3

fixed up key press parsing

618a7a9

reverted cmdrunner.go

e34c846

fixed cmdrunner.go again :p

a71afd7

fixed cmdrunner again lol

d2158ec

first pass fixing key press

76dea98

added key util check file

6262a50

addressed rebase artifacts

eb61b9c

fixed more rebase artifacts

c8dd7a4

fixed keybindings

31aa56f

removed log

85f132d

fixed a ton of rebase artifacts

202ee84

MrStashley force-pushed the MrStashley-add-checkKeyPress-function branch from 635f5dc to 202ee84 Compare January 23, 2024 23:37

MrStashley commented Jan 23, 2024

View reviewed changes

MrStashley added 2 commits January 23, 2024 22:21

fixed a ton of rebase artifacts

c2b53ac

added cmd maps to altKey if not on macos

cb6fbac

sawka reviewed Jan 25, 2024

View reviewed changes

fixed platform check code and fixed some small bugs

b7e5548

sawka merged commit 0648d48 into main Jan 26, 2024
4 checks passed

MrStashley deleted the MrStashley-add-checkKeyPress-function branch February 15, 2024 19:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Intenational key binding fixes #234

Intenational key binding fixes #234

MrStashley commented Jan 17, 2024 •

edited

Loading

MrStashley Jan 17, 2024

sawka commented Jan 17, 2024

MrStashley Jan 23, 2024

MrStashley Jan 23, 2024

MrStashley Jan 23, 2024

MrStashley Jan 23, 2024

MrStashley Jan 23, 2024

MrStashley Jan 23, 2024 •

edited

Loading

sawka Jan 25, 2024

sawka commented Jan 25, 2024

sawka Jan 25, 2024

sawka commented Jan 25, 2024


		type ModKeyStrs = "Cmd" \| "Option" \| "Shift" \| "Ctrl";

		interface WaveKeyboardEvent {

Intenational key binding fixes #234

Intenational key binding fixes #234

Conversation

MrStashley commented Jan 17, 2024 • edited Loading

Choose a reason for hiding this comment

sawka commented Jan 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MrStashley Jan 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sawka commented Jan 25, 2024

Choose a reason for hiding this comment

sawka commented Jan 25, 2024

MrStashley commented Jan 17, 2024 •

edited

Loading

MrStashley Jan 23, 2024 •

edited

Loading