Skip to content

Extract a drag session from the sortable-lists root #25792

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
myabc wants to merge 4 commits into from
Draft

Extract a drag session from the sortable-lists root #25792

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
abe468c
Pin the drop over a batch-mate's row
myabc Oct 2, 2026
f35e8e0
Add a drag session for sortable lists
myabc Oct 2, 2026
bca2a86
Route drag state through the drag session
myabc Oct 2, 2026
97a6232
Name the Stimulus drag payload after its role
myabc Oct 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
178 changes: 134 additions & 44 deletions frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts
View file
Open in desktop

Large diffs are not rendered by default.

177 changes: 61 additions & 116 deletions frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,10 @@ import {
type SortableListData,
type SortableListsRoot,
} from './sortable-lists/drag-and-drop';
import { selectionKey, type SelectionItem, type SelectionKey } from 'core-common/batch-selection';
import { selectionKey, type SelectionItem } from 'core-common/batch-selection';
import {
captureRowPositions,
isOrderableItem,
itemAcceptsDestination,
reorderRows,
resolveDirectionalPreviousItemId,
resolveItemId,
Expand All @@ -68,7 +67,8 @@ import {
type MoveDirection,
} from './sortable-lists/list-dom';
import { SelectionOrchestrator, type SelectionHost } from './sortable-lists/selection-orchestrator';
import { itemIdentity, orderedItemElements } from './sortable-lists/selection';
import { itemElementsByKey, itemIdentity } from './sortable-lists/selection';
import { DragSession, type DragSessionHost } from './sortable-lists/drag-session';

type CleanupFn = () => void;
type ElementDropPayload = ElementEventPayloadMap['onDrop'];
Expand All @@ -83,7 +83,7 @@ function relativeUrl(url:URL):string {
return `${url.pathname}${url.search}${url.hash}`;
}

export default class SortableListsController extends Controller<HTMLElement> implements SortableListsRoot, SelectionHost {
export default class SortableListsController extends Controller<HTMLElement> implements SortableListsRoot, SelectionHost, DragSessionHost {
static outlets = ['sortable-lists--list', 'sortable-lists--item', 'sortable-lists--scrollable'];

static values = {
Expand Down Expand Up @@ -125,6 +125,11 @@ export default class SortableListsController extends Controller<HTMLElement> imp
this.monitorCleanupFn = monitorForElements({
canMonitor: ({ source }) => !this.busy
&& isItemFromRoot(this.element, source.data),
// Pragmatic dispatches onDragStart a frame after the preview; an item
// controller reconnecting in between must not leave the batch unmarked.
onDragStart: () => {
this.dragSession?.start();
},
onDrop: (args) => {
void this.handleDrop(args);
},
Expand All @@ -146,9 +151,7 @@ export default class SortableListsController extends Controller<HTMLElement> imp
this.monitorCleanupFn = undefined;
// A drag in flight when the controller disconnects would otherwise leave
// its marks in the cached page and its frozen batch in this instance.
this.clearDraggingRows();
this.activeDragBatch = null;
this.dragOwnerDestinations = null;
this.endDrag();
}

// A Turbo morph can toggle the permission-gated value on a live root
Expand Down Expand Up @@ -235,118 +238,61 @@ export default class SortableListsController extends Controller<HTMLElement> imp
}
}

// Frozen at drag start and consumed exactly once per drop, cancelled ones
// included: neither Escape nor a mid-drag morph can change what is
// submitted, and no stale batch leaks into the next drag.
private activeDragBatch:SelectionItem[]|null = null;
// The drag in flight, or the prospective session of a press that did not
// become one (Pragmatic checks the drag handle after canDrag).
dragSession:DragSession|null = null;

// Every item drop target asks for its owner on each dragover, and the
// answer holds for the whole drag, so it is remembered alongside the batch.
private dragOwnerDestinations:WeakMap<HTMLElement, DestinationIdentity|null>|null = null;
beginDrag(itemElement:HTMLElement):DragSession {
// A session a press outside the drag handle or an aborted dragstart left
// behind holds no marks and no batch; ending it keeps one session per root.
this.endDrag();
const session = new DragSession(this, itemElement);

// Pragmatic dispatches onGenerateDragPreview before onDragStart; the
// preview needs the count, the drag start marks the rows.
freezeDragBatch(itemElement:HTMLElement):number {
const scope = this.selection?.selectForAction(itemElement);
this.activeDragBatch = scope?.kind === 'batch'
? scope.items.map((item) => itemIdentity(item)).filter((item):item is SelectionItem => item !== null)
: null;
this.dragOwnerDestinations = new WeakMap();
// Announced from canDrag, the earliest point a drag can be stopped, so
// an oversized batch is told before any preview or drop feedback appears.
if (session.refused) {
void announce(
I18n.t(`${this.moveAnnouncementScopeValue}.batch_too_large`, { count: session.size, max: this.maxBatchSizeValue }),
{ politeness: 'assertive' },
);
return session;
}

return Math.max(1, this.activeDragBatch?.length ?? 0);
this.dragSession = session;
return session;
}

markDragBatch():void {
if (this.activeDragBatch) {
this.markDraggingRows(this.activeDragBatch);
}
private endDrag():SelectionItem[]|null {
const batch = this.dragSession?.end() ?? null;
this.dragSession = null;
return batch;
}

// The destinations every member of the prospective batch accepts, null when
// the block reaches all of them. A batch may span lists, so a member that
// only accepts its own pins the block there, never to the dragged card's
// list.
dragPermittedDestinations(itemElement:HTMLElement):DestinationIdentity[]|null {
const scope = this.selection?.actionScopeFor(itemElement);
const members = scope?.kind === 'batch' ? scope.items : [itemElement];
const ownerDestinationOf = (item:HTMLElement) => this.ownerDestinationOf(item);

const lists = this.ownedListOutlets();
const permitted = lists
.map((list) => destinationOfList(list.listData))
.filter((destination) => members.every((member) => itemAcceptsDestination(member, destination, ownerDestinationOf)));

return permitted.length === lists.length ? null : permitted;
get maxBatchSize():number {
return this.maxBatchSizeValue;
}

// Asked in canDrag, the earliest point a drag can be stopped: an oversized
// batch is told so before any preview or drop feedback appears.
dragRefused(itemElement:HTMLElement):boolean {
if (this.maxBatchSizeValue <= 0) {
return false;
}

prospectiveMembers(itemElement:HTMLElement):HTMLElement[] {
const scope = this.selection?.actionScopeFor(itemElement);
const count = scope?.kind === 'batch' ? scope.items.length : 1;
if (count <= this.maxBatchSizeValue) {
return false;
}
return scope?.kind === 'batch' ? scope.items : [itemElement];
}

void announce(
I18n.t(`${this.moveAnnouncementScopeValue}.batch_too_large`, { count, max: this.maxBatchSizeValue }),
{ politeness: 'assertive' },
);
return true;
frozenMembers(itemElement:HTMLElement):SelectionItem[]|null {
const scope = this.selection?.selectForAction(itemElement);
return scope?.kind === 'batch'
? scope.items.map((item) => itemIdentity(item)).filter((item):item is SelectionItem => item !== null)
: null;
}

externalDragItems(itemElement:HTMLElement):HTMLElement[] {
const scope = this.selection?.actionScopeFor(itemElement);
return scope?.kind === 'batch' ? scope.items : [itemElement];
ownedDestinations():DestinationIdentity[] {
return this.ownedListOutlets().map((list) => destinationOfList(list.listData));
}

// Outlets match document-wide; another root's lists are not ours.
private ownedListOutlets() {
return this.sortableListsListOutlets.filter((list) => this.element.contains(list.element));
}

// Marked on the item element itself, the same one the item controller's
// own onDragStart marks, so CSS keys off one convention regardless of
// which controller did the marking.
private markDraggingRows(items:SelectionItem[]):void {
const elements = this.itemElementsByKey();
items.forEach((item) => {
elements.get(selectionKey(item))?.setAttribute('data-dragging', 'source');
});
}

// Every mark under the root, not just the frozen batch's own rows: a
// cancelled drop, or the item controller's onDrop missing a row, would
// otherwise leave one behind.
private clearDraggingRows():void {
this.element.querySelectorAll('[data-dragging]').forEach((element) => element.removeAttribute('data-dragging'));
}

// One document query per callback; never kept, so a morph cannot leave it
// stale. Keyed on type as well as id: ids collide across source tables.
private itemElementsByKey():Map<SelectionKey, HTMLElement> {
const map = new Map<SelectionKey, HTMLElement>();
orderedItemElements(this.element).forEach((element) => {
const identity = itemIdentity(element);
if (identity) {
map.set(selectionKey(identity), element);
}
});
return map;
}

private takeActiveDragBatch():SelectionItem[]|null {
const batch = this.activeDragBatch;
this.clearDraggingRows();
this.activeDragBatch = null;
this.dragOwnerDestinations = null;
return batch;
}

// A morph desyncs the children's drag-and-drop state in two ways. Stimulus
// outlet-connected callbacks do not fire reliably for elements a morph
// replaces, so those children never receive the root reference and refuse
Expand Down Expand Up @@ -393,12 +339,10 @@ export default class SortableListsController extends Controller<HTMLElement> imp
// the model.
this.selection?.reconcile();

// A row a morph replaces mid-drag comes back as fresh server HTML that
// never went through markDragBatch, so it loses data-dragging with the
// element it replaced.
if (this.activeDragBatch) {
this.markDraggingRows(this.activeDragBatch);
}
// A morph can reparent a row the owner memo already answered for and
// replace a marked row with fresh server HTML without its mark.
this.dragSession?.forgetOwners();
this.dragSession?.remark();
});
};

Expand Down Expand Up @@ -520,22 +464,23 @@ export default class SortableListsController extends Controller<HTMLElement> imp
return this.ownerListOf(itemElement)?.rowsContainer ?? null;
}

// Remembered only once the drag is real: a prospective session left by a
// press outside the handle must not pin answers given outside any drag.
ownerDestinationOf(element:HTMLElement):DestinationIdentity|null {
const remembered = this.dragOwnerDestinations?.get(element);
if (remembered !== undefined) {
return remembered;
}
return this.dragSession && this.dragSession.phase !== 'prospective'
? this.dragSession.ownerDestinationOf(element)
: this.liveOwnerDestinationOf(element);
}

liveOwnerDestinationOf(element:HTMLElement):DestinationIdentity|null {
const listData = this.ownerListOf(element)?.listData;
const destination = listData ? destinationOfList(listData) : null;
this.dragOwnerDestinations?.set(element, destination);
return destination;
return listData ? destinationOfList(listData) : null;
}

private async handleDrop({ location, source }:ElementDropPayload) {
// Before any bail-out below: a cancelled drop still consumes the frozen
// snapshot rather than leaking it into the next drag.
const frozenBatch = this.takeActiveDragBatch();
// Before any bail-out below: a cancelled drop still ends the session
// rather than leaking its frozen batch into the next drag.
const frozenBatch = this.endDrag();

if (this.busy) {
debugLog('sortable-lists: ignoring drop, a move is already in progress');
Expand Down Expand Up @@ -628,7 +573,7 @@ export default class SortableListsController extends Controller<HTMLElement> imp
// Refused whole when a row is missing: a member that vanished mid-drag
// means a partial block would diverge from the ids the request claims.
private rowsForItems(items:SelectionItem[]):HTMLElement[]|null {
const elements = this.itemElementsByKey();
const elements = itemElementsByKey(this.element);
const rows:HTMLElement[] = [];

for (const item of items) {
Expand Down
7 changes: 4 additions & 3 deletions frontend/src/stimulus/controllers/dynamic/sortable-lists/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,10 @@ reconciliation preserves it.

## Batch movement

Dragging a selected item moves the whole batch. The root freezes the batch in
the preview callback (`freezeDragBatch`) and marks its rows at drag start
(`markDragBatch`), so later selection changes do not change the submitted items.
Dragging a selected item moves the whole batch. The drag session
([drag-session.ts](drag-session.ts)) resolves the batch when the drag is first
permitted, freezes it in the preview callback and marks its rows at drag
start, so later selection changes do not change the submitted items.
Dragging an unselected item selects it, collapsing any wider selection.

A batch may drop only on a destination every member accepts. A `confined` member
Expand Down
Loading
Loading