github / github/hotkey

API alterations to allow injection of hotkey parsing and event processing

未关闭
#69 4 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

还没有人认领这个 Issue。

主要语言
JavaScript
星标
3.3k
派生
99
平均合并
14 小时 22 分钟
30 天内合并 PR
6

描述

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?

贡献指南

打开贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

调研方向

先阅读 hotkey.ts 和 radix-trie.ts,然后将当前的 install/uninstall API 与提议的 HotkeyManager、ProcessHotkey 和 ProcessEvent 接口进行比较。确定如何在不更改现有 default exports 的情况下集成自定义序列化和事件处理;当 API 设计和兼容性行为达成一致并完成实现时,即视为完成。

由索引模型根据 Issue 内容生成。

评估

技术栈
typescript
领域
frontend
Issue 类型
功能
难度
5/5
预计耗时
一周以上
活跃度
停滞
描述清晰度
基本清楚
新手友好度
35/100

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。