-
-
Notifications
You must be signed in to change notification settings - Fork 43
feat: callback version of conflictBehavior #54
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| '@tanstack/hotkeys-devtools': patch | ||
| '@tanstack/hotkeys': patch | ||
| --- | ||
|
|
||
| feat: callback variant of conflictBehavior |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -119,7 +119,18 @@ function getConflictLabel( | |
| if (behavior === 'allow') return 'allowed' | ||
| if (behavior === 'error') return 'error' | ||
| if (behavior === 'replace') return 'replaced' | ||
| return 'warning' | ||
| if (behavior === 'warn') return 'warning' | ||
| return 'callback handled conflict with' | ||
| } | ||
|
|
||
| function serializeConflictBehavior( | ||
| behavior: ConflictBehavior | ||
| ): string { | ||
| if (typeof behavior === 'string') { | ||
| return behavior | ||
| } | ||
|
|
||
| return '[Function function]' | ||
| } | ||
|
|
||
| function HotkeyDetails(props: { | ||
|
|
@@ -285,7 +296,7 @@ function HotkeyDetails(props: { | |
| </div> | ||
| <div class={styles().optionRow}> | ||
| <span class={styles().optionLabel}>conflictBehavior</span> | ||
| <span class={styles().optionValue}>{conflictBehavior()}</span> | ||
| <span class={styles().optionValue}>{serializeConflictBehavior(conflictBehavior())}</span> | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| </div> | ||
| <div class={styles().optionRow}> | ||
| <span class={styles().optionLabel}>hasFired</span> | ||
|
|
@@ -498,7 +509,7 @@ function SequenceDetails(props: { | |
| </div> | ||
| <div class={styles().optionRow}> | ||
| <span class={styles().optionLabel}>conflictBehavior</span> | ||
| <span class={styles().optionValue}>{conflictBehavior()}</span> | ||
| <span class={styles().optionValue}>{serializeConflictBehavior(conflictBehavior())}</span> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,10 @@ | ||
| import type { ParsedHotkey } from './hotkey' | ||
|
|
||
| type CustomConflictHandler = ( | ||
| keyDisplay: string, | ||
| unregisterAnotherConflictingId: () => void | ||
| ) => void | ||
|
|
||
| /** | ||
| * Behavior when registering a hotkey/sequence that conflicts with an existing registration. | ||
| * | ||
|
|
@@ -8,7 +13,7 @@ import type { ParsedHotkey } from './hotkey' | |
| * - `'replace'` - Unregister the existing registration and register the new one | ||
| * - `'allow'` - Allow multiple registrations without warning | ||
| */ | ||
| export type ConflictBehavior = 'warn' | 'error' | 'replace' | 'allow' | ||
| export type ConflictBehavior = 'warn' | 'error' | 'replace' | 'allow' | CustomConflictHandler | ||
|
|
||
| /** | ||
| * Default options for hotkey/sequence registration. | ||
|
|
@@ -164,6 +169,11 @@ export function handleConflict( | |
| ) | ||
| } | ||
|
|
||
| if (typeof conflictBehavior === 'function') { | ||
| conflictBehavior(keyDisplay, () => unregister(conflictingId)) | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it makes sense to expose |
||
| return | ||
| } | ||
|
|
||
| // At this point, conflictBehavior must be 'replace' | ||
| unregister(conflictingId) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -286,5 +286,20 @@ describe('manager.utils', () => { | |
|
|
||
| expect(unregister).toHaveBeenCalledWith('id-1') | ||
| }) | ||
|
|
||
| it('should call custom callback if passed as conflictBehaviour', () => { | ||
| const unregister = vi.fn() | ||
| const handleConflictCallback = vi.fn() | ||
|
|
||
| handleConflict('id-1', 'Mod+S', handleConflictCallback, unregister) | ||
|
|
||
| expect(unregister).not.toHaveBeenCalledWith() | ||
| expect(handleConflictCallback).toHaveBeenCalledWith( | ||
| 'Mod+S', | ||
| expect.any(Function), | ||
| ) | ||
| handleConflictCallback.mock.calls[0]?.[1]() | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mimics custom async logic inside the callback, e.g. call |
||
| expect(unregister).toHaveBeenCalledWith('id-1') | ||
| }) | ||
| }) | ||
| }) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a new entry here but autofix removed it.
How could I keep it?