Open
Conversation
Contributor
🚀 Changeset Version Preview6 package(s) bumped directly, 0 bumped as dependents. 🟨 Minor bumps
|
@tanstack/angular-hotkeys
@tanstack/hotkeys
@tanstack/hotkeys-devtools
@tanstack/preact-hotkeys
@tanstack/preact-hotkeys-devtools
@tanstack/react-hotkeys
@tanstack/react-hotkeys-devtools
@tanstack/solid-hotkeys
@tanstack/solid-hotkeys-devtools
@tanstack/svelte-hotkeys
@tanstack/vue-hotkeys
@tanstack/vue-hotkeys-devtools
commit: |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (91)
📝 WalkthroughWalkthroughAdds plural hotkey-sequence registration APIs across frameworks: Changes
Sequence Diagram(s)sequenceDiagram
participant Component as Framework Component / Hook
participant FrameworkLifecycle as Framework Lifecycle (effect/watch/createEffect)
participant SequenceManager as SequenceManager (singleton)
participant Target as Target (document | element)
Component->>FrameworkLifecycle: provide definitions + commonOptions
FrameworkLifecycle->>FrameworkLifecycle: resolve getters/refs\nmerge options (provider < common < per-definition)
FrameworkLifecycle->>SequenceManager: diff registrations\nregister/unregister handles (with target)
SequenceManager->>Target: attach listeners to target
Target->>SequenceManager: key events
SequenceManager->>SequenceManager: detect sequence match\ninvoke callback
SequenceManager->>Component: callback runs (updates state)
FrameworkLifecycle->>SequenceManager: on update -> update callbacks/options or re-register
Component->>FrameworkLifecycle: unmount/destroy
FrameworkLifecycle->>SequenceManager: unregister remaining handles
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 Tip You can customize the tone of the review comments and chat replies.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (3)
docs/framework/preact/guides/sequences.md (1)
38-38: Optional: add acommonOptionsexample snippet.Since this line explains second-argument precedence, adding a tiny example with
useHotkeySequences(definitions, commonOptions)would make the behavior immediately actionable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/framework/preact/guides/sequences.md` at line 38, Add a short example demonstrating the second-argument precedence for useHotkeySequences so readers can see how HotkeysProvider defaults, the second-argument commonOptions, and each definition’s options merge (e.g., show calling useHotkeySequences(definitions, commonOptions) with one shared option overridden by a per-definition options entry). Refer to useHotkeySequences, HotkeysProvider, commonOptions, definitions, and each definition’s options in the example so it clearly shows the precedence order in practice.examples/angular/injectHotkeySequences/src/app/app.component.html (1)
72-74: Announce triggered sequence updates to assistive tech.At Line 73, this status changes dynamically; adding
aria-live="polite"improves screen-reader feedback.♿ Suggested tweak
- <div class="info-box success"><strong>Triggered:</strong> {{ seq }}</div> + <div class="info-box success" aria-live="polite"> + <strong>Triggered:</strong> {{ seq }} + </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/angular/injectHotkeySequences/src/app/app.component.html` around lines 72 - 74, The dynamic "Triggered" status rendered via the lastSequence() template binding (as seq) should announce updates to assistive tech; add aria-live="polite" to the div with class "info-box success" that displays {{ seq }} so screen readers are notified of changes without interrupting. Target the template block using lastSequence() / seq and update that div to include the aria-live attribute.packages/angular-hotkeys/src/injectHotkeySequences.ts (1)
106-110: Inconsistentenabledhandling compared to other framework implementations.The Angular implementation skips registration when
enabled: falseat lines 106-110, and always passesenabled: truewhen registering (line 158). Other framework implementations (React, Vue, Solid, Preact, Svelte) pass the resolvedenabledvalue through tosetOptions()or the registration call, allowing the underlying manager to handle enable/disable state.This inconsistency means:
- Angular: Toggling
enabledcauses unregister/re-register cycles- Other frameworks: Toggling
enabledupdates the existing handle viasetOptions()Consider aligning with other frameworks by passing the
enabledvalue through to the handle:♻️ Suggested alignment with other frameworks
- const { enabled = true, ...sequenceOpts } = mergedOptions + const { enabled = true, target, ...restOptions } = mergedOptions - if (!enabled || resolvedSequence.length === 0) { + if (resolvedSequence.length === 0) { continue } + const resolvedTarget = + target ?? (typeof document !== 'undefined' ? document : null) + + if (!resolvedTarget) { + continue + } + + const sequenceOpts = { ...restOptions, enabled }And when registering:
const handle = manager.register(p.resolvedSequence, p.def.callback, { ...p.sequenceOpts, - enabled: true, target: p.resolvedTarget, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/angular-hotkeys/src/injectHotkeySequences.ts` around lines 106 - 110, The Angular implementation currently destructures enabled from mergedOptions and skips registration when enabled is false (using const { enabled = true, ...sequenceOpts } = mergedOptions and continue), and later always registers with enabled: true; instead, preserve and pass the resolved enabled value through like other frameworks: do not early-continue on enabled === false (still register the sequence handle), and when calling the registration or setOptions helper (the code path that currently uses sequenceOpts/registration call), include enabled (the actual resolved enabled) rather than hardcoding true so the manager can toggle state via setOptions/update instead of unregistering/re-registering; adjust uses of mergedOptions, sequenceOpts, and the register/setOptions call accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/framework/angular/guides/sequences.md`:
- Around line 28-48: The example is missing the import for the Angular decorator
used: add an import for Component from '@angular/core' at the top of the snippet
so the `@Component` decorator on AppComponent resolves; keep the existing import
of injectHotkeySequences from '@tanstack/angular-hotkeys' and the AppComponent
class/constructor unchanged.
In `@examples/angular/injectHotkeySequences/src/app/app.component.ts`:
- Line 12: The history property uses the bracket-array form which violates the
lint rule; change its type to the generic form by replacing any occurrences of
string[] with Array<string> (e.g., update history = signal<string[]>([]) to
history = signal<Array<string>>([]) and adjust any other annotations or uses of
history in the AppComponent class accordingly).
In `@examples/angular/injectHotkeySequences/src/app/app.config.ts`:
- Around line 1-6: The file currently imports provideHotkeys but doesn't use it
and pulls in ApplicationConfig as a value import; change the ApplicationConfig
import to a type-only import (import type { ApplicationConfig }) and add
provideHotkeys(...) into the providers array of the exported appConfig so the
example demonstrates hotkey setup; reference the symbols ApplicationConfig,
provideHotkeys, appConfig and provideZoneChangeDetection when making these
edits.
In `@examples/angular/injectHotkeySequences/src/styles.css`:
- Around line 22-27: Remove the duplicate margin declaration in the CSS rule for
selector "header p": delete the earlier "margin: 0" and keep the later "margin:
0 auto" so only one margin declaration remains in the "header p" rule to satisfy
Stylelint and avoid redundancy.
In `@examples/preact/useHotkeySequences/src/index.css`:
- Around line 26-30: The CSS block for the selector "header p" contains a
duplicate margin declaration; remove the redundant "margin: 0;" so the rule only
keeps the intended layout declaration ("margin: 0 auto") in the header p rule to
avoid dead code and ensure the paragraph is centered horizontally.
In `@examples/react/useHotkeySequences/src/index.css`:
- Around line 26-31: The CSS selector "header p" contains a duplicate margin
declaration: remove the redundant "margin: 0" so only "margin: 0 auto" remains
(or consolidate into a single margin rule) within the header p block to satisfy
Stylelint and prevent the overwritten property.
In `@examples/solid/createHotkeySequences/src/index.css`:
- Around line 22-27: The selector "header p" contains a duplicated margin
declaration; remove the redundant "margin: 0" so only the intended "margin: 0
auto" remains (or merge them into a single "margin: 0 auto") alongside the
existing "max-width: 500px" and other properties to satisfy Stylelint and avoid
the overridden rule.
In `@examples/solid/createHotkeySequences/src/index.tsx`:
- Around line 3-8: Reorder the named imports to satisfy the ESLint sort-imports
rule: in the first import list swap createSignal and Show so Show comes before
createSignal, and in the second import list reorder the three symbols so
HotkeysProvider appears before createHotkey and createHotkeySequences
(preserving the relative order between the two create* names).
In `@examples/svelte/create-hotkey-sequences/README.md`:
- Around line 16-19: Update the recreate command in the README so the generated
project name matches the example directory: replace the erroneous
"create-hotkey" token in the shell command with "create-hotkey-sequences" (the
command string in the README's recreate example).
In `@examples/svelte/create-hotkey-sequences/src/index.css`:
- Around line 26-31: The CSS rule for the selector "header p" contains a
duplicate margin declaration; remove the redundant "margin: 0;" and keep the
intended "margin: 0 auto;" in the header p rule (locate the header p block in
index.css) so only a single margin declaration remains and the stylelint error
is resolved.
In `@examples/vue/useHotkeySequences/src/index.css`:
- Around line 26-31: The CSS rule for selector "header p" contains a duplicate
margin declaration; remove the redundant "margin: 0;" and keep a single
consolidated margin declaration (e.g., "margin: 0 auto;") in the "header p" rule
so there is no overridden/unused property.
In `@packages/solid-hotkeys/tests/createHotkeySequences.test.tsx`:
- Around line 2-10: Reorder and split the imports to satisfy ESLint rules:
alphabetize named members inside each import, move type-only imports to
top-level "import type" statements, and place the "solid-js" import before the
relative module import. Specifically, convert "type
CreateHotkeySequenceDefinition" and "type Component" to top-level import type
lines, alphabetize members in the vitest import (afterEach, beforeEach,
describe, expect, it, vi) and in the `@tanstack/hotkeys` import (SequenceManager),
ensure "createSignal" and "Component" from "solid-js" are imported before
"../src/createHotkeySequences", and keep "render" from
"@solidjs/testing-library" ordered appropriately.
---
Nitpick comments:
In `@docs/framework/preact/guides/sequences.md`:
- Line 38: Add a short example demonstrating the second-argument precedence for
useHotkeySequences so readers can see how HotkeysProvider defaults, the
second-argument commonOptions, and each definition’s options merge (e.g., show
calling useHotkeySequences(definitions, commonOptions) with one shared option
overridden by a per-definition options entry). Refer to useHotkeySequences,
HotkeysProvider, commonOptions, definitions, and each definition’s options in
the example so it clearly shows the precedence order in practice.
In `@examples/angular/injectHotkeySequences/src/app/app.component.html`:
- Around line 72-74: The dynamic "Triggered" status rendered via the
lastSequence() template binding (as seq) should announce updates to assistive
tech; add aria-live="polite" to the div with class "info-box success" that
displays {{ seq }} so screen readers are notified of changes without
interrupting. Target the template block using lastSequence() / seq and update
that div to include the aria-live attribute.
In `@packages/angular-hotkeys/src/injectHotkeySequences.ts`:
- Around line 106-110: The Angular implementation currently destructures enabled
from mergedOptions and skips registration when enabled is false (using const {
enabled = true, ...sequenceOpts } = mergedOptions and continue), and later
always registers with enabled: true; instead, preserve and pass the resolved
enabled value through like other frameworks: do not early-continue on enabled
=== false (still register the sequence handle), and when calling the
registration or setOptions helper (the code path that currently uses
sequenceOpts/registration call), include enabled (the actual resolved enabled)
rather than hardcoding true so the manager can toggle state via
setOptions/update instead of unregistering/re-registering; adjust uses of
mergedOptions, sequenceOpts, and the register/setOptions call accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 216e776a-35f5-4f4b-97a0-20356289fa95
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (95)
.changeset/plural-sequences.mddocs/config.jsondocs/framework/angular/guides/sequences.mddocs/framework/angular/reference/functions/injectHotkeySequences.mddocs/framework/angular/reference/index.mddocs/framework/angular/reference/interfaces/InjectHotkeySequenceDefinition.mddocs/framework/preact/guides/sequences.mddocs/framework/preact/reference/functions/useHotkeySequences.mddocs/framework/preact/reference/index.mddocs/framework/preact/reference/interfaces/UseHotkeySequenceDefinition.mddocs/framework/react/guides/sequences.mddocs/framework/react/reference/functions/useHotkeySequences.mddocs/framework/react/reference/index.mddocs/framework/react/reference/interfaces/UseHotkeySequenceDefinition.mddocs/framework/solid/guides/sequences.mddocs/framework/solid/reference/functions/createHotkeySequences.mddocs/framework/solid/reference/index.mddocs/framework/solid/reference/interfaces/CreateHotkeySequenceDefinition.mddocs/framework/svelte/guides/sequences.mddocs/framework/svelte/reference/functions/createHotkeySequences.mddocs/framework/svelte/reference/functions/createHotkeySequencesAttachment.mddocs/framework/svelte/reference/index.mddocs/framework/svelte/reference/interfaces/CreateHotkeySequenceDefinition.mddocs/framework/vue/guides/sequences.mddocs/framework/vue/quick-start.mddocs/framework/vue/reference/functions/useHotkeySequences.mddocs/framework/vue/reference/index.mddocs/framework/vue/reference/interfaces/UseHotkeySequenceDefinition.mdexamples/angular/injectHotkeySequences/angular.jsonexamples/angular/injectHotkeySequences/package.jsonexamples/angular/injectHotkeySequences/src/app/app.component.cssexamples/angular/injectHotkeySequences/src/app/app.component.htmlexamples/angular/injectHotkeySequences/src/app/app.component.tsexamples/angular/injectHotkeySequences/src/app/app.config.tsexamples/angular/injectHotkeySequences/src/index.htmlexamples/angular/injectHotkeySequences/src/main.tsexamples/angular/injectHotkeySequences/src/styles.cssexamples/angular/injectHotkeySequences/tsconfig.jsonexamples/preact/useHotkeySequences/eslint.config.jsexamples/preact/useHotkeySequences/index.htmlexamples/preact/useHotkeySequences/package.jsonexamples/preact/useHotkeySequences/src/index.cssexamples/preact/useHotkeySequences/src/index.tsxexamples/preact/useHotkeySequences/tsconfig.jsonexamples/preact/useHotkeySequences/vite.config.tsexamples/react/useHotkeySequences/eslint.config.jsexamples/react/useHotkeySequences/index.htmlexamples/react/useHotkeySequences/package.jsonexamples/react/useHotkeySequences/src/index.cssexamples/react/useHotkeySequences/src/index.tsxexamples/react/useHotkeySequences/tsconfig.jsonexamples/react/useHotkeySequences/vite.config.tsexamples/solid/createHotkeySequences/index.htmlexamples/solid/createHotkeySequences/package.jsonexamples/solid/createHotkeySequences/src/index.cssexamples/solid/createHotkeySequences/src/index.tsxexamples/solid/createHotkeySequences/tsconfig.jsonexamples/solid/createHotkeySequences/vite.config.tsexamples/svelte/create-hotkey-sequences/.gitignoreexamples/svelte/create-hotkey-sequences/.npmrcexamples/svelte/create-hotkey-sequences/README.mdexamples/svelte/create-hotkey-sequences/index.htmlexamples/svelte/create-hotkey-sequences/package.jsonexamples/svelte/create-hotkey-sequences/src/App.svelteexamples/svelte/create-hotkey-sequences/src/Root.svelteexamples/svelte/create-hotkey-sequences/src/index.cssexamples/svelte/create-hotkey-sequences/src/main.tsexamples/svelte/create-hotkey-sequences/static/robots.txtexamples/svelte/create-hotkey-sequences/svelte.config.jsexamples/svelte/create-hotkey-sequences/tsconfig.jsonexamples/svelte/create-hotkey-sequences/vite.config.tsexamples/vue/useHotkeySequences/eslint.config.jsexamples/vue/useHotkeySequences/index.htmlexamples/vue/useHotkeySequences/package.jsonexamples/vue/useHotkeySequences/src/App.vueexamples/vue/useHotkeySequences/src/index.cssexamples/vue/useHotkeySequences/src/index.tsexamples/vue/useHotkeySequences/src/vue.d.tsexamples/vue/useHotkeySequences/tsconfig.jsonexamples/vue/useHotkeySequences/vite.config.tspackages/angular-hotkeys/src/index.tspackages/angular-hotkeys/src/injectHotkeySequences.tspackages/preact-hotkeys/src/index.tspackages/preact-hotkeys/src/useHotkeySequences.tspackages/preact-hotkeys/tests/useHotkeySequences.test.tsxpackages/react-hotkeys/src/index.tspackages/react-hotkeys/src/useHotkeySequences.tspackages/react-hotkeys/tests/useHotkeySequences.test.tsxpackages/solid-hotkeys/src/createHotkeySequences.tspackages/solid-hotkeys/src/index.tspackages/solid-hotkeys/tests/createHotkeySequences.test.tsxpackages/svelte-hotkeys/src/createHotkeySequences.svelte.tspackages/svelte-hotkeys/src/index.tspackages/vue-hotkeys/src/index.tspackages/vue-hotkeys/src/useHotkeySequences.ts
examples/angular/injectHotkeySequences/src/app/app.component.ts
Outdated
Show resolved
Hide resolved
KevinVandy
force-pushed
the
hotkeysequences
branch
from
1f650ae to
4767bbb
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/angular/injectHotkeySequences/src/app/app.component.html`:
- Line 112: Escape the '>' in arrow functions inside the HTML example: replace
each arrow token "=>" in the callback expressions with "=>" so the snippets
like "callback: () => scrollToTop()" become "callback: () => scrollToTop()";
apply the same change for the other occurrences referenced (lines containing the
callback arrow at the sequence entries on the same block).
- Line 112: The code example uses uppercase sequence ['G','G'] but the
docs/table describe lowercase "g g" (Vim semantics: gg -> top); update the
sequence in the example (the object containing "sequence: ['G','G'], callback:
() => scrollToTop()") to use lowercase ['g','g'] so the example matches the
table and expected behavior.
In `@examples/angular/injectHotkeySequences/src/app/app.config.ts`:
- Around line 1-3: Reorder the imports so the regular imports come before the
type-only import: place the existing `import { provideZoneChangeDetection } from
'@angular/core'` and `import { provideHotkeys } from
'@tanstack/angular-hotkeys'` first, and then move the type import `import type {
ApplicationConfig } from '@angular/core'` after them to satisfy the project's
`import/order` ESLint rule; update any references to `ApplicationConfig`,
`provideZoneChangeDetection`, or `provideHotkeys` accordingly.
In `@packages/solid-hotkeys/tests/createHotkeySequences.test.tsx`:
- Around line 5-8: The import order violates ESLint import/order: move all value
imports (createSignal from 'solid-js' and createHotkeySequences from
'../src/createHotkeySequences') before type-only imports, and group type imports
together (Component from 'solid-js' and CreateHotkeySequenceDefinition from
'../src/createHotkeySequences'); update the import statements so value imports
appear first then type imports to satisfy the rule.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f44e66a4-e2be-4fcf-8ae7-ad91880b1242
📒 Files selected for processing (27)
.changeset/plural-sequences.mddocs/framework/angular/guides/sequences.mddocs/framework/angular/reference/functions/injectHotkeySequences.mddocs/framework/angular/reference/interfaces/InjectHotkeySequenceDefinition.mddocs/framework/preact/reference/functions/useHotkeySequences.mddocs/framework/preact/reference/interfaces/UseHotkeySequenceDefinition.mddocs/framework/react/reference/functions/useHotkeySequences.mddocs/framework/react/reference/interfaces/UseHotkeySequenceDefinition.mddocs/framework/solid/reference/functions/createHotkeySequences.mddocs/framework/solid/reference/interfaces/CreateHotkeySequenceDefinition.mddocs/framework/svelte/reference/functions/createHotkeySequences.mddocs/framework/svelte/reference/functions/createHotkeySequencesAttachment.mddocs/framework/svelte/reference/interfaces/CreateHotkeySequenceDefinition.mddocs/framework/vue/reference/functions/useHotkeySequences.mddocs/framework/vue/reference/interfaces/UseHotkeySequenceDefinition.mdexamples/angular/injectHotkeySequences/src/app/app.component.htmlexamples/angular/injectHotkeySequences/src/app/app.component.tsexamples/angular/injectHotkeySequences/src/app/app.config.tsexamples/angular/injectHotkeySequences/src/styles.cssexamples/preact/useHotkeySequences/src/index.cssexamples/react/useHotkeySequences/src/index.cssexamples/solid/createHotkeySequences/src/index.cssexamples/solid/createHotkeySequences/src/index.tsxexamples/svelte/create-hotkey-sequences/README.mdexamples/svelte/create-hotkey-sequences/src/index.cssexamples/vue/useHotkeySequences/src/index.csspackages/solid-hotkeys/tests/createHotkeySequences.test.tsx
✅ Files skipped from review due to trivial changes (23)
- docs/framework/angular/guides/sequences.md
- docs/framework/preact/reference/interfaces/UseHotkeySequenceDefinition.md
- docs/framework/svelte/reference/functions/createHotkeySequences.md
- docs/framework/react/reference/interfaces/UseHotkeySequenceDefinition.md
- docs/framework/svelte/reference/functions/createHotkeySequencesAttachment.md
- docs/framework/angular/reference/interfaces/InjectHotkeySequenceDefinition.md
- .changeset/plural-sequences.md
- examples/svelte/create-hotkey-sequences/README.md
- docs/framework/react/reference/functions/useHotkeySequences.md
- docs/framework/angular/reference/functions/injectHotkeySequences.md
- docs/framework/vue/reference/interfaces/UseHotkeySequenceDefinition.md
- docs/framework/svelte/reference/interfaces/CreateHotkeySequenceDefinition.md
- examples/react/useHotkeySequences/src/index.css
- docs/framework/solid/reference/functions/createHotkeySequences.md
- examples/solid/createHotkeySequences/src/index.tsx
- examples/angular/injectHotkeySequences/src/styles.css
- examples/preact/useHotkeySequences/src/index.css
- docs/framework/preact/reference/functions/useHotkeySequences.md
- docs/framework/vue/reference/functions/useHotkeySequences.md
- docs/framework/solid/reference/interfaces/CreateHotkeySequenceDefinition.md
- examples/solid/createHotkeySequences/src/index.css
- examples/svelte/create-hotkey-sequences/src/index.css
- examples/vue/useHotkeySequences/src/index.css
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/angular/injectHotkeySequences/src/app/app.component.ts
|
|
||
| // In constructor or injection context: | ||
| injectHotkeySequences([ | ||
| {{ '{' }} sequence: ['G', 'G'], callback: () => scrollToTop() {{ '}' }}, |
There was a problem hiding this comment.
Escape arrow function > characters in the code example.
The > in arrow functions should be escaped as > for strict HTML compliance within the <pre> block. You've already escaped @ as @ on line 108, but the > characters on lines 112, 115, 118, and 121 also need escaping.
🔧 Proposed fix
injectHotkeySequences([
- { sequence: ['G', 'G'], callback: () => scrollToTop() },
+ { sequence: ['G', 'G'], callback: () => scrollToTop() },
{
sequence: ['ArrowUp', 'ArrowUp', 'ArrowDown', 'ArrowDown'],
- callback: () => activateCheatMode(),
+ callback: () => activateCheatMode(),
options: { timeout: 1500 },
},
- { sequence: ['C', 'I', 'W'], callback: () => changeInnerWord() },
- { sequence: ['Shift+R', 'Shift+T'], callback: () => doSomething() },
+ { sequence: ['C', 'I', 'W'], callback: () => changeInnerWord() },
+ { sequence: ['Shift+R', 'Shift+T'], callback: () => doSomething() },
])Also applies to: 115-115, 118-118, 121-121
🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 112-112: Special characters must be escaped : [ > ].
(spec-char-escape)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/angular/injectHotkeySequences/src/app/app.component.html` at line
112, Escape the '>' in arrow functions inside the HTML example: replace each
arrow token "=>" in the callback expressions with "=>" so the snippets like
"callback: () => scrollToTop()" become "callback: () => scrollToTop()"; apply
the same change for the other occurrences referenced (lines containing the
callback arrow at the sequence entries on the same block).
Fix inconsistency between table and code example.
The table at lines 22-24 shows g g (lowercase) for "Go to top", but the code example uses ['G', 'G'] (uppercase). In Vim semantics, gg goes to top and G goes to bottom. The code example should use ['g', 'g'] to match the documented behavior.
📝 Proposed fix
injectHotkeySequences([
- { sequence: ['G', 'G'], callback: () => scrollToTop() },
+ { sequence: ['g', 'g'], callback: () => scrollToTop() },
{🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 112-112: Special characters must be escaped : [ > ].
(spec-char-escape)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/angular/injectHotkeySequences/src/app/app.component.html` at line
112, The code example uses uppercase sequence ['G','G'] but the docs/table
describe lowercase "g g" (Vim semantics: gg -> top); update the sequence in the
example (the object containing "sequence: ['G','G'], callback: () =>
scrollToTop()") to use lowercase ['g','g'] so the example matches the table and
expected behavior.
Comment on lines
+1
to
+3
| import type { ApplicationConfig } from '@angular/core' | ||
| import { provideZoneChangeDetection } from '@angular/core' | ||
| import { provideHotkeys } from '@tanstack/angular-hotkeys' |
There was a problem hiding this comment.
Fix import order to satisfy ESLint.
The type import from @angular/core should be placed after regular imports per the project's import/order rule.
📝 Proposed fix
-import type { ApplicationConfig } from '@angular/core'
import { provideZoneChangeDetection } from '@angular/core'
import { provideHotkeys } from '@tanstack/angular-hotkeys'
+import type { ApplicationConfig } from '@angular/core'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import type { ApplicationConfig } from '@angular/core' | |
| import { provideZoneChangeDetection } from '@angular/core' | |
| import { provideHotkeys } from '@tanstack/angular-hotkeys' | |
| import { provideZoneChangeDetection } from '@angular/core' | |
| import { provideHotkeys } from '@tanstack/angular-hotkeys' | |
| import type { ApplicationConfig } from '@angular/core' |
🧰 Tools
🪛 ESLint
[error] 1-1: @angular/core type import should occur after import of @tanstack/angular-hotkeys
(import/order)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/angular/injectHotkeySequences/src/app/app.config.ts` around lines 1
- 3, Reorder the imports so the regular imports come before the type-only
import: place the existing `import { provideZoneChangeDetection } from
'@angular/core'` and `import { provideHotkeys } from
'@tanstack/angular-hotkeys'` first, and then move the type import `import type {
ApplicationConfig } from '@angular/core'` after them to satisfy the project's
`import/order` ESLint rule; update any references to `ApplicationConfig`,
`provideZoneChangeDetection`, or `provideHotkeys` accordingly.
Comment on lines
+5
to
+8
| import { createSignal } from 'solid-js' | ||
| import type { Component } from 'solid-js' | ||
| import { createHotkeySequences } from '../src/createHotkeySequences' | ||
| import type { CreateHotkeySequenceDefinition } from '../src/createHotkeySequences' |
There was a problem hiding this comment.
Import order still violates ESLint import/order rule.
The value import from ../src/createHotkeySequences (line 7) should occur before the type import from solid-js (line 6). Group all value imports together, then all type imports.
Proposed fix
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { render } from '@solidjs/testing-library'
import { SequenceManager } from '@tanstack/hotkeys'
import { createSignal } from 'solid-js'
-import type { Component } from 'solid-js'
import { createHotkeySequences } from '../src/createHotkeySequences'
+import type { Component } from 'solid-js'
import type { CreateHotkeySequenceDefinition } from '../src/createHotkeySequences'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { createSignal } from 'solid-js' | |
| import type { Component } from 'solid-js' | |
| import { createHotkeySequences } from '../src/createHotkeySequences' | |
| import type { CreateHotkeySequenceDefinition } from '../src/createHotkeySequences' | |
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' | |
| import { render } from '@solidjs/testing-library' | |
| import { SequenceManager } from '@tanstack/hotkeys' | |
| import { createSignal } from 'solid-js' | |
| import { createHotkeySequences } from '../src/createHotkeySequences' | |
| import type { Component } from 'solid-js' | |
| import type { CreateHotkeySequenceDefinition } from '../src/createHotkeySequences' |
🧰 Tools
🪛 ESLint
[error] 7-7: ../src/createHotkeySequences import should occur before type import of solid-js
(import/order)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/solid-hotkeys/tests/createHotkeySequences.test.tsx` around lines 5 -
8, The import order violates ESLint import/order: move all value imports
(createSignal from 'solid-js' and createHotkeySequences from
'../src/createHotkeySequences') before type-only imports, and group type imports
together (Component from 'solid-js' and CreateHotkeySequenceDefinition from
'../src/createHotkeySequences'); update the import statements so value imports
appear first then type imports to satisfy the rule.
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.
🎯 Changes
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Behavior Change
Examples
Documentation
Tests
Chores