mirror of
https://github.com/aculix/negotium.git
synced 2026-09-11 07:28:17 +00:00
fix: moving a task between days is no longer silent
Deferring was the one action that changed your list and said nothing about it. The task disappeared, no message, and Cmd/Ctrl+Z would not bring it back because the undo stack only recorded deletes and clear-completed. That was worse than it sounds. The app had just taught people, via the bar that appears when something is deleted, that things which vanish come with a way back. Deferring quietly broke that promise, and anyone who misread the arrow would reasonably believe they had destroyed the task. It now uses the same bar, saying which way the task went, and undo sends it back to the row it left rather than the bottom of the list. That needed an insert position on moveTaskToDay, which is tested along with the round trip, clamping and the default append. Undo for a move is handled separately from applyUndo, which edits a single day's array and cannot express something that spans two.
This commit is contained in:
+27
-3
@@ -139,8 +139,21 @@
|
||||
* which day you are looking at, so one control covers both. */
|
||||
function deferTask(taskId) {
|
||||
cancelEdit();
|
||||
const destination = viewingToday ? tomorrowKey() : todayKey;
|
||||
tasks = moveTaskToDay(storage, selectedKey, destination, taskId);
|
||||
|
||||
const index = tasks.findIndex(task => task.id === taskId);
|
||||
if (index === -1) return;
|
||||
|
||||
const movingToTomorrow = viewingToday;
|
||||
const from = selectedKey;
|
||||
const to = movingToTomorrow ? tomorrowKey() : todayKey;
|
||||
|
||||
tasks = moveTaskToDay(storage, from, to, taskId);
|
||||
|
||||
// A task vanishing from the list with nothing said about it reads as a
|
||||
// delete, especially right after the app has taught you that deletes come
|
||||
// with a way back. It gets the same treatment.
|
||||
undoStack.push({ type: 'move', id: taskId, from, to, index });
|
||||
showUndo(movingToTomorrow ? 'Moved to Tomorrow' : 'Moved to Today');
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -170,8 +183,19 @@
|
||||
|
||||
function undo() {
|
||||
const entry = undoStack.pop();
|
||||
if (entry) setTasks(applyUndo(tasks, entry));
|
||||
dismissUndo();
|
||||
if (!entry) return;
|
||||
|
||||
// A move spans two days, so it cannot be expressed as an edit to the one
|
||||
// list applyUndo works on. It is sent back to the row it came from, which
|
||||
// is why the index was recorded.
|
||||
if (entry.type === 'move') {
|
||||
moveTaskToDay(storage, entry.to, entry.from, entry.id, entry.index);
|
||||
tasks = storage.loadTasks(selectedKey);
|
||||
return;
|
||||
}
|
||||
|
||||
setTasks(applyUndo(tasks, entry));
|
||||
}
|
||||
|
||||
function toggleTheme() {
|
||||
|
||||
+12
-2
@@ -9,8 +9,12 @@
|
||||
* A task already sitting at the destination is not duplicated. It still leaves
|
||||
* the source day, which is what someone dragging a stray copy around would
|
||||
* expect.
|
||||
*
|
||||
* `insertAt` places the task at a position rather than at the end. Undoing a
|
||||
* move needs it: a task sent to Tomorrow should come back to the row it left,
|
||||
* not to the bottom of the list.
|
||||
*/
|
||||
export function moveTaskToDay(storage, fromKey, toKey, taskId) {
|
||||
export function moveTaskToDay(storage, fromKey, toKey, taskId, insertAt = null) {
|
||||
const source = storage.loadTasks(fromKey)
|
||||
if (fromKey === toKey) return source
|
||||
|
||||
@@ -22,7 +26,13 @@ export function moveTaskToDay(storage, fromKey, toKey, taskId) {
|
||||
const alreadyThere = destination.some(task => task.id === taskId)
|
||||
|
||||
storage.saveTasks(fromKey, remaining)
|
||||
if (!alreadyThere) storage.saveTasks(toKey, [...destination, moving])
|
||||
|
||||
if (!alreadyThere) {
|
||||
const next = [...destination]
|
||||
const at = insertAt === null ? next.length : Math.max(0, Math.min(insertAt, next.length))
|
||||
next.splice(at, 0, moving)
|
||||
storage.saveTasks(toKey, next)
|
||||
}
|
||||
|
||||
return remaining
|
||||
}
|
||||
|
||||
@@ -81,3 +81,39 @@ describe('moveTaskToDay', () => {
|
||||
expect(storage.loadTasks(TODAY)).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('moveTaskToDay with an insert position', () => {
|
||||
it('inserts at the given index rather than appending', () => {
|
||||
const storage = setup({ [TOMORROW]: [task('x')], [TODAY]: [task('a'), task('b'), task('c')] })
|
||||
moveTaskToDay(storage, TOMORROW, TODAY, 'x', 1)
|
||||
expect(storage.loadTasks(TODAY).map(t => t.id)).toEqual(['a', 'x', 'b', 'c'])
|
||||
})
|
||||
|
||||
it('puts a task back at the front', () => {
|
||||
const storage = setup({ [TOMORROW]: [task('x')], [TODAY]: [task('a')] })
|
||||
moveTaskToDay(storage, TOMORROW, TODAY, 'x', 0)
|
||||
expect(storage.loadTasks(TODAY).map(t => t.id)).toEqual(['x', 'a'])
|
||||
})
|
||||
|
||||
it('clamps an index past the end', () => {
|
||||
const storage = setup({ [TOMORROW]: [task('x')], [TODAY]: [task('a')] })
|
||||
moveTaskToDay(storage, TOMORROW, TODAY, 'x', 99)
|
||||
expect(storage.loadTasks(TODAY).map(t => t.id)).toEqual(['a', 'x'])
|
||||
})
|
||||
|
||||
it('still appends when no index is given', () => {
|
||||
const storage = setup({ [TOMORROW]: [task('x')], [TODAY]: [task('a')] })
|
||||
moveTaskToDay(storage, TOMORROW, TODAY, 'x')
|
||||
expect(storage.loadTasks(TODAY).map(t => t.id)).toEqual(['a', 'x'])
|
||||
})
|
||||
|
||||
it('round-trips a defer back to where it started', () => {
|
||||
const storage = setup({ [TODAY]: [task('a'), task('b'), task('c')] })
|
||||
moveTaskToDay(storage, TODAY, TOMORROW, 'b')
|
||||
expect(storage.loadTasks(TODAY).map(t => t.id)).toEqual(['a', 'c'])
|
||||
|
||||
moveTaskToDay(storage, TOMORROW, TODAY, 'b', 1)
|
||||
expect(storage.loadTasks(TODAY).map(t => t.id)).toEqual(['a', 'b', 'c'])
|
||||
expect(storage.loadTasks(TOMORROW)).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user