fix(hotkeys): support numpad plus shortcuts#133
Open
superposition wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughThis PR adds comprehensive support for the plus ( ChangesPlus key hotkey implementation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/hotkeys/src/validate.ts (2)
78-92: ⚡ Quick winConsider extracting
splitHotkeyPartsto a shared utility.This function is duplicated in
parse.ts(lines 78-92) with an identical implementation. Extracting it toconstants.tsor a dedicated utils module would improve maintainability and ensure consistent behavior.♻️ Refactor approach
Export
splitHotkeyPartsfromconstants.ts:// In constants.ts export function splitHotkeyParts(hotkey: string): Array<string> { const parts = hotkey.split('+').map((p) => p.trim()) if (hotkey.endsWith('+') && parts.length > 1) { parts.pop() if (parts.at(-1) === '') { parts.pop() } parts.push('+') } return parts }Then import and use in both
parse.tsandvalidate.ts:import { splitHotkeyParts, ... } from './constants'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/hotkeys/src/validate.ts` around lines 78 - 92, Extract the duplicate splitHotkeyParts implementation into a shared module (e.g., constants.ts or utils.ts) and export it, then replace the local implementations in both validate.ts and parse.ts with imports of that exported splitHotkeyParts; ensure the exported function signature and behavior (handling trailing '+' and trimming parts) remains identical and update any imports to reference the new shared symbol so both files use the single implementation.
64-64: ⚡ Quick winSimplify double normalization to a single
normalizeKeyNamecall.The current code applies both
normalizeKeyForValidationandnormalizeKeyName. SincenormalizeKeyNamealready handles letter uppercasing, function key normalization, and alias resolution (a superset ofnormalizeKeyForValidation's logic), the inner call is redundant.♻️ Proposed simplification
- const normalizedKey = normalizeKeyName(normalizeKeyForValidation(keyPart)) + const normalizedKey = normalizeKeyName(keyPart)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/hotkeys/src/validate.ts` at line 64, Replace the redundant double-normalization call by passing the raw keyPart directly to normalizeKeyName: remove the inner normalizeKeyForValidation invocation and call normalizeKeyName(keyPart) when assigning normalizedKey (the variable and its assignment using normalizeKeyName(normalizeKeyForValidation(keyPart))). This uses normalizeKeyName's superset behavior (letter uppercasing, function key normalization, alias resolution) and eliminates the unnecessary normalizeKeyForValidation call in validate.ts.packages/hotkeys/src/match.ts (1)
92-95: ⚡ Quick winUpdate inline comment to reflect punctuation code fallback.
The inline comment describes dead keys and single-char mismatches, but doesn't mention that the fallback also activates for punctuation codes (line 104's
hasPunctuationCodecondition). Consider updating it to:-// Fallback to event.code for dead keys or single-char mismatches where -// event.key is a non-letter special character. +// Fallback to event.code for dead keys, single-char mismatches where +// event.key is a non-letter special character, or punctuation keys. // Dead keys: Option+letter on macOS, international layouts produce event.key === 'Dead' -// Single-char mismatches: Cmd+Option+T gives '†' instead of 'T', Shift+4 gives '$' +// Single-char mismatches: Cmd+Option+T gives '†' instead of 'T', Shift+4 gives '$' +// Punctuation keys: NumpadAdd may produce event.key='Unidentified' in some browsersThe main API documentation (lines 15-27) is already accurate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/hotkeys/src/match.ts` around lines 92 - 95, Update the inline comment above the fallback block to mention that the fallback to event.code also covers punctuation codes in addition to dead keys and single-character mismatches: reference the hasPunctuationCode check and the event.key === 'Dead' case so the comment explains that punctuation key mappings (checked by hasPunctuationCode) will trigger using event.code as a fallback when event.key is a non-letter or produces an unexpected punctuation character; keep the existing examples for dead keys and single-char mismatches and add a short note naming hasPunctuationCode and event.code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/hotkeys/src/match.ts`:
- Around line 92-95: Update the inline comment above the fallback block to
mention that the fallback to event.code also covers punctuation codes in
addition to dead keys and single-character mismatches: reference the
hasPunctuationCode check and the event.key === 'Dead' case so the comment
explains that punctuation key mappings (checked by hasPunctuationCode) will
trigger using event.code as a fallback when event.key is a non-letter or
produces an unexpected punctuation character; keep the existing examples for
dead keys and single-char mismatches and add a short note naming
hasPunctuationCode and event.code.
In `@packages/hotkeys/src/validate.ts`:
- Around line 78-92: Extract the duplicate splitHotkeyParts implementation into
a shared module (e.g., constants.ts or utils.ts) and export it, then replace the
local implementations in both validate.ts and parse.ts with imports of that
exported splitHotkeyParts; ensure the exported function signature and behavior
(handling trailing '+' and trimming parts) remains identical and update any
imports to reference the new shared symbol so both files use the single
implementation.
- Line 64: Replace the redundant double-normalization call by passing the raw
keyPart directly to normalizeKeyName: remove the inner normalizeKeyForValidation
invocation and call normalizeKeyName(keyPart) when assigning normalizedKey (the
variable and its assignment using
normalizeKeyName(normalizeKeyForValidation(keyPart))). This uses
normalizeKeyName's superset behavior (letter uppercasing, function key
normalization, alias resolution) and eliminates the unnecessary
normalizeKeyForValidation call in validate.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6f771263-8cfd-49cd-b30c-080bf6659ed7
📒 Files selected for processing (10)
packages/hotkeys/src/constants.tspackages/hotkeys/src/format.tspackages/hotkeys/src/hotkey.tspackages/hotkeys/src/match.tspackages/hotkeys/src/parse.tspackages/hotkeys/src/validate.tspackages/hotkeys/tests/hotkey-manager.test.tspackages/hotkeys/tests/match.test.tspackages/hotkeys/tests/parse.test.tspackages/hotkeys/tests/validate.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #127
Summary
Mod++and normalize them to the unambiguousMod+PlusformKeyboardEvent.code === "NumpadAdd"to the plus key in event-code fallback matching{ key: "+", mod: true }registration and numpad-plus matchingTest plan
corepack pnpm --filter @tanstack/hotkeys exec vitest run tests/match.test.ts tests/parse.test.ts tests/validate.test.ts tests/hotkey-manager.test.tscorepack pnpm --filter @tanstack/hotkeys test:typescorepack pnpm --filter @tanstack/hotkeys test:eslintcorepack pnpm --filter @tanstack/hotkeys exec vitest runcorepack pnpm --filter @tanstack/hotkeys buildgit diff --checkSummary by CodeRabbit