mirror of
https://github.com/supabase/supabase.git
synced 2026-10-09 03:15:06 +03:00
## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? Feature — a set of new keyboard shortcuts for the table editor, along with infrastructure to register, gate, and surface them. ## What is the current behavior? Clicking into the grid "traps" the keyboard: Escape doesn't pop out, there are no shortcuts for row selection / deletion / navigation, and the search-tables input grabs focus on page load. ## What is the new behavior? ### New shortcuts (all scoped to the table editor) | Keybind | Action | Surface | |---|---|---| | `Esc` | Exit grid selection — clears the highlighted cell and drops focus back to the page | hotkey | | `↑` / `↓` | Start grid navigation from the first cell when no cell is selected | hotkey | | `Shift+Space` | Toggle selection on the current row | hotkey + checkbox tooltip | | `Mod+A` | Toggle selection on all displayed rows (matches Excel) | hotkey + header-checkbox tooltip + Cmd+K | | `Mod+Shift+A` | Toggle selection on all rows in the table | hotkey + "Select all rows in table" button tooltip + Cmd+K | | `Mod+Backspace` | Delete selected rows | hotkey + delete-button tooltip + Cmd+K | ### Infrastructure - **Split registry** — table-editor shortcuts moved to `state/shortcuts/registry/table-editor.ts`, spread into `SHORTCUT_IDS`. Makes it easy to scope a runtime check to a specific surface. - **`eventMatchesAnyShortcut`** (`state/shortcuts/matchEvent.ts`) — queries the hotkey library's live `SequenceManager` so gated shortcuts (`enabled: false`) are correctly excluded. Covered by `matchEvent.test.ts`. - **`handleCellKeyDown`** now calls `event.preventGridDefault()` whenever the keystroke matches an active table-editor shortcut, so rdg's "start editing on key press" default doesn't compete with shortcut actions (e.g. typing `Shift+X` no longer opens edit mode with `X` as input). - **`<Shortcut>` / `<ShortcutTooltip>`** used on the header checkbox, the per-row checkbox, the "Select all rows in table" button, and the delete button — keybinds show up on hover (Linear-style) so users can discover them without reading docs. - **CSS** — `.rdg:not(:focus-within) .rdg-cell[aria-selected='true']` drops the selected-cell outline whenever focus leaves the grid, reinforcing the "you're out" feedback after `Esc`. - **`useShortcut`** wraps the Cmd+K-registered action to close the command menu after firing (previously menu stayed open after selecting an action). - **Search-tables input** no longer auto-focuses on load, so arrow shortcuts work immediately without clicking out first. ## Additional context Linear: FE-3057 ### Test plan - [x] Open any table → `↓` selects the first cell; subsequent arrows navigate rows - [x] `Esc` drops focus out of the grid and re-enables `↓` to re-enter - [x] Click a cell → `Shift+Space` toggles that row's selection (checkbox) - [x] `Mod+A` toggles all displayed rows - [x] With pagination + some rows selected → `Mod+Shift+A` toggles "Select all rows in table" - [x] With rows selected → `Mod+Backspace` deletes them (existing confirmation flow) - [x] Hover the header checkbox / per-row checkbox / delete button → keybind tooltip after ~500ms - [x] Cmd+K with selection → the relevant action shows up; selecting it closes the palette and runs <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added table editor keyboard shortcuts for navigation, row selection, and cell actions, with command-menu integration and visible shortcut tooltips. * **Improvements** * Better keyboard handling in grid cells allowing external shortcuts to override default behavior. * Select-all/deselect-all toggle and improved select-row UX; selected-cell styling no longer shows when grid loses focus. * Command menu now reliably closes before executing shortcut actions. * Removed autofocus on the table editor search input for consistent focus behavior. * **Tests** * Added unit tests covering shortcut matching and command-menu shortcut behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
122 lines
3.6 KiB
TypeScript
122 lines
3.6 KiB
TypeScript
import { getSequenceManager } from '@tanstack/react-hotkeys'
|
|
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
|
|
|
import { eventMatchesAnyShortcut } from './matchEvent'
|
|
import type { RegistryDefinations } from './types'
|
|
|
|
vi.mock('@tanstack/react-hotkeys', async () => {
|
|
const actual =
|
|
await vi.importActual<typeof import('@tanstack/react-hotkeys')>('@tanstack/react-hotkeys')
|
|
return {
|
|
...actual,
|
|
getSequenceManager: vi.fn(),
|
|
}
|
|
})
|
|
|
|
type FakeRegistration = {
|
|
options: { enabled?: boolean }
|
|
sequence: string[]
|
|
}
|
|
|
|
/** Swap the SequenceManager's registrations for the ones we care about. */
|
|
function withRegistrations(registrations: FakeRegistration[]) {
|
|
const state = new Map(registrations.map((r, i) => [String(i), r]))
|
|
vi.mocked(getSequenceManager).mockReturnValue({
|
|
registrations: { state },
|
|
} as unknown as ReturnType<typeof getSequenceManager>)
|
|
}
|
|
|
|
const shiftX = new KeyboardEvent('keydown', {
|
|
key: 'X',
|
|
code: 'KeyX',
|
|
shiftKey: true,
|
|
})
|
|
|
|
const arrowDown = new KeyboardEvent('keydown', {
|
|
key: 'ArrowDown',
|
|
code: 'ArrowDown',
|
|
})
|
|
|
|
const g = new KeyboardEvent('keydown', {
|
|
key: 'g',
|
|
code: 'KeyG',
|
|
})
|
|
|
|
const shiftK = new KeyboardEvent('keydown', {
|
|
key: 'K',
|
|
code: 'KeyK',
|
|
shiftKey: true,
|
|
})
|
|
|
|
const registry: RegistryDefinations<string> = {
|
|
'table.shift-x': {
|
|
id: 'table.shift-x',
|
|
label: 'Toggle row',
|
|
sequence: ['Shift+X'],
|
|
},
|
|
'table.arrow-down': {
|
|
id: 'table.arrow-down',
|
|
label: 'Start navigation (down)',
|
|
sequence: ['ArrowDown'],
|
|
},
|
|
'table.chord': {
|
|
id: 'table.chord',
|
|
label: 'Chord',
|
|
sequence: ['G', 'T'],
|
|
},
|
|
}
|
|
|
|
describe('eventMatchesAnyShortcut', () => {
|
|
beforeEach(() => {
|
|
vi.clearAllMocks()
|
|
})
|
|
|
|
it('returns false when there are no active registrations', () => {
|
|
withRegistrations([])
|
|
expect(eventMatchesAnyShortcut(shiftX, registry)).toBe(false)
|
|
})
|
|
|
|
it('returns true when an active, enabled shortcut in scope matches', () => {
|
|
withRegistrations([{ options: {}, sequence: ['Shift+X'] }])
|
|
expect(eventMatchesAnyShortcut(shiftX, registry)).toBe(true)
|
|
})
|
|
|
|
it('returns false when the matching shortcut is disabled', () => {
|
|
withRegistrations([{ options: { enabled: false }, sequence: ['ArrowDown'] }])
|
|
expect(eventMatchesAnyShortcut(arrowDown, registry)).toBe(false)
|
|
})
|
|
|
|
it('treats enabled: undefined and enabled: true as enabled', () => {
|
|
withRegistrations([
|
|
{ options: {}, sequence: ['Shift+X'] },
|
|
{ options: { enabled: true }, sequence: ['ArrowDown'] },
|
|
])
|
|
expect(eventMatchesAnyShortcut(arrowDown, registry)).toBe(true)
|
|
})
|
|
|
|
it('returns false when the matching registration is not in the target registry', () => {
|
|
// 'Shift+K' is registered and matches the event, but our scoped registry
|
|
// doesn't include it — so we should not claim a match.
|
|
withRegistrations([{ options: {}, sequence: ['Shift+K'] }])
|
|
expect(eventMatchesAnyShortcut(shiftK, registry)).toBe(false)
|
|
})
|
|
|
|
it('matches any individual step of a chord sequence', () => {
|
|
withRegistrations([{ options: {}, sequence: ['G', 'T'] }])
|
|
expect(eventMatchesAnyShortcut(g, registry)).toBe(true)
|
|
})
|
|
|
|
it('returns true if any enabled duplicate matches, even when a disabled one is also present', () => {
|
|
withRegistrations([
|
|
{ options: { enabled: false }, sequence: ['Shift+X'] },
|
|
{ options: {}, sequence: ['Shift+X'] },
|
|
])
|
|
expect(eventMatchesAnyShortcut(shiftX, registry)).toBe(true)
|
|
})
|
|
|
|
it('returns false when no active sequence matches the event', () => {
|
|
withRegistrations([{ options: {}, sequence: ['Shift+X'] }])
|
|
expect(eventMatchesAnyShortcut(arrowDown, registry)).toBe(false)
|
|
})
|
|
})
|