diff --git a/src/App.svelte b/src/App.svelte index 4752897..8ce5b70 100644 --- a/src/App.svelte +++ b/src/App.svelte @@ -9,6 +9,7 @@ import { rollover, msUntilNextMidnight } from './lib/rollover.js'; import * as taskOps from './lib/tasks.js'; import { createUndoStack, applyUndo } from './lib/undo.js'; + import { shouldHandleUndo } from './lib/shortcuts.js'; const storage = createStorage(); const undoStack = createUndoStack(); @@ -88,9 +89,7 @@ } function handleGlobalKeydown(event) { - if (!(event.metaKey || event.ctrlKey) || event.key.toLowerCase() !== 'z') return; - // Leave the text field its own native undo. - if (event.target instanceof HTMLInputElement) return; + if (!shouldHandleUndo(event)) return; event.preventDefault(); undo(); } diff --git a/src/lib/shortcuts.js b/src/lib/shortcuts.js new file mode 100644 index 0000000..0127e31 --- /dev/null +++ b/src/lib/shortcuts.js @@ -0,0 +1,32 @@ +const TEXT_FIELDS = new Set(['INPUT', 'TEXTAREA']) + +/** + * Whether a keydown should trigger task undo. + * + * The subtlety is the text field. Deferring to a focused field's own undo + * sounds right, but the add-task input is where focus normally sits — you + * click it to add a task and focus stays there, and on macOS clicking a + * button does not move focus. Guarding on focus alone therefore disables + * undo in precisely the situation it is needed: right after deleting a task. + * + * So it defers only when the field actually holds text worth undoing. An + * empty input has nothing for the browser to restore, and task undo wins. + */ +export function shouldHandleUndo(event) { + const isUndoChord = + (event.metaKey || event.ctrlKey) && + !event.shiftKey && // Shift+Cmd+Z means redo, which this app does not have. + !event.altKey && + typeof event.key === 'string' && + event.key.toLowerCase() === 'z' + + if (!isUndoChord) return false + + const target = event.target + if (!target) return true + + if (target.isContentEditable) return false + if (TEXT_FIELDS.has(target.tagName) && target.value) return false + + return true +} diff --git a/src/lib/shortcuts.test.js b/src/lib/shortcuts.test.js new file mode 100644 index 0000000..64fa83b --- /dev/null +++ b/src/lib/shortcuts.test.js @@ -0,0 +1,78 @@ +import { describe, it, expect } from 'vitest' +import { shouldHandleUndo } from './shortcuts.js' + +const evt = (overrides = {}) => ({ + metaKey: false, + ctrlKey: false, + shiftKey: false, + altKey: false, + key: 'z', + target: { tagName: 'BODY' }, + ...overrides, +}) + +describe('shouldHandleUndo', () => { + it('accepts Cmd+Z', () => { + expect(shouldHandleUndo(evt({ metaKey: true }))).toBe(true) + }) + + it('accepts Ctrl+Z', () => { + expect(shouldHandleUndo(evt({ ctrlKey: true }))).toBe(true) + }) + + it('accepts an uppercase key from caps lock', () => { + expect(shouldHandleUndo(evt({ metaKey: true, key: 'Z' }))).toBe(true) + }) + + it('rejects Z with no modifier', () => { + expect(shouldHandleUndo(evt())).toBe(false) + }) + + it('rejects a different letter', () => { + expect(shouldHandleUndo(evt({ metaKey: true, key: 'y' }))).toBe(false) + }) + + it('rejects Shift+Cmd+Z, which means redo', () => { + expect(shouldHandleUndo(evt({ metaKey: true, shiftKey: true }))).toBe(false) + }) + + it('rejects Alt+Cmd+Z', () => { + expect(shouldHandleUndo(evt({ metaKey: true, altKey: true }))).toBe(false) + }) + + it('tolerates a missing key', () => { + expect(shouldHandleUndo(evt({ metaKey: true, key: undefined }))).toBe(false) + }) + + // The regression this module exists for: the add-task input is where focus + // normally sits, so guarding on focus alone disabled undo in the one + // situation it is actually needed. + it('handles undo when the focused input is empty', () => { + const target = { tagName: 'INPUT', value: '' } + expect(shouldHandleUndo(evt({ metaKey: true, target }))).toBe(true) + }) + + it('defers to the field when the focused input has text to undo', () => { + const target = { tagName: 'INPUT', value: 'half-typed task' } + expect(shouldHandleUndo(evt({ metaKey: true, target }))).toBe(false) + }) + + it('defers to a textarea holding text', () => { + const target = { tagName: 'TEXTAREA', value: 'notes' } + expect(shouldHandleUndo(evt({ metaKey: true, target }))).toBe(false) + }) + + it('defers to a contenteditable target', () => { + const target = { tagName: 'DIV', isContentEditable: true } + expect(shouldHandleUndo(evt({ metaKey: true, target }))).toBe(false) + }) + + it('handles undo when the target is a button', () => { + const target = { tagName: 'BUTTON' } + expect(shouldHandleUndo(evt({ metaKey: true, target }))).toBe(true) + }) + + it('tolerates a missing target', () => { + expect(shouldHandleUndo(evt({ metaKey: true, target: null }))).toBe(true) + }) +})