[AGILE-284] Reorder sortable lists optimistically

Moves the dragged row to its target position in the DOM as soon as the
drop lands and persists the move in the background, so that list
reordering no longer blocks on the server round-trip. A rejected move
restores the row to its original slot and eases it back with a FLIP
animation, while the forward drop needs none since Pragmatic DnD already
leaves the row where the user released it.

https://community.openproject.org/wp/AGILE-284
This commit is contained in:
Alexander Brandon Coles
2026-06-09 19:07:12 +01:00
parent e51e0ad6ca
commit 63ddbb2ae9
4 changed files with 347 additions and 12 deletions
@@ -209,6 +209,11 @@ describe('Sortable lists controller', () => {
return vi.mocked(dropTargetForElements).mock.calls.find(([options]) => options.element === element)?.[0];
}
function itemIds(list:HTMLElement):string[] {
return Array.from(list.querySelectorAll('[data-sortable-lists--item-id-value]'))
.map((element) => element.getAttribute('data-sortable-lists--item-id-value')!);
}
beforeEach(async () => {
vi.clearAllMocks();
@@ -460,6 +465,49 @@ describe('Sortable lists controller', () => {
window.removeEventListener('op:toasters:add', onToast);
});
it('moves the dropped row into the target list before the move request resolves', async () => {
let resolveMove:(response:{ ok:boolean }) => void;
vi.mocked(FetchRequest).mockImplementationOnce(function FetchRequest() {
return {
perform: vi.fn(() => new Promise<{ ok:boolean }>((resolve) => {
resolveMove = resolve;
})),
};
});
const { sourceList, targetList, firstSourceItem } = renderFixture();
await ctx.nextFrame();
await dropCurrentItemOnList(firstSourceItem, targetList);
// The row is appended optimistically while the request is still pending.
expect(itemIds(targetList)).toEqual(['4', '5', '1']);
expect(itemIds(sourceList)).toEqual(['2', '3']);
resolveMove!({ ok: true });
await flushPromises();
expect(itemIds(targetList)).toEqual(['4', '5', '1']);
});
it('rolls the dropped row back to its original position when the move request fails', async () => {
vi.mocked(FetchRequest).mockImplementationOnce(function FetchRequest() {
return {
perform: vi.fn(() => Promise.reject(new Error('Network failure'))),
};
});
const { sourceList, targetList, firstSourceItem } = renderFixture();
await ctx.nextFrame();
await dropCurrentItemOnList(firstSourceItem, targetList);
await flushPromises();
expect(itemIds(sourceList)).toEqual(['1', '2', '3']);
expect(itemIds(targetList)).toEqual(['4', '5']);
});
it('registers Backlogs lists as drop targets', async () => {
const { sourceList, targetList } = renderFixture();
@@ -45,12 +45,16 @@ import {
isSortableItemData,
isSourceListTarget,
resolveFallbackDropTarget,
captureRowPositions,
flipMove,
reorderRows,
restoreRowPositions,
resolveListAppendPreviousItemId,
resolveListData,
resolvePreviousSortableItemId,
sortableItemSelector,
sortableListSelector,
sortableListsMovingAttribute,
type SortableItemData,
type SortableListData,
} from './sortable-lists/drag-and-drop';
@@ -225,11 +229,29 @@ export default class SortableListsController extends Controller<HTMLElement> {
list: targetElement,
});
await this.moveItem({
const sourceRow = source.element.closest('li');
const listElement = targetElement.closest(sortableListSelector);
if (!(sourceRow instanceof HTMLElement) || !(listElement instanceof HTMLElement)) {
return;
}
// Move the row optimistically, then persist. The server response (a
// turbo-stream) reconciles the list on success; a failure rolls the row
// back to where it started.
const rows = [sourceRow];
const rollback = captureRowPositions(rows);
reorderRows({ rows, list: listElement, previousItemId });
const moved = await this.moveItem({
listData,
previousItemId,
sourceData: source.data,
moveUrl,
});
if (!moved) {
flipMove(rows, () => restoreRowPositions(rollback));
this.dispatchErrorToast();
}
}
// A Turbo morph can strip the Pragmatic DnD attributes/listeners an item
@@ -249,17 +271,12 @@ export default class SortableListsController extends Controller<HTMLElement> {
private async moveItem({
listData,
previousItemId,
sourceData,
moveUrl,
}:{
listData:SortableListData;
previousItemId:string|null;
sourceData:SortableItemData;
}):Promise<void> {
const moveUrl = this.resolveMoveUrl(sourceData);
if (!moveUrl) {
return;
}
moveUrl:string;
}):Promise<boolean> {
const request = new FetchRequest(
'put',
moveUrl,
@@ -280,9 +297,11 @@ export default class SortableListsController extends Controller<HTMLElement> {
if (!response.ok) {
debugLog(`Failed to move sortable list item: ${response.statusCode}`);
}
return response.ok;
} catch (error) {
debugLog('Failed to move sortable list item due to request error', error);
this.dispatchErrorToast();
return false;
} finally {
this.setMoving(false);
}
@@ -26,17 +26,23 @@
// See COPYRIGHT and LICENSE files for more details.
//++
import { vi } from 'vitest';
import { extractClosestEdge } from '@atlaskit/pragmatic-drag-and-drop-hitbox/closest-edge';
import {
acceptsSortableItemType,
buildMoveFormData,
captureRowPositions,
flipMove,
isSortableItemData,
isSortableListData,
reorderRows,
resolveFallbackDropTarget,
resolveItemType,
resolveListAppendPreviousItemId,
resolveListData,
resolvePreviousSortableItemId,
restoreRowPositions,
sortableItemData,
sortableListData,
} from './drag-and-drop';
@@ -436,4 +442,166 @@ describe('sortable lists drag and drop helpers', () => {
expect(resolveListAppendPreviousItemId({ sourceItemId: '1', list })).toBeNull();
});
});
function listElement():HTMLUListElement {
const list = document.createElement('ul');
list.setAttribute('data-sortable-lists-target', 'list');
return list;
}
function itemIdOrder(list:HTMLElement):string[] {
return Array.from(list.querySelectorAll('[data-sortable-lists--item-id-value]'))
.map((element) => element.getAttribute('data-sortable-lists--item-id-value')!);
}
describe('reorderRows', () => {
it('moves a row to sit immediately after the previous item anchor', () => {
const list = listElement();
const [one, two, three] = ['1', '2', '3'].map(itemRow);
list.append(one, two, three);
reorderRows({ rows: [one], list, previousItemId: '2' });
expect(itemIdOrder(list)).toEqual(['2', '1', '3']);
});
it('moves a row to the top of the list before the first existing row', () => {
const list = listElement();
const [one, two, three] = ['1', '2', '3'].map(itemRow);
list.append(one, two, three);
reorderRows({ rows: [three], list, previousItemId: null });
expect(itemIdOrder(list)).toEqual(['3', '1', '2']);
});
it('keeps a top-of-list move inside a nested list element instead of escaping it', () => {
const list = document.createElement('div');
const inner = document.createElement('ul');
const [one, two, three] = ['1', '2', '3'].map(itemRow);
list.setAttribute('data-sortable-lists-target', 'list');
inner.append(one, two, three);
list.append(inner);
reorderRows({ rows: [three], list, previousItemId: null });
expect(three.parentElement).toBe(inner);
expect(itemIdOrder(list)).toEqual(['3', '1', '2']);
});
it('inserts a moved group after the anchor preserving their order', () => {
const list = listElement();
const [one, two, three, four] = ['1', '2', '3', '4'].map(itemRow);
list.append(one, two, three, four);
reorderRows({ rows: [three, four], list, previousItemId: '1' });
expect(itemIdOrder(list)).toEqual(['1', '3', '4', '2']);
});
it('anchors on a truncation marker row when the previous item is hidden', () => {
const list = listElement();
const [one, two, three] = ['1', '2', '3'].map(itemRow);
const marker = showMoreRow('hidden');
list.append(three, one, marker, two);
reorderRows({ rows: [three], list, previousItemId: 'hidden' });
expect(three.previousElementSibling).toBe(marker);
expect(itemIdOrder(list)).toEqual(['1', '3', '2']);
});
});
describe('captureRowPositions / restoreRowPositions', () => {
it('restores a row to its original position after an optimistic move', () => {
const list = listElement();
const [one, two, three] = ['1', '2', '3'].map(itemRow);
list.append(one, two, three);
const snapshot = captureRowPositions([three]);
reorderRows({ rows: [three], list, previousItemId: null });
expect(itemIdOrder(list)).toEqual(['3', '1', '2']);
restoreRowPositions(snapshot);
expect(itemIdOrder(list)).toEqual(['1', '2', '3']);
});
it('restores a multi-row group to its original order', () => {
const list = listElement();
const [one, two, three, four] = ['1', '2', '3', '4'].map(itemRow);
list.append(one, two, three, four);
const snapshot = captureRowPositions([two, three]);
reorderRows({ rows: [two, three], list, previousItemId: '4' });
expect(itemIdOrder(list)).toEqual(['1', '4', '2', '3']);
restoreRowPositions(snapshot);
expect(itemIdOrder(list)).toEqual(['1', '2', '3', '4']);
});
});
describe('flipMove', () => {
afterEach(() => {
vi.restoreAllMocks();
});
function stubReducedMotion(matches:boolean) {
vi.spyOn(window, 'matchMedia').mockReturnValue({ matches } as MediaQueryList);
}
function stubRects(row:HTMLElement, first:{ top:number; left:number }, last:{ top:number; left:number }) {
vi.spyOn(row, 'getBoundingClientRect')
.mockReturnValueOnce({ ...first } as DOMRect)
.mockReturnValueOnce({ ...last } as DOMRect);
}
it('runs the mutation and animates each row from its previous position', () => {
stubReducedMotion(false);
const row = itemRow('1');
const animate = vi.spyOn(row, 'animate').mockReturnValue({} as Animation);
const mutate = vi.fn();
stubRects(row, { top: 100, left: 20 }, { top: 0, left: 0 });
flipMove([row], mutate);
expect(mutate).toHaveBeenCalledOnce();
expect(animate).toHaveBeenCalledWith(
[{ transform: 'translate(20px, 100px)' }, { transform: 'none' }],
{ duration: 200, easing: 'ease' },
);
});
it('does not animate a row that did not move', () => {
stubReducedMotion(false);
const row = itemRow('1');
const animate = vi.spyOn(row, 'animate');
const mutate = vi.fn();
stubRects(row, { top: 50, left: 0 }, { top: 50, left: 0 });
flipMove([row], mutate);
expect(mutate).toHaveBeenCalledOnce();
expect(animate).not.toHaveBeenCalled();
});
it('applies the mutation without animating when reduced motion is preferred', () => {
stubReducedMotion(true);
const row = itemRow('1');
const animate = vi.spyOn(row, 'animate');
const mutate = vi.fn();
flipMove([row], mutate);
expect(mutate).toHaveBeenCalledOnce();
expect(animate).not.toHaveBeenCalled();
});
});
});
@@ -286,6 +286,106 @@ export function resolvePreviousSortableItemId({
return null;
}
export interface RowPlacement {
row:HTMLElement;
parent:Node|null;
nextSibling:Node|null;
}
// Snapshot each row's current location so an optimistic move can be undone if
// the server rejects it. Captured before the move; restored in reverse so the
// stored nextSibling references are still valid when reinserting.
export function captureRowPositions(rows:HTMLElement[]):RowPlacement[] {
return rows.map((row) => ({
row,
parent: row.parentNode,
nextSibling: row.nextSibling,
}));
}
export function restoreRowPositions(positions:RowPlacement[]):void {
for (let i = positions.length - 1; i >= 0; i -= 1) {
const { row, parent, nextSibling } = positions[i];
parent?.insertBefore(row, nextSibling);
}
}
// FLIP: run a DOM mutation, then slide each affected row from where it was to
// where it landed. Used to soften the rollback when a move is rejected.
// Honours prefers-reduced-motion by applying the mutation without animation.
export function flipMove(rows:HTMLElement[], mutate:() => void, durationMs = 200):void {
if (window.matchMedia('(prefers-reduced-motion: reduce)').matches) {
mutate();
return;
}
const firstRects = rows.map((row) => row.getBoundingClientRect());
mutate();
rows.forEach((row, index) => {
const last = row.getBoundingClientRect();
const dx = firstRects[index].left - last.left;
const dy = firstRects[index].top - last.top;
if (dx || dy) {
row.animate(
[{ transform: `translate(${dx}px, ${dy}px)` }, { transform: 'none' }],
{ duration: durationMs, easing: 'ease' },
);
}
});
}
// Optimistically move rows on the client without waiting for the server.
// `rows` are the moved <li>s in order (one today, the selected set once
// multi-item DnD lands); `previousItemId` of null means top of list.
export function reorderRows({
rows,
list,
previousItemId,
}:{
rows:HTMLElement[];
list:HTMLElement;
previousItemId:string|null;
}):void {
let anchor:Element|null = previousItemId ? resolveAnchorRow(list, previousItemId) : null;
for (const row of rows) {
if (anchor) {
anchor.after(row);
} else {
insertAtListTop(list, row);
}
anchor = row;
}
}
// The previous item id can point at a hidden item collapsed behind a truncation
// marker row, which carries the id on data-sortable-lists-prev-item-id rather
// than exposing an item element. Anchor on that marker so the row lands next to
// the collapsed block instead of jumping to the top.
function resolveAnchorRow(list:HTMLElement, previousItemId:string):HTMLElement|null {
const escaped = CSS.escape(previousItemId);
const anchor = list.querySelector(`[data-sortable-lists--item-id-value="${escaped}"]`)
?? list.querySelector(`[${sortablePreviousItemIdAttribute}="${escaped}"]`);
return anchor?.closest('li') ?? null;
}
// Rows can sit directly under the list element or inside a nested <ul>, so a
// plain list.prepend() would drop the row before that <ul>. Insert before the
// first existing row instead, keeping it in the same container as its siblings.
function insertAtListTop(list:HTMLElement, row:HTMLElement):void {
const firstRow = list.querySelector(':scope > li, :scope > ul > li');
if (firstRow && firstRow !== row) {
firstRow.before(row);
} else if (!firstRow) {
(list.querySelector(':scope > ul') ?? list).prepend(row);
}
}
export function resolveListAppendPreviousItemId({
sourceItemId,
list,