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

breaking: fix shortcut conflicts #165

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jorroll
Copy link

@jorroll jorroll commented Jun 13, 2022

This PR fixes #37 but includes a breaking change for the public parseKeybinding method. As I write this though, it occurs to me that I don't think that breaking change was necessary and I can probably revert that function back to the previous API if desired. Even doing that though, this PR changes the way Tinykeys processes the hotkey bindings and so is a breaking change.

Whereas before this would trigger both keybindings when g and then a was pressed:

createKeybindingsHandler({
  "g a": () => console.log("g a"),
  "a": () => console.log("g"),
});

With this change it will only trigger the "g a" keybinding. This change also supports bindings in the form

createKeybindingsHandler({
  "g a": () => console.log("g a"),
  "g": () => console.log("g"),
});

In this case, pressing "g" will not trigger the "g" keybinding until after the sequence timeout is reached. And pressing "g a" will never trigger the "g" hotkey. Basically, whereas before tinykeys only tracked the expected next keypress, with this change tinykeys tracks the entire sequence of keypresses and can use that to determine which keybindings are matches and which aren't. As you press keys, Tinykeys now knows if you are inside of a keybinding sequence and will disregard potential commands outside of the sequence.

I think the behavior that this PR adopts is what most users would expect from this library. The major downside is that before the library clocked in at 530 bytes and after this change it clocks in at 620 bytes. Though it's quite possible some additional refactoring could reduce the size more. Size wasn't my primary concern when putting it together.

Let me know what you think. Feel free to make further edits.

@jorroll
Copy link
Author

jorroll commented Jun 13, 2022

Oh another potentially breaking change: whereas before I think some keys were capitalization agnostic (e.g. $mod+enter and $mod+Enter were both valid), after this change keys must be entered with strict capitalization (i.e. always $mod+Enter and never $mod+enter). It was unclear if the previous behavior was intentional or a side effect of the implementation.

@aquark
Copy link

aquark commented May 21, 2023

This patch breaks shortcuts like "Shift+h", which rely on comparing key values ignoring case (since the key value is upper-case "H").

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.

Shortcut conflicts
2 participants