diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts index bfe8fce375d..9afb3b09349 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts @@ -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(); diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts index 989578b8748..e003c87a880 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts @@ -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 { 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 { private async moveItem({ listData, previousItemId, - sourceData, + moveUrl, }:{ listData:SortableListData; previousItemId:string|null; - sourceData:SortableItemData; - }):Promise { - const moveUrl = this.resolveMoveUrl(sourceData); - if (!moveUrl) { - return; - } - + moveUrl:string; + }):Promise { const request = new FetchRequest( 'put', moveUrl, @@ -280,9 +297,11 @@ export default class SortableListsController extends Controller { 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); } diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.spec.ts index a792dc0398e..1cc40dfea57 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.spec.ts @@ -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(); + }); + }); }); diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.ts index 9f5362bb58e..f063c8a3bcb 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.ts @@ -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
  • 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
      , so a +// plain list.prepend() would drop the row before that
        . 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,