From 143fcc205d4d139a3e0a7a2c7a19aa0501ecd53f Mon Sep 17 00:00:00 2001 From: Aculix Technologies Date: Sun, 16 Aug 2026 01:20:55 +0530 Subject: [PATCH] fix: undo no longer swallowed when focus is in the task input Cmd/Ctrl+Z did nothing in normal use. The handler bailed out whenever the event target was an input, which was meant to protect the field's own text undo. But the add-task input is where focus usually sits: you click it to add a task and stay there, and on macOS clicking a button doesn't move focus. So the guard killed undo in the one situation it exists for, right after deleting something. It now defers to the field only when the field actually holds text. An empty input has nothing to restore. The decision moves to src/lib/shortcuts.js as a pure function, because the inline version couldn't be tested. Shift+Cmd+Z means redo and no longer triggers an undo. --- src/App.svelte | 5 +-- src/lib/shortcuts.js | 32 ++++++++++++++++ src/lib/shortcuts.test.js | 78 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 src/lib/shortcuts.js create mode 100644 src/lib/shortcuts.test.js 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) + }) +})