github / hotkey Goto Github PK

eventToHotKeyString outputs ? event as Shift+?

Since the updates in the latest release which include a logic change to address a different bug, eventToHotkeyString is includingShift unexpectedly.

? ==> Shift+?
! ==> Shift+!

Can we revert the logic to what it was before?

Related PRs:

• #40
• #41
• #43

I'm throwing errors every time I keydown in an element with a dotted id. For example, <div id="account.firm">.

I think this should be resolvable by quoting and escaping the target id in the querySelector, index.ts line 23, from

if (!target.ownerDocument.querySelector(`[data-hotkey-scope=${target.id}]`)) return

to

if (!target.ownerDocument.querySelector(`[data-hotkey-scope="${CSS.escape(target.id)}"]`)) return

It would be nice to be able to set platform specific hotkeys. This is important because often there's a keyboard shortcut that might collide with an os specific combo.

<button type="submit" data-hotkey-mac="Control+M" data-hotkey-win="Alt+M">
  Save
</button>

👋 Thanks for the nice library.

I know that GitHub supports keyboard navigation in views such as the Issues and Pull Request views. Was that implemented by building on top of this library, or is it using something else entirely?

(I'm hoping to implement something similar for my own app).

We will reuse these components, but not the entire library, when we build the command platform internally. So we just need to export these:

• https://github.com/github/hotkey/blob/main/src/hotkey.ts
• https://github.com/github/hotkey/blob/main/src/sequence.ts

It appears to be missing a GH token.

https://github.com/github/hotkey/runs/4519905291?check_suite_focus=true

As a result, the 1.6.1 release is published on npm but does not appear on the GitHub packages list — 1.6.0 is listed as the latest version.

This is a tracking issue for hotkey version 3 which will introduce a few breaking changes:

Some of this was split from #65

For details, please click to seeing to 👇
Mottie/GitHub-userscripts#123

Hi,

I have tried to set data-hotkey with Control+Shift+p also Control+Shift+s but that doesn't work.
anyone can help?

the issue occurs on both in mac & windows.
the browser: Chrome Version 83.0.4103.116 (Official Build) (64-bit)

thanks in advance.

Hey fine folks! I've been using hotkey for a couple of weeks, it's very cool, thanks a lot for the work you put into this already 🙏🏽

I was wondering if there were any possibilities to define a custom event to fire when the hotkey is pressed by the user?

My example: I setup a hotkey onto a select element, and did hope for the select element to be opened on hotkey press. Unfortunately, because of how the fireDeterminedAction function is defined, it only focuses on the select, which I need to then trigger to open by navigating with arrow keys, for instance.

Would it be something useful to add to the library, or sth to prevent developers from doing for specific reasons I don't grasp yet?

Either way, thanks for taking the time to read this!

Problem

Currently data-hotkey-scope only supports matching the (event.target i.e. the scope) as an id, this has a few limitations:

1. I may want to specify multiple elements as the scope for my hotkey.
2. I may need to specify an element rendered by a third part library as my scope and do not have access to set an id on that element. (This is my current actual issue).
3. Likely there are other uses-cases where an id doesn't work as a selector...

Suggestion

Add the ability to specify a class-name instead of an id for data-hotkey-scope.

Some possible API thoughts:

Support a 'mini selector syntax'

The API could be updated to take a # prefix (for id) or . prefix (for class name). We could continue to treat an un-prefixed value as an id:

<button data-hotkey-scope=".my-scope-class-name" />
<button data-hotkey-scope="#my-scope-id" />
<button data-hotkey-scope="my-scope-id" /> <!-- legacy support -->

Add new data-attributes:

We could add data-hotkey-scope-class and data-hotkey-scope-id attributes. Again we could continue to treat the existing data-hotkey-scope as an id scope.

<button data-hotkey-scope-class="my-scope-class-name" />
<button data-hotkey-scope-id="my-scope-id" />
<button data-hotkey-scope="my-scope-id" /> <!-- legacy support -->

Considerations

Why not allow full css-selector syntax?

I thought about the possibility of extending data-hotkey-scope to support full css selector syntax like data-hotkey-scope="div#parent > .target-element" ... but after talking this over with @keithamus we decided the complexity and likely performance cost of such an API would not be worth it given that id and class name already support many use-cases.

It would be great if we could also display chords in the mapper tool, similarly to VSCode.

I dug into this a bit and it's more complicated than I expected. If you just have a timer and track keydown events, you get chords like Meta Meta+b because the keydown for Meta is different from the keydown for b. It looks like this is solved in the main code with the 'radix trie' logic, but this seems pretty closely tied to the hotkeys that are configured on the element, and we can't configure all possible hotkeys onto the mapper tool.

This is a tracking issue for hotkey version 2 which will introduce a few breaking changes:

• #64 fixes a bug with duplicate modifier keys.
• #57
• #71

Some planned work was moved to v3 #74

I think a useful migration strategy would be to allow serialization/comparison to be overridden as part of the API.

When we talk about what this library does that is "novel" (as in, something you cannot reasonably achieve in a few LOC locally) the library does three things:

• It manages many shortcuts by implementing a Radix Trie.
• It manages event listening & dispatch avoiding footguns: for example delegated events, ensuring hotkeys aren't fired on form fields, etc.
• It provides the above with a simple install/uninstall API to simplify adding/removing event listeners and state from the Radix Trie.
• It comes up with some lose specification of how to expand/compare hotkey strings to decide when to fire a hotkey combo.

The last one is what causes us a lot of trouble and causes some trashing in this library. radix-trie.ts hasn't been touched in 9 months, prior to that 2 years ago. I'd say radix-trie is "feature complete". Meanwhile hotkey.ts has a regular cadence of alterations every few months as we reach edge cases and scale our use of this library.

The chief problems with the serialization format are:

• It is a psuedo specification. There's no formal set of possible values or a well defined grammar or spec. It is an ad-hoc grammar using RegExps. While this works fine for the most part, it is a source of bugs and confusion, as well as differences of opinion.
• It doesn't properly encode all of the state about what we as developers intend shortcuts to be. We've discussed this quite a lot synchronously; the concept of "logical" (WSAD) vs "Semiotic" (?) shortcuts.

Effectively the serialization of these shortcuts is, what you might call, unsolved. So I say let's make that apparent by allowing it to be overridden in the API.

The current API is as follows:

export install(element: HTMLElement, hotkey?: string): void {}
export uninstall(element: HTMLElement): void {}

I propose we expand this to the following:

export type ProcessHotkey = (hotkey: string): string[][]
export type ProcessEvent = (event: KeyboardEvent): string

export class HotkeyManager {
  constructor(
    processHotkey: ProcessHotkey = expandHotkeyToEdges, 
    processEvent: ProcessEvent = eventToHotkeyString
  )

  install(element: HTMLelement, hotkey?: string): void {}
  
  uninstall(element: HTMLElement): void {}
}

const defaultManager = new HotkeyManager()
export const install = defaultManager.install
export const uninstall = defaultManager.uninstall

By making a class, we can define custom processing for shortcut keys which allows users to define their own shortcut models, but also allows us to make more breaking changes behind experimental APIs, and even feature flag them. By still exposing the install/uninstall functions per the existing API, we ensure backwards compatibility which minimizes breaking changes, and allows us to "make the hard change easy then make the easy change". A 2.0 change could effectively swap the default hotkey functions out for the newer API, as a one line change.

Thoughts @github/ui-frameworks?

I’m using a keyboard with a German layout. The keys Z and Y are reversed in comparison to the US layout.

When I set a KeyCombo with Key(string: "z") it get’s recognised as "y" and vice versa.

I suspect this is because keyCodes are hardware-related, aren’t they? What could be a possible fix?

On a Mac, the alt (option) key acts as a modifier. For example, when alt and k are pressed, the serialized event string produced on a Mac is Alt+˚, while on Windows and Linux the serialized string is Alt+k.

• This relates to #66 as it means that Alt can not reliably be used with the "Mod" modifier.
• This relates to #54 as the fix for that will likely be applicable here as well.

While it may prove impossible to smooth over all OS / keyboard layout differences in event.key, providing access to mapping hotkeys to an event.code may help in some situations (for example WASD navigation)

VS Code allows this via wrapping the code in []: for example [KeyA] would match on event.code === "KeyA".

This allows for some flexibility to map shortcuts to physical locations on the keyboard (eg WASD navigation etc.)

We are observing a TypeError from this line:

const elideShift = event.code.startsWith('Key') && event.shiftKey && event.key.toUpperCase() === event.key;

See: Sentry

Though this was first seen 9 months ago, looks like there has been a recent spike.

Because of the way eventToHotkeyString works, it will return a "double modifier" for a key-combination where a modifier key (Control, Alt, Meta, or Shift) is also the event.key.

This is visible by looking at the hotkey mapper tool:

To Reproduce:

1. Visit https://github.github.io/hotkey/examples/hotkey_mapper.html
2. Input a ctrl.
3. Observe the hotkey string is Control+Control.
4. Without lifting your finger from the crtl key, input a shift key.
5. Observe the hotkey string is Control+Shift+Shift.
6. Lift your fingers.
7. Input a shift.
8. Observe the hotkey string is Shift+Shift.
9. Input a ctrl.
10. Observe the hotkey string is Control+Shift+Control.
11. Note that the hotkey string in 10 is different than the hotkey string in 5: this is due to the order that these keys were pressed.

Expected

When the event.key is a modifier, it should only appear once in the hotkey string. The order that modifiers appear in the string should be consistent, regarldless of the order that keys are pressed (see 11 in repro steps above).

There are two possibilities:

1. Filter out the modifier when it is the same as the event.key
   2. ctrl then shift would now be: [Control+]Shift+Control: Shift+Control
   3. shift then ctrl would be Control+[Shift+]Shift: Control+Shift
2. When the vent.key is a modifier, don't map it to the stirng
   2. ctrl then shift would now be: Control+Shift+[Control]: Control+Shift+
   3. shift then ctrl would be Control+Shift+[Shift]: Control+Shift+

So the solution is to do 2. But we'll have to avoid the trailing +

When typing in an input (textarea, input tag, editable, …) which is itself placed inside of a custom html element, the hotkey is still fired. That's because the isFormField handler checks only the target element, which is this case is the custom element. It should instead use the explicitOriginalTarget property, since that contains the actual input element.

Here's a minimal reproduction: https://jsfiddle.net/zm7etdh3/

How doees github listen for all command palette hotkeys meta+k on all inputs across github with this plugin?

In the readme there is

<a href="/search" data-hotkey="s,/">Search</a>

the same code is used in the GitHub toolbar that says "Type / to search".

The problem is that pressing / (shift + 7 on a finnish keyboard) does not trigger the search. It does nothing. Pressing "s" does trigger it.

From a chat with @jscholes off-site:

This is a difficult sort of mechanism to make accessible for some users, because reliance on keyboard input (or tools that emulate it like speech recognition software) will conflict with the tool's requirement to take all keyboard input and swallow it. Indeed, as a screen reader user, it's difficult to get rid of it once loaded! :)

Here's what I'd suggest:

1. Give the hotkey input area (id="hotkey-code") a tabindex of "0", so users can explicitly place focus on it.
2. Give the same area the following attributes: role="application", aria-roledescription="Input Capture", aria-label="Hotkey", aria-live="assertive", and aria-atomic="true".
3. Bind your keydown handler to that input div#hotkey-code, instead of the entire document.
4. Make it visually obvious in some way that mouse users should click (or keyboard users should focus) the input area in order to trigger the behaviour, as it will no longer apply to the full page. This should include a focus outline.
5. Disallow Tab and Shift+Tab from being used as tool input, so users can get out of the input capture area. This is both an explicit accessibility recommendation for this page, as well as an inclusive design tip: users shouldn't be rebinding Tab and Shift+Tab so the tool should discourage it.
6. Remove tabindex="0" and role="button" from the clipboard-copy component. The inner button already has all of the semantics that are needed.
7. Move the copy button after the input area, both visually and in the mark-up, so users don't have to navigate backwards to reach it.

In some situations, we may want to map shortcuts to physical keys instead of characters that change based on the keyboard layout.

This may be useful for websites that have an international audience, with the keyboard layout being hard to guess.

There are many cases where we want to be able to support either the Meta or Control modifier for a shortcut; as opposed to cases where we support both Meta and Control.

One way to handle this would be to add a special Mod character that would map to Meta on Mac / iOS, and Control on Window or Linux.

<button data-hotkey="Mod+s">Save</button>

would map to Meta+s on Mac / Control+s otherwise.

Hi~ I am not a developer and do not understand internal code,

• 💥 Current bugged behavior:

　　　　keypress ꘌ keydown / keyup

• ❥ Expected/fixed behavior:

　　　　keypress ≠ keydown / keyup

Add a e.g. 30ms delay to keydown to judge keypress event ?

see more at: Mottie/GitHub-userscripts#136

Thanks for the great lib!

Currently in the keyDownHandler I see this:

if (event.target instanceof Node && isFormField(event.target)) return

In my use-case I would like to attach an "Escape" hotkey to a cancel button, and a "Command+Enter" hotkey to a submit button, and have these hotkeys be active whilst the focus is in an input/textarea. The above line prevents this.

Understandable that single character and Shift+* hotkeys shouldn't be active, but what about everything else? What's the recommended approach here?

It seems that the eventToHotkeyString function returns " " for a spacebar keystroke, as described in the key values docs. But the expandHotkeyToEdges function also uses the space character to split hotkey combinations.

So how might one define a single hotkey or hotkey combination containing a space?

We might consider a slight deviation from the key values docs in this instance to add the Space shorthand as an alias for " " when defining a hotkey via the data-hotkey attribute or install function. Thank you! 💛