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

[Bug] Keycode picker search filter ignores selected OS keyboard layout #1372

Open
precondition opened this issue Nov 8, 2024 · 0 comments
Open
Labels

Comments

@precondition
Copy link
Contributor

precondition commented Nov 8, 2024

Describe the Bug

src/store/keycodes.js

    setSearchFilter(newVal) {
      this.searchFilter = newVal;
      if (this.searchFilter !== '') {
        this.searchCounters = {
          ANSI: countMatches(this.searchFilter, ansi),
          'ISO/JIS': countMatches(this.searchFilter, iso_jis),
          Quantum: countMatches(this.searchFilter, quantum),
          KeyboardSettings: countMatches(this.searchFilter, settings),
          AppMediaMouse: countMatches(this.searchFilter, media)
        };
      }
    }

The collections passed to countMatches (ansi, iso_jis, etc.) are read-only variables imported from ./modules/keycodes/###. Consequently, these keycode collections are not localized at all causing incorrect match counts to be reported.

For example, if I search for "ß" with the German OS layout, the ß key will be highlighted but the count in both ANSI and ISO tabs will be "(0)". Same story if I search for "Y" with the German OS layout; DE_Y and KC_Y (=DE_Z) are both highlighted but the ANSI tab will only report a single match because it successfully finds a "Y" in the uppercase version of KC_Y's name but KC_Z aka DE_Y has "Z" as its name in the ansi variable so the countMatches does not count it.

Current:
image
image

Expected:
image
image

(All those matches for ß in other tabs comes from the fact that "ß".toUpperCase() == "SS" but whether or not matching the TO(layer) key with a "ß" query is the expected behavior because its tooltip says "Turn on layer when preSSed" is up for debate...)

The solution would be to map the toLocalKeycode on each collection before passing it to the countMatches.
As a quick proof-of-concept, I came up with the following diff to produce the "Expected" screenshots linked above:

diff --git a/src/store/keycodes.js b/src/store/keycodes.js
index bde6fe1c..3ab62b9d 100644
--- a/src/store/keycodes.js
+++ b/src/store/keycodes.js
@@ -159,13 +159,36 @@ export const useKeycodesStore = defineStore('keycodes', {
     },
     setSearchFilter(newVal) {
       this.searchFilter = newVal;
+
+      const { keycodeLUT } = keymapExtras[getOSKeyboardLayout()];
+
+      const localized_ansi = ansi.map((keycodeObject) =>
+        toLocaleKeycode(keycodeLUT, keycodeObject)
+      );
+
+      const localized_iso_jis = iso_jis.map((keycodeObject) =>
+        toLocaleKeycode(keycodeLUT, keycodeObject)
+      );
+
+      const localized_quantum = quantum.map((keycodeObject) =>
+        toLocaleKeycode(keycodeLUT, keycodeObject)
+      );
+
+      const localized_settings = settings.map((keycodeObject) =>
+        toLocaleKeycode(keycodeLUT, keycodeObject)
+      );
+
+      const localized_media = media.map((keycodeObject) =>
+        toLocaleKeycode(keycodeLUT, keycodeObject)
+      );
+
       if (this.searchFilter !== '') {
         this.searchCounters = {
-          ANSI: countMatches(this.searchFilter, ansi),
-          'ISO/JIS': countMatches(this.searchFilter, iso_jis),
-          Quantum: countMatches(this.searchFilter, quantum),
-          KeyboardSettings: countMatches(this.searchFilter, settings),
-          AppMediaMouse: countMatches(this.searchFilter, media)
+          ANSI: countMatches(this.searchFilter, localized_ansi),
+          'ISO/JIS': countMatches(this.searchFilter, localized_iso_jis),
+          Quantum: countMatches(this.searchFilter, localized_quantum),
+          KeyboardSettings: countMatches(this.searchFilter, localized_settings),
+          AppMediaMouse: countMatches(this.searchFilter, localized_media)
         };
       }
     }

However, doing the mapping inside the setSearchFilter callback is probably a bad idea as that would imply doing a relatively long operation on every key event in the keycode picker search bar.

Additional Context?

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant