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

Expose tools for building & normalizing hotkey & sequence strings #94

Merged
merged 11 commits into from
Oct 10, 2023

Conversation

iansan5653
Copy link
Member

@iansan5653 iansan5653 commented Oct 9, 2023

This PR expands the usefulness of the hotkey package by exposing lower-level utilities so that external tools can work with hotkey strings directly instead of only binding event handlers. This closes #93.

The changes include the following:

New types NormalizedHotkeyString and NormalizedSequenceString. These are simple branded string types that should help to encourage the safe use of the hotkey & sequence strings. Normalized hotkey strings can be parsed from plain strings with normalizeHotkey or they can be obtained from events with eventToHotkeyString. This way, a comparison of string === NormalizedHotkeyString will obviously be false and cause a lint error, ensuring that consumers don't forget to normalize before comparing. The same applies to `NormalizedSequenceString.

Combined the sequence.ts and normalize-sequence.ts files since they are closely related. This groups the code that creates sequence strings together.

Added a new normalizeSequence helper that is the sequence form of normalizeHotkey, enabling abstracting away the splitting and rejoining of sequences. Also added a SEQUENCE_DELIMETER constant.

Exposed all exports from sequence.ts and hotkey.ts publicly.

@iansan5653 iansan5653 requested a review from a team as a code owner October 9, 2023 19:04
@iansan5653 iansan5653 requested a review from dgreif October 9, 2023 19:04
@iansan5653 iansan5653 changed the base branch from main to theinterned/mod-character October 9, 2023 19:05
@theinterned
Copy link
Contributor

theinterned commented Oct 10, 2023

Given this is a breaking change, perhaps we should target this change a v3-candidate branch to keep the main branch deployable?

@theinterned theinterned mentioned this pull request Oct 10, 2023
@theinterned
Copy link
Contributor

I added this PR to #74

Copy link
Contributor

@theinterned theinterned left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like the branded type use here. Very cool

src/hotkey.ts Outdated Show resolved Hide resolved
Base automatically changed from theinterned/mod-character to main October 10, 2023 17:03
@iansan5653 iansan5653 merged commit 8071994 into main Oct 10, 2023
2 checks passed
@iansan5653 iansan5653 deleted the expose-utils branch October 10, 2023 18:53
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.

Expose hotkey and SequenceTracker components
2 participants