From d67050c44eab78f2c026c527aa06320c4fda82fa Mon Sep 17 00:00:00 2001 From: Ali Waseem Date: Thu, 23 Apr 2026 09:22:16 -0600 Subject: [PATCH] test(studio): add unit tests for shortcut primitives (#45162) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Closes [FE-3056](https://linear.app/supabase/issue/FE-3056/add-unit-tests-for-new-shortcut-primitives). Backfills unit coverage for the shortcut primitives added in the recent keyboard-shortcut overhaul — these shipped with no tests. - **`state/shortcuts/formatShortcut.test.ts`** (16 tests) — `hotkeyToKeys`: `Mod → Meta` mapping (case-sensitive, substring-safe), single/multi-modifier combos, named keys, punctuation, edge cases. - **`state/shortcuts/useShortcut.test.tsx`** (26 tests) — mocks `useHotkeySequence`, `useRegisterCommands`, and `useIsShortcutEnabled` to unit-test: sequence/callback wiring (single-step + multi-step G-chords), full option-resolution priority (caller → registry → fallback) for `enabled` / `timeout` / `ignoreInputs`, global-preference AND gating, command-menu registration across all four enable permutations, stable action identity, stale-closure protection via `useLatest`, and badge rendering with/without the "then" separator. - **`components/ui/Shortcut.test.tsx`** (12 tests) — mocks `useShortcut` and `ShortcutTooltip` to verify prop forwarding (including rerender behavior), tooltip positioning props, unset-prop hygiene, and the `asChild` pass-through contract (clicks on the child do not fire `onTrigger`). 54 tests total, all passing. ## Test plan - [x] `pnpm --filter studio test state/shortcuts/formatShortcut.test.ts state/shortcuts/useShortcut.test.tsx components/ui/Shortcut.test.tsx` - [x] `tsc --noEmit` clean across the three test files ## Summary by CodeRabbit * **Tests** * Added comprehensive test coverage for the Shortcut component, verifying rendering, hook integration, and tooltip behavior. * Added test suite for hotkey key formatting, covering plain keys, named keys, modifiers, and edge cases. * Added test suite for the useShortcut hook, validating hotkey sequences, enablement logic, command menu integration, and callback handling. --- apps/studio/components/ui/Shortcut.test.tsx | 197 +++++++++++++ .../state/shortcuts/formatShortcut.test.ts | 99 +++++++ .../state/shortcuts/useShortcut.test.tsx | 265 ++++++++++++++++++ 3 files changed, 561 insertions(+) create mode 100644 apps/studio/components/ui/Shortcut.test.tsx create mode 100644 apps/studio/state/shortcuts/formatShortcut.test.ts create mode 100644 apps/studio/state/shortcuts/useShortcut.test.tsx diff --git a/apps/studio/components/ui/Shortcut.test.tsx b/apps/studio/components/ui/Shortcut.test.tsx new file mode 100644 index 00000000000..b0b90d101ec --- /dev/null +++ b/apps/studio/components/ui/Shortcut.test.tsx @@ -0,0 +1,197 @@ +import { fireEvent, render, screen } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { Shortcut } from './Shortcut' +import { SHORTCUT_IDS } from '@/state/shortcuts/registry' + +const { mockUseShortcut, mockShortcutTooltip } = vi.hoisted(() => ({ + mockUseShortcut: vi.fn(), + mockShortcutTooltip: vi.fn((props: any) => ( +
{props.children}
+ )), +})) + +vi.mock('@/state/shortcuts/useShortcut', () => ({ + useShortcut: mockUseShortcut, +})) + +vi.mock('./ShortcutTooltip', () => ({ + ShortcutTooltip: mockShortcutTooltip, +})) + +describe('Shortcut', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + describe('rendering', () => { + it('renders the wrapped child', () => { + render( + {}}> + + + ) + expect(screen.getByRole('button', { name: 'Open' })).toBeInTheDocument() + }) + + it('wraps the child in ShortcutTooltip', () => { + render( + {}}> + + + ) + const tooltip = screen.getByTestId('shortcut-tooltip') + expect(tooltip).toContainElement(screen.getByRole('button', { name: 'Open' })) + }) + }) + + describe('useShortcut wiring', () => { + it('forwards id and onTrigger to useShortcut', () => { + const handler = vi.fn() + render( + + + + ) + expect(mockUseShortcut).toHaveBeenCalledWith(SHORTCUT_IDS.ACTION_BAR_SAVE, handler, undefined) + }) + + it('forwards options to useShortcut', () => { + const handler = vi.fn() + const options = { enabled: true, registerInCommandMenu: true } + render( + + + + ) + expect(mockUseShortcut).toHaveBeenCalledWith(SHORTCUT_IDS.ACTION_BAR_SAVE, handler, options) + }) + + it('passes updated options to useShortcut on rerender', () => { + const handler = vi.fn() + const { rerender } = render( + + + + ) + expect(mockUseShortcut).toHaveBeenLastCalledWith(SHORTCUT_IDS.ACTION_BAR_SAVE, handler, { + enabled: false, + }) + + rerender( + + + + ) + expect(mockUseShortcut).toHaveBeenLastCalledWith(SHORTCUT_IDS.ACTION_BAR_SAVE, handler, { + enabled: true, + }) + }) + + it('passes updated id to useShortcut on rerender', () => { + const handler = vi.fn() + const { rerender } = render( + + + + ) + expect(mockUseShortcut).toHaveBeenLastCalledWith(SHORTCUT_IDS.NAV_HOME, handler, undefined) + + rerender( + + + + ) + expect(mockUseShortcut).toHaveBeenLastCalledWith( + SHORTCUT_IDS.NAV_TABLE_EDITOR, + handler, + undefined + ) + }) + }) + + describe('ShortcutTooltip wiring', () => { + it('forwards shortcutId to ShortcutTooltip', () => { + render( + {}}> + + + ) + expect(mockShortcutTooltip).toHaveBeenCalled() + expect(mockShortcutTooltip.mock.calls.at(-1)![0].shortcutId).toBe(SHORTCUT_IDS.NAV_HOME) + }) + + it('forwards tooltip positioning props', () => { + render( + {}} + side="right" + align="start" + sideOffset={8} + delayDuration={100} + > + + + ) + const props = mockShortcutTooltip.mock.calls.at(-1)![0] + expect(props.side).toBe('right') + expect(props.align).toBe('start') + expect(props.sideOffset).toBe(8) + expect(props.delayDuration).toBe(100) + }) + + it('forwards label override to ShortcutTooltip', () => { + render( + {}} label="Go home"> + + + ) + expect(mockShortcutTooltip.mock.calls.at(-1)![0].label).toBe('Go home') + }) + + it('omits undefined positioning props rather than fabricating them', () => { + render( + {}}> + + + ) + const props = mockShortcutTooltip.mock.calls.at(-1)![0] + expect(props.side).toBeUndefined() + expect(props.align).toBeUndefined() + expect(props.sideOffset).toBeUndefined() + expect(props.delayDuration).toBeUndefined() + expect(props.label).toBeUndefined() + }) + }) + + describe('child interactivity', () => { + it('does not intercept clicks on the wrapped child', () => { + const onClick = vi.fn() + const onTrigger = vi.fn() + render( + + + + ) + + fireEvent.click(screen.getByRole('button', { name: 'Open' })) + expect(onClick).toHaveBeenCalledTimes(1) + expect(onTrigger).not.toHaveBeenCalled() + }) + + it('does not invoke onTrigger during render — only the hotkey should trigger it', () => { + const onTrigger = vi.fn() + render( + + + + ) + expect(onTrigger).not.toHaveBeenCalled() + }) + }) +}) diff --git a/apps/studio/state/shortcuts/formatShortcut.test.ts b/apps/studio/state/shortcuts/formatShortcut.test.ts new file mode 100644 index 00000000000..3dc1047db67 --- /dev/null +++ b/apps/studio/state/shortcuts/formatShortcut.test.ts @@ -0,0 +1,99 @@ +import { describe, expect, it } from 'vitest' + +import { hotkeyToKeys } from './formatShortcut' + +describe('hotkeyToKeys', () => { + describe('single-step hotkeys', () => { + it('returns a single letter key unchanged', () => { + expect(hotkeyToKeys('K')).toEqual(['K']) + }) + + it('preserves named keys', () => { + expect(hotkeyToKeys('Enter')).toEqual(['Enter']) + expect(hotkeyToKeys('Escape')).toEqual(['Escape']) + expect(hotkeyToKeys('Tab')).toEqual(['Tab']) + expect(hotkeyToKeys('ArrowUp')).toEqual(['ArrowUp']) + expect(hotkeyToKeys('ArrowDown')).toEqual(['ArrowDown']) + expect(hotkeyToKeys('ArrowLeft')).toEqual(['ArrowLeft']) + expect(hotkeyToKeys('ArrowRight')).toEqual(['ArrowRight']) + }) + + it('preserves punctuation keys', () => { + expect(hotkeyToKeys(',')).toEqual([',']) + expect(hotkeyToKeys('.')).toEqual(['.']) + expect(hotkeyToKeys('/')).toEqual(['/']) + }) + }) + + describe('Mod mapping', () => { + it('converts standalone Mod to Meta', () => { + expect(hotkeyToKeys('Mod')).toEqual(['Meta']) + }) + + it('converts Mod to Meta when combined with a single key', () => { + expect(hotkeyToKeys('Mod+K')).toEqual(['Meta', 'K']) + }) + + it('is case sensitive — only exact "Mod" is converted', () => { + expect(hotkeyToKeys('mod+K')).toEqual(['mod', 'K']) + expect(hotkeyToKeys('MOD+K')).toEqual(['MOD', 'K']) + }) + + it('does not rewrite keys that merely contain "Mod" as a substring', () => { + expect(hotkeyToKeys('Model')).toEqual(['Model']) + expect(hotkeyToKeys('Mod+Model')).toEqual(['Meta', 'Model']) + }) + }) + + describe('modifier combos', () => { + it('handles Mod+Shift combos', () => { + expect(hotkeyToKeys('Mod+Shift+M')).toEqual(['Meta', 'Shift', 'M']) + expect(hotkeyToKeys('Mod+Shift+J')).toEqual(['Meta', 'Shift', 'J']) + expect(hotkeyToKeys('Mod+Shift+C')).toEqual(['Meta', 'Shift', 'C']) + }) + + it('preserves modifier order', () => { + expect(hotkeyToKeys('Shift+Mod+K')).toEqual(['Shift', 'Meta', 'K']) + expect(hotkeyToKeys('Alt+Mod+K')).toEqual(['Alt', 'Meta', 'K']) + }) + + it('leaves Ctrl, Alt, Shift, Meta literals untouched', () => { + expect(hotkeyToKeys('Ctrl+K')).toEqual(['Ctrl', 'K']) + expect(hotkeyToKeys('Alt+K')).toEqual(['Alt', 'K']) + expect(hotkeyToKeys('Shift+K')).toEqual(['Shift', 'K']) + expect(hotkeyToKeys('Meta+K')).toEqual(['Meta', 'K']) + }) + + it('handles three-modifier combos', () => { + expect(hotkeyToKeys('Mod+Alt+Shift+K')).toEqual(['Meta', 'Alt', 'Shift', 'K']) + }) + }) + + describe('named-key combos from the registry', () => { + it('handles Mod+ArrowUp/Down/Left/Right', () => { + expect(hotkeyToKeys('Mod+ArrowUp')).toEqual(['Meta', 'ArrowUp']) + expect(hotkeyToKeys('Mod+ArrowDown')).toEqual(['Meta', 'ArrowDown']) + expect(hotkeyToKeys('Mod+ArrowLeft')).toEqual(['Meta', 'ArrowLeft']) + expect(hotkeyToKeys('Mod+ArrowRight')).toEqual(['Meta', 'ArrowRight']) + }) + + it('handles Mod+Enter and Mod+Escape', () => { + expect(hotkeyToKeys('Mod+Enter')).toEqual(['Meta', 'Enter']) + expect(hotkeyToKeys('Mod+Escape')).toEqual(['Meta', 'Escape']) + }) + + it('handles Mod+. (operation queue toggle / logs reset)', () => { + expect(hotkeyToKeys('Mod+.')).toEqual(['Meta', '.']) + }) + }) + + describe('edge cases', () => { + it('returns a single-element array for empty input', () => { + expect(hotkeyToKeys('')).toEqual(['']) + }) + + it('preserves empty segments from trailing plus', () => { + expect(hotkeyToKeys('Mod+')).toEqual(['Meta', '']) + }) + }) +}) diff --git a/apps/studio/state/shortcuts/useShortcut.test.tsx b/apps/studio/state/shortcuts/useShortcut.test.tsx new file mode 100644 index 00000000000..101ffb00ba8 --- /dev/null +++ b/apps/studio/state/shortcuts/useShortcut.test.tsx @@ -0,0 +1,265 @@ +import { render, renderHook } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { SHORTCUT_DEFINITIONS, SHORTCUT_IDS } from './registry' +import { useShortcut } from './useShortcut' + +const { mockUseHotkeySequence, mockUseRegisterCommands, mockUseIsShortcutEnabled } = vi.hoisted( + () => ({ + mockUseHotkeySequence: vi.fn(), + mockUseRegisterCommands: vi.fn(), + mockUseIsShortcutEnabled: vi.fn(), + }) +) + +vi.mock('@tanstack/react-hotkeys', () => ({ + useHotkeySequence: mockUseHotkeySequence, +})) + +vi.mock('ui-patterns/CommandMenu', () => ({ + useRegisterCommands: mockUseRegisterCommands, +})) + +vi.mock('./useIsShortcutEnabled', () => ({ + useIsShortcutEnabled: mockUseIsShortcutEnabled, +})) + +const getLastHotkeyOptions = () => { + const call = mockUseHotkeySequence.mock.calls.at(-1) + if (!call) throw new Error('useHotkeySequence was not called') + return call[2] as { enabled: boolean; timeout: number | undefined; ignoreInputs?: boolean } +} + +const getLastRegisterCall = () => { + const call = mockUseRegisterCommands.mock.calls.at(-1) + if (!call) throw new Error('useRegisterCommands was not called') + return call as [ + string, + Array<{ id: string; name: string; action: () => void; badge: () => any }>, + { enabled: boolean; deps: unknown[] }, + ] +} + +describe('useShortcut', () => { + beforeEach(() => { + vi.clearAllMocks() + mockUseIsShortcutEnabled.mockReturnValue(true) + }) + + describe('hotkey wiring', () => { + it('passes the registry sequence to useHotkeySequence', () => { + const cb = vi.fn() + renderHook(() => useShortcut(SHORTCUT_IDS.COMMAND_MENU_OPEN, cb)) + + const [sequence, callback] = mockUseHotkeySequence.mock.calls[0] + expect(sequence).toEqual(SHORTCUT_DEFINITIONS[SHORTCUT_IDS.COMMAND_MENU_OPEN].sequence) + expect(callback).toBe(cb) + }) + + it('passes multi-step sequences (G-chords) through unchanged', () => { + renderHook(() => useShortcut(SHORTCUT_IDS.NAV_HOME, vi.fn())) + expect(mockUseHotkeySequence.mock.calls[0][0]).toEqual(['G', 'H']) + }) + + it('wires the callback by reference — useHotkeySequence receives the same function', () => { + const cb = vi.fn() + renderHook(() => useShortcut(SHORTCUT_IDS.ACTION_BAR_SAVE, cb)) + const passedCallback = mockUseHotkeySequence.mock.calls[0][1] + passedCallback() + expect(cb).toHaveBeenCalledTimes(1) + }) + }) + + describe('enabled resolution', () => { + it('defaults to enabled: true when no options and no registry default', () => { + renderHook(() => useShortcut(SHORTCUT_IDS.COMMAND_MENU_OPEN, vi.fn())) + expect(getLastHotkeyOptions().enabled).toBe(true) + }) + + it('caller option takes priority over fallback', () => { + renderHook(() => useShortcut(SHORTCUT_IDS.COMMAND_MENU_OPEN, vi.fn(), { enabled: false })) + expect(getLastHotkeyOptions().enabled).toBe(false) + }) + + it('global disable forces enabled to false, regardless of caller', () => { + mockUseIsShortcutEnabled.mockReturnValue(false) + renderHook(() => useShortcut(SHORTCUT_IDS.COMMAND_MENU_OPEN, vi.fn(), { enabled: true })) + expect(getLastHotkeyOptions().enabled).toBe(false) + }) + + it('global enabled AND caller enabled = true', () => { + mockUseIsShortcutEnabled.mockReturnValue(true) + renderHook(() => useShortcut(SHORTCUT_IDS.COMMAND_MENU_OPEN, vi.fn(), { enabled: true })) + expect(getLastHotkeyOptions().enabled).toBe(true) + }) + + it('global enabled AND caller undefined = true', () => { + mockUseIsShortcutEnabled.mockReturnValue(true) + renderHook(() => useShortcut(SHORTCUT_IDS.COMMAND_MENU_OPEN, vi.fn())) + expect(getLastHotkeyOptions().enabled).toBe(true) + }) + + it('subscribes to the correct shortcut id for global preference', () => { + renderHook(() => useShortcut(SHORTCUT_IDS.NAV_TABLE_EDITOR, vi.fn())) + expect(mockUseIsShortcutEnabled).toHaveBeenCalledWith(SHORTCUT_IDS.NAV_TABLE_EDITOR) + }) + }) + + describe('timeout resolution', () => { + it('defaults to undefined (falls through to TanStack default)', () => { + renderHook(() => useShortcut(SHORTCUT_IDS.COMMAND_MENU_OPEN, vi.fn())) + expect(getLastHotkeyOptions().timeout).toBeUndefined() + }) + + it('uses caller-provided timeout', () => { + renderHook(() => useShortcut(SHORTCUT_IDS.COMMAND_MENU_OPEN, vi.fn(), { timeout: 2000 })) + expect(getLastHotkeyOptions().timeout).toBe(2000) + }) + }) + + describe('ignoreInputs resolution', () => { + it('defaults to undefined when no registry default and no caller override', () => { + renderHook(() => useShortcut(SHORTCUT_IDS.COMMAND_MENU_OPEN, vi.fn())) + expect(getLastHotkeyOptions().ignoreInputs).toBeUndefined() + }) + + it('uses the registry default when no caller override', () => { + renderHook(() => useShortcut(SHORTCUT_IDS.TABLE_EDITOR_JUMP_FIRST_ROW, vi.fn())) + expect(getLastHotkeyOptions().ignoreInputs).toBe(true) + }) + + it('caller override takes priority over registry default', () => { + renderHook(() => + useShortcut(SHORTCUT_IDS.TABLE_EDITOR_JUMP_FIRST_ROW, vi.fn(), { ignoreInputs: false }) + ) + expect(getLastHotkeyOptions().ignoreInputs).toBe(false) + }) + }) + + describe('command menu registration', () => { + it('calls useRegisterCommands under the "Shortcuts" section', () => { + renderHook(() => useShortcut(SHORTCUT_IDS.COMMAND_MENU_OPEN, vi.fn())) + const [section] = getLastRegisterCall() + expect(section).toBe('Shortcuts') + }) + + it('is disabled by default (registerInCommandMenu defaults to false)', () => { + renderHook(() => useShortcut(SHORTCUT_IDS.COMMAND_MENU_OPEN, vi.fn())) + expect(getLastRegisterCall()[2].enabled).toBe(false) + }) + + it('is enabled when registerInCommandMenu: true AND enabled', () => { + renderHook(() => + useShortcut(SHORTCUT_IDS.COMMAND_MENU_OPEN, vi.fn(), { registerInCommandMenu: true }) + ) + expect(getLastRegisterCall()[2].enabled).toBe(true) + }) + + it('is disabled when globally disabled, even with registerInCommandMenu: true', () => { + mockUseIsShortcutEnabled.mockReturnValue(false) + renderHook(() => + useShortcut(SHORTCUT_IDS.COMMAND_MENU_OPEN, vi.fn(), { registerInCommandMenu: true }) + ) + expect(getLastRegisterCall()[2].enabled).toBe(false) + }) + + it('is disabled when caller passes enabled: false, even with registerInCommandMenu: true', () => { + renderHook(() => + useShortcut(SHORTCUT_IDS.COMMAND_MENU_OPEN, vi.fn(), { + enabled: false, + registerInCommandMenu: true, + }) + ) + expect(getLastRegisterCall()[2].enabled).toBe(false) + }) + + it('registers the command with id and label from the registry', () => { + renderHook(() => + useShortcut(SHORTCUT_IDS.RESULTS_COPY_MARKDOWN, vi.fn(), { registerInCommandMenu: true }) + ) + const [, commands] = getLastRegisterCall() + expect(commands).toHaveLength(1) + expect(commands[0].id).toBe(SHORTCUT_IDS.RESULTS_COPY_MARKDOWN) + expect(commands[0].name).toBe(SHORTCUT_DEFINITIONS[SHORTCUT_IDS.RESULTS_COPY_MARKDOWN].label) + }) + + it('command action calls the LATEST callback after a rerender (no stale closure)', () => { + const cb1 = vi.fn() + const cb2 = vi.fn() + const { rerender } = renderHook( + ({ cb }: { cb: () => void }) => + useShortcut(SHORTCUT_IDS.COMMAND_MENU_OPEN, cb, { registerInCommandMenu: true }), + { initialProps: { cb: cb1 } } + ) + + // Capture the action from the first render and fire it after a rerender — + // it should call the NEW callback, proving we dodge the stale closure. + const firstAction = mockUseRegisterCommands.mock.calls[0][1][0].action + + rerender({ cb: cb2 }) + + firstAction() + expect(cb1).not.toHaveBeenCalled() + expect(cb2).toHaveBeenCalledTimes(1) + }) + + it('command action identity is stable across renders', () => { + const { rerender } = renderHook( + ({ cb }: { cb: () => void }) => + useShortcut(SHORTCUT_IDS.COMMAND_MENU_OPEN, cb, { registerInCommandMenu: true }), + { initialProps: { cb: vi.fn() } } + ) + const firstAction = mockUseRegisterCommands.mock.calls[0][1][0].action + + rerender({ cb: vi.fn() }) + const secondAction = mockUseRegisterCommands.mock.calls.at(-1)![1][0].action + + expect(firstAction).toBe(secondAction) + }) + + it('deps track enabled + label so downstream can invalidate correctly', () => { + renderHook(() => + useShortcut(SHORTCUT_IDS.COMMAND_MENU_OPEN, vi.fn(), { registerInCommandMenu: true }) + ) + const [, , options] = getLastRegisterCall() + expect(options.deps).toEqual([ + true, + SHORTCUT_DEFINITIONS[SHORTCUT_IDS.COMMAND_MENU_OPEN].label, + ]) + }) + + describe('badge rendering', () => { + it('renders a single KeyboardShortcut pill for single-step sequences (no "then")', () => { + renderHook(() => + useShortcut(SHORTCUT_IDS.COMMAND_MENU_OPEN, vi.fn(), { registerInCommandMenu: true }) + ) + const badgeNode = getLastRegisterCall()[1][0].badge() + const { container } = render(badgeNode) + expect(container.textContent).not.toContain('then') + }) + + it('renders a "then" separator between steps for multi-step sequences', () => { + renderHook(() => + useShortcut(SHORTCUT_IDS.NAV_HOME, vi.fn(), { registerInCommandMenu: true }) + ) + const badgeNode = getLastRegisterCall()[1][0].badge() + const { container } = render(badgeNode) + expect(container.textContent).toContain('then') + }) + + it('renders the converted keys (Mod → ⌘ or Ctrl via KeyboardShortcut)', () => { + renderHook(() => + useShortcut(SHORTCUT_IDS.RESULTS_COPY_MARKDOWN, vi.fn(), { + registerInCommandMenu: true, + }) + ) + const badgeNode = getLastRegisterCall()[1][0].badge() + const { container } = render(badgeNode) + // Mod+Shift+M → the "M" key is always rendered as-is; the platform-specific + // ⌘/Ctrl handling lives in KeyboardShortcut and is asserted there. + expect(container.textContent).toContain('M') + expect(container.textContent).toContain('⇧') + }) + }) + }) +})