mirror of
https://github.com/aculix/negotium.git
synced 2026-09-11 07:28:17 +00:00
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.
This commit is contained in:
+2
-3
@@ -9,6 +9,7 @@
|
|||||||
import { rollover, msUntilNextMidnight } from './lib/rollover.js';
|
import { rollover, msUntilNextMidnight } from './lib/rollover.js';
|
||||||
import * as taskOps from './lib/tasks.js';
|
import * as taskOps from './lib/tasks.js';
|
||||||
import { createUndoStack, applyUndo } from './lib/undo.js';
|
import { createUndoStack, applyUndo } from './lib/undo.js';
|
||||||
|
import { shouldHandleUndo } from './lib/shortcuts.js';
|
||||||
|
|
||||||
const storage = createStorage();
|
const storage = createStorage();
|
||||||
const undoStack = createUndoStack();
|
const undoStack = createUndoStack();
|
||||||
@@ -88,9 +89,7 @@
|
|||||||
}
|
}
|
||||||
|
|
||||||
function handleGlobalKeydown(event) {
|
function handleGlobalKeydown(event) {
|
||||||
if (!(event.metaKey || event.ctrlKey) || event.key.toLowerCase() !== 'z') return;
|
if (!shouldHandleUndo(event)) return;
|
||||||
// Leave the text field its own native undo.
|
|
||||||
if (event.target instanceof HTMLInputElement) return;
|
|
||||||
event.preventDefault();
|
event.preventDefault();
|
||||||
undo();
|
undo();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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
|
||||||
|
}
|
||||||
@@ -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)
|
||||||
|
})
|
||||||
|
})
|
||||||
Reference in New Issue
Block a user