mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-18 08:56:41 +00:00
* feat(tasks): allow dragging tasks into subtask lists
* fix(tasks): support childless subtask drop targets
* fix(tasks): support dragging subtasks back to main list
* fix(tasks): preserve nested subtask sorting
* fix(tasks): address subtask drag review feedback
* fix(tasks): address drag conversion review
* test: skip onboarding in migration fresh-start e2e
* fix(tasks): harden convertToSubTask guards and dedupe tag cleanup
Address review findings on the drag-to-subtask feature (#7905):
- Keep the section and crud meta-reducer guards in lock-step via a shared
canApplyConvertToSubTask(). Previously the section reducer stripped a task
from its section even when the crud reducer rejected the convert (missing
target parent or self-target), leaving the task top-level yet dropped from
section ordering on a replayed/concurrent op.
- Reject nesting under a target that is itself a subtask. The UI renders only
two levels, so deeper nesting would orphan the task and leave the
grandparent's time aggregation stale.
- Tighten op-log payload validation to require string taskId/targetParentId.
- Dedupe the "remove task ids from all tags" logic shared by
convertToSubTask/deleteTask/deleteTasks into removeTasksFromAllTags().
- Collapse the tri-state afterTaskId positioning in handleConvertToMainTask
(undefined and null both prepend via moveItemAfterAnchor).
- Name the DragPointer type in DropListService.
Adds reducer specs for the rejected-target-parent cases.
* fix(tasks): remove empty subtask drop-target for childless parents
The dashed empty sub-task drop zone that appeared under childless parent
tasks during a drag was unwanted UI. Remove it along with its supporting
machinery (the subTaskDropCandidate signal in ScheduleExternalDragService,
the pointerdown candidate-arming in TaskListComponent, the
isEmptySubTaskDropTargetMounted computed, and the related template/SCSS).
Consequence: a task can now be nested by dragging only onto a parent that
already has a subtask list. Dragging a subtask back out to a main list
(convertToMainTask) and nesting into an existing subtask list
(convertToSubTask) are unchanged.
* fix(tasks): reliably convert subtasks dragged to the top-level list
Two issues prevented dragging a subtask out to the top-level list to
convert it into a main task:
- CDK only caches a sibling drop-list's geometry when its enterPredicate
passes at drag start (_startReceiving). The pointer is always over the
source subtask list then, so the top-level list was never cached and
conversion silently failed until an unrelated parent drag warmed it.
Open a one-microtask accept window at subtask drag start so CDK caches
the top-level lists' geometry; the pointer guard resumes afterwards.
- An expanded neighbour's subtask list "caught" the drag in the dead-band
just above the next parent (sibling order resolves subtask lists before
the top-level list), silently re-parenting the subtask instead of
converting it, and growing/sticking once entered. Treat only an actual
subtask row as "inside" a foreign subtask list; its trailing padding now
falls through to the top-level list for conversion. The source list
still blocks anywhere, so in-list sorting is unaffected.
* fix(tasks): use midpoint-crossing sort for drag preview placement
CDK's SingleAxisSortStrategy swaps siblings as soon as the cursor enters
any part of their clientRect. For task lists whose item rects span the
parent's full element (header + its expanded subtask list), that swap
displaces the drop preview far below the cursor — landing it past the
target's subtasks instead of where the user is pointing.
Monkey-patch the strategy's `_getItemIndexFromPointerPosition` with a
relative-position midpoint rule (the pattern used by dnd-kit and
react-beautiful-dnd): a sibling only swaps with the dragged item once
the cursor crosses into the half of the sibling that's on the dragged
item's approach side. `enter()` keeps CDK's first-inside semantics so
initial placement still lands somewhere sensible.
Also extend `_pointerSubTaskList()` with a `.sub-tasks` wrapper fallback
so the leading strip between a parent's header and its first subtask
routes the drag into that SUB list at index 0 instead of falling through
to the top-level list and converting at the parent's slot.
* test(tasks): align drag predicate spec data
* test(tasks): reconcile createMockDrop calls after combining drag PRs
Combining the two #7905 PR lineages crossed a stale signature:
the midpoint-sort commit added enterPredicate cases calling the old
3-arg createMockDrop(modelId, filteredTasks, listId), while keilogic's
spec-alignment commit narrowed it to (modelId, listId) since
enterPredicate never reads filteredTasks. Drop the vestigial arrays
from the three affected cases so they match the simplified helper.
* fix(tasks): widen first/last subtask drop targets
Dragging a task to the first/last position of a subtask list was hard:
the midpoint sort patch gives each slot a half-row trigger, and the
two end slots have no neighbouring row to borrow the other half from,
so they were only half of the edge row. The existing "easier dragging"
padding lived on :host, outside the `.task-list-inner` cdkDropList, so
it never grew the CDK hit-rect.
Move that padding inside the sub-task drop list so the strip above the
first and below the last subtask is part of the drop rect. CDK's
enter() then drops a task arriving there as the first/last child
(SingleAxisSortStrategy._shouldEnterAsFirstChild). The matching :host
padding is dropped for sub-lists so the visible gap stays compact.
* refactor(tasks): apply multi-review follow-ups to subtask drag
Hardening and fixes surfaced by the multi-agent review of the
drag-into-subtask feature:
- Scope the CDK midpoint sort patch to VERTICAL lists. It mutates the
shared SingleAxisSortStrategy prototype app-wide, so horizontal lists
(boards, issue panel) were silently swept into the new hit-test;
gating to vertical keeps them on CDK's stock behaviour and sidesteps
the right-to-left index-inversion question. Update the spec to assert
the horizontal fall-back.
- Keep first-child re-parent in a sub-list's leading pad. The drop-target
CSS fix moved the SUB list's top padding inside `.task-list-inner`, so
part of the header→first-row strip now lives in the drop rect; report
the leading pad as a row (re-parent) while the trailing pad stays a
convert-to-main dead-band. Doc comment updated to match.
- Memoise the per-pointer subtask-list hit-test in DropListService so the
several enterPredicate calls CDK fires per pointer move reuse one
document.elementFromPoint.
- Tighten convertToSubTask op-log validation to isValidEntityId (rejects
'' / 'undefined' / 'null') instead of a bare typeof string check.
- Log ids, not full task objects, in the drop handler (titles/notes must
not reach the exportable log).
- Drop the dead _resetMidpointSortPatchForTests export; note why the
seeded drag pointer is inert after a plain tap.
- Add direct unit tests for the canConvert/canApply guard pair.
* test(tasks): cover drop()-to-convert dispatch and the drag e2e
Close the two coverage gaps flagged in review:
- Unit: drive the public drop() handler with CdkDragDrop events and
assert the resulting convertToSubTask / convertToMainTask dispatch,
including the newIds→afterTaskId placement math (after-anchor,
first-slot null anchor) and the isDone flag for the DONE list. This
was previously untested — only _move() and the reducer were.
- E2E: a real CDK drag of a top-level task onto a subtask row, asserting
it converts to a subtask. Uses the manual stepped-mouse gesture (CDK
ignores HTML5 dragTo), matching work-view/sections.spec.ts. Verified
green 3× in a row.
* fix(tasks): cut drop snap-back flicker by emitting default list sync
On drop, CDK tears down its preview/placeholder but never moves the real
DOM node (the list is NgRx-driven), so the un-moved task is briefly
revealed at its old slot until the store re-render lands. customizeUndone
Tasks() pushed *every* work-view list emission through
observeOn(animationFrameScheduler), adding a guaranteed extra frame to
that re-render and widening the visible snap-back.
Keep the frame-defer only for the customized (sort/group/filter) path —
it does heavier work and is driven by CD-bound signals, where a sync emit
can re-enter change detection. The default path is store-driven only, so
emit it on the same tick: the list re-renders without the extra frame and
the dropped task lands in place with far less (ideally no) snap-back.
task-view-customizer spec (43) green; drag-into-subtask e2e green.
* fix(tasks): scope CDK midpoint sort patch to task-list instances
Patch the dragged list's own SingleAxisSortStrategy instance instead of
the shared prototype. registerDropList is only called by task lists, so
the prototype mutation silently changed the hit-test of every other
vertical CDK list (planner, notes, boards, tree-dnd). Shadowing the
method per instance keeps the midpoint-crossing rule scoped to task
lists and leaves all other lists on CDK's stock behaviour.
The CDK-rename guard and 'this' binding are unchanged; the global
isPatched flag is dropped since each instance now patches independently.
* fix(sync): validate afterTaskId in convertToSubTask payload
The convertToSubTask op-log validation branch accepted any payload with
valid taskId/targetParentId, ignoring afterTaskId. Require it to be a
string or null (the action type's contract) so a crafted/malformed sync
payload is rejected at the validation boundary rather than treated as a
not-found anchor downstream.
* docs(tasks): clarify customizer frame-defer and drag-to-done intent
Comment-only. Correct the task-view-customizer note: the customized
path stays on the animation-frame scheduler to batch the signal-driven
emission burst on context switch (commit fddedf3fa6), not because a sync
emit re-enters change detection (the consumer is toSignal). Document
that dragging a subtask onto the DONE list intentionally converts it to
a main task done today, even outside the Today context.
* fix(tasks): keep midpoint sort patch alive across CDK strategy recreation
The previous commit scoped the patch by shadowing the method on the
strategy *instance*. That regressed subtask drag 100% of the time: CDK
rebuilds the SingleAxisSortStrategy on every drag start
(DropListRef.withOrientation runs in its beforeStarted hook), so the
instance shadow was discarded before the first pointer move and task
lists fell back to CDK's stock first-inside hit-test — the exact tall-
item preview misplacement the patch exists to prevent.
Patch the shared prototype instead (survives recreation) but apply the
midpoint rule only when the strategy's container is a registered task
list, delegating to CDK's original otherwise. This keeps the scoping
goal (planner/notes/boards/tree-dnd stay on stock behaviour) without the
recreation fragility. Extract the scoping dispatcher as a pure function
and unit-test it.
---------
Co-authored-by: kei <keletrh@gmail.com>
109 lines
4 KiB
TypeScript
109 lines
4 KiB
TypeScript
import { expect, test } from '../../fixtures/test.fixture';
|
|
import type { Locator, Page } from '@playwright/test';
|
|
|
|
// End-to-end coverage for the "drag a task into a subtask list" feature
|
|
// (PR #7944 / #7905). The unit specs cover the reducer + the drop() payload in
|
|
// isolation; this exercises the real CDK drag → enterPredicate → convert flow.
|
|
test.describe('Drag task into subtask list', () => {
|
|
const stableBoundingBox = async (
|
|
locator: Locator,
|
|
): Promise<{ x: number; y: number; width: number; height: number }> => {
|
|
await locator.waitFor({ state: 'visible' });
|
|
let box = await locator.boundingBox();
|
|
// Wait until layout settles (non-null, non-zero height).
|
|
await expect
|
|
.poll(async () => {
|
|
box = await locator.boundingBox();
|
|
return !!box && box.height > 0;
|
|
})
|
|
.toBe(true);
|
|
if (!box) throw new Error('drag source/target has no bounding box');
|
|
return box;
|
|
};
|
|
|
|
/**
|
|
* CDK drag-drop is event-driven; Playwright's `dragTo` uses HTML5 DnD which
|
|
* CDK ignores. Drive the gesture manually so the CDK threshold + drag start
|
|
* fire (same approach as work-view/sections.spec.ts).
|
|
*/
|
|
const cdkDragTo = async (
|
|
page: Page,
|
|
source: Locator,
|
|
target: Locator,
|
|
): Promise<void> => {
|
|
const s = await stableBoundingBox(source);
|
|
const t = await stableBoundingBox(target);
|
|
/* eslint-disable no-mixed-operators */
|
|
const sx = s.x + s.width / 2;
|
|
const sy = s.y + s.height / 2;
|
|
const tx = t.x + t.width / 2;
|
|
const ty = t.y + t.height / 2;
|
|
/* eslint-enable no-mixed-operators */
|
|
await page.mouse.move(sx, sy);
|
|
await page.mouse.down();
|
|
// Nudge past CDK's 5px drag threshold, then move smoothly so each
|
|
// mousemove re-evaluates the drop target.
|
|
await page.mouse.move(sx + 10, sy + 10, { steps: 5 });
|
|
await page.mouse.move(tx, ty, { steps: 20 });
|
|
await page.mouse.up();
|
|
};
|
|
|
|
const disableAnimations = async (page: Page): Promise<void> => {
|
|
await expect
|
|
.poll(() =>
|
|
page.evaluate(() => {
|
|
const store = (
|
|
window as unknown as {
|
|
__e2eTestHelpers?: { store?: { dispatch: (a: unknown) => void } };
|
|
}
|
|
).__e2eTestHelpers?.store;
|
|
if (!store) return false;
|
|
store.dispatch({
|
|
type: '[Global Config] Update Global Config Section',
|
|
sectionKey: 'misc',
|
|
sectionCfg: { isDisableAnimations: true },
|
|
isSkipSnack: true,
|
|
});
|
|
return true;
|
|
}),
|
|
)
|
|
.toBe(true);
|
|
await expect(page.locator('body.isDisableAnimations')).toBeVisible();
|
|
};
|
|
|
|
test('converts a top-level task to a subtask when dropped onto a subtask list', async ({
|
|
page,
|
|
workViewPage,
|
|
}) => {
|
|
await workViewPage.waitForTaskList();
|
|
|
|
// Parent with one existing subtask → its subtask drop list is rendered.
|
|
await workViewPage.addTask('DragParent');
|
|
const parent = page.locator('task').filter({ hasText: 'DragParent' }).first();
|
|
await workViewPage.addSubTask(parent, 'ExistingSub');
|
|
await parent.locator('.sub-tasks task').first().waitFor({ state: 'visible' });
|
|
|
|
// A second top-level task to drag in.
|
|
await workViewPage.addTask('DragMover');
|
|
const mover = page.locator('task').filter({ hasText: 'DragMover' }).first();
|
|
await expect(mover).toBeVisible();
|
|
|
|
await disableAnimations(page);
|
|
|
|
// Drop onto the centre of the existing subtask row — squarely inside the
|
|
// subtask drop list, the robust core of the convert-to-subtask feature.
|
|
const dragHandle = mover.locator('done-toggle').first();
|
|
const subRow = parent
|
|
.locator('.sub-tasks task')
|
|
.filter({ hasText: 'ExistingSub' })
|
|
.first();
|
|
await cdkDragTo(page, dragHandle, subRow);
|
|
|
|
// DragMover is now a subtask of DragParent.
|
|
const subTasks = parent.locator('.sub-tasks task');
|
|
await expect(subTasks.filter({ hasText: 'DragMover' })).toBeVisible({
|
|
timeout: 5000,
|
|
});
|
|
await expect(subTasks).toHaveCount(2);
|
|
});
|
|
});
|