Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ import {
isOrderableItem,
itemAcceptsDestination,
reorderRows,
resolveBlockMove,
resolveBlockMoveAvailability,
resolveDirectionalPreviousItemId,
resolveItemId,
resolveItemLabel,
Expand Down Expand Up @@ -490,20 +492,29 @@ export default class SortableListsController extends Controller<HTMLElement> imp
return this.element.hasAttribute(sortableListsBusyAttribute);
}

// A direction is offered exactly when the move resolver can produce a
// target for it, and never for an item moveInDirection would refuse below.
// Null means the item is not in an owned list yet. A snapshot for menu
// gating; the click path re-resolves the live DOM.
// A direction is offered exactly when the move resolver can produce a target
// for it, which keeps the menu honest about truncated lists. Null means the
// item is not in an owned list yet, or takes no part in ordering. A snapshot
// for menu gating; the click path re-resolves the live DOM.
moveAvailability(itemElement:HTMLElement):MoveAvailability|null {
if (!isOrderableItem(itemElement)) {
return {
top: false, up: false, down: false, bottom: false,
};
const list = this.ownerListOf(itemElement);
if (!list || !isOrderableItem(itemElement)) {
return null;
}

const list = this.ownerListOf(itemElement);
const scope = this.actionScopeFor(itemElement);
if (scope.kind === 'refused') {
return resolveMoveAvailability({
itemElement,
rowsContainer: list.rowsContainer,
});
}
Comment thread
myabc marked this conversation as resolved.

return list ? resolveMoveAvailability({ itemElement, rowsContainer: list.rowsContainer }) : null;
if (!this.resolveCollectionMoveUrl()) {
return { top: false, up: false, down: false, bottom: false };
}

return resolveBlockMoveAvailability({ itemElements: scope.items, rowsContainer: list.rowsContainer });
}

moveToDestination(itemElement:HTMLElement, target:DestinationIdentity):void {
Expand Down Expand Up @@ -537,6 +548,62 @@ export default class SortableListsController extends Controller<HTMLElement> imp
return;
}

if (this.selection) {
const moveUrl = this.resolveCollectionMoveUrl();
if (!moveUrl) {
return;
}

// Resolved without mutating: every check below can still refuse the
// move, and a stale menu must not replace the user's batch with the
// invoker for a move that then never runs.
const scope = this.actionScopeFor(itemElement);
if (scope.kind === 'refused') {
return;
}

const list = this.ownerListOf(itemElement);
if (!list) {
return;
}

const resolution = resolveBlockMove({
itemElements: scope.items,
direction,
rowsContainer: list.rowsContainer,
});
if (!resolution.available) {
return;
}

// Scope members are resolved candidates, so both identity attributes
// exist; refusing on a mismatch keeps the moved rows and the submitted
// ids from ever diverging.
const items = scope.items.flatMap((element):SelectionItem[] => {
const type = resolveItemType(element);
const id = resolveItemId(element);
return type && id ? [{ type, id }] : [];
});
if (items.length !== scope.items.length) {
return;
}

// Committed only now the move is known executable: invoking a position
// action on an unselected card selects it, and a failed request keeps
// that selection for the retry.
this.selectForAction(itemElement);

void this.performMove({
rows: resolution.rows,
items,
rowsContainer: list.rowsContainer,
listData: list.listData,
previousItemId: resolution.previousItemId,
moveUrl,
});
return;
}

const list = this.ownerListOf(itemElement);
if (!list) {
return;
Expand All @@ -558,8 +625,6 @@ export default class SortableListsController extends Controller<HTMLElement> imp
return;
}

this.selection?.collapseForAction(itemElement);

void this.performMove({
rows: [sourceRow],
items: null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ export interface SortableListsRoot {
availableDestinations(scope:ActionScope, candidates:DestinationIdentity[]):DestinationIdentity[];
moveToDestination(itemElement:HTMLElement, target:DestinationIdentity):void;
moveInDirection(itemElement:HTMLElement, direction:MoveDirection):void;
// A snapshot for menu gating; the click path re-resolves against the live DOM.
// A snapshot for menu gating over the invoker's prospective action scope;
// the click path re-resolves against the live DOM.
moveAvailability(itemElement:HTMLElement):MoveAvailability|null;
// The element of the list an item currently belongs to; null outside any
// registered list. Items carry no list reference, so the root resolves it.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ describe('Sortable lists item controller', () => {
return {
element,
busy,
actionScopeFor: vi.fn((item:HTMLElement):ActionScope => ({ kind: 'refused', items: [] })),
selectForAction: vi.fn((item:HTMLElement):ActionScope => ({ kind: 'refused', items: [] })),
actionScopeFor: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
selectForAction: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
availableDestinations: vi.fn(() => []),
moveToDestination: vi.fn(),
moveInDirection: vi.fn(),
Expand Down Expand Up @@ -1124,8 +1124,8 @@ describe('Sortable lists item controller', () => {
controller.connectRoot({
element: row,
busy: false,
actionScopeFor: vi.fn((item:HTMLElement):ActionScope => ({ kind: 'refused', items: [] })),
selectForAction: vi.fn((item:HTMLElement):ActionScope => ({ kind: 'refused', items: [] })),
actionScopeFor: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
selectForAction: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
availableDestinations: vi.fn(() => []),
moveToDestination: vi.fn(),
moveInDirection: vi.fn(),
Expand Down Expand Up @@ -1222,8 +1222,8 @@ describe('Sortable lists item controller', () => {
controller.connectRoot({
element: row,
busy: false,
actionScopeFor: vi.fn((actionItem:HTMLElement):ActionScope => ({ kind: 'refused', items: [] })),
selectForAction: vi.fn((actionItem:HTMLElement):ActionScope => ({ kind: 'refused', items: [] })),
actionScopeFor: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
selectForAction: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
availableDestinations: vi.fn(() => []),
moveToDestination: vi.fn(),
moveInDirection: vi.fn(),
Expand Down Expand Up @@ -1415,16 +1415,16 @@ describe('Sortable lists item controller', () => {
) => ({
element: el,
busy: false,
actionScopeFor: vi.fn((item:HTMLElement):ActionScope => ({ kind: 'refused', items: [] })),
selectForAction: vi.fn((item:HTMLElement):ActionScope => ({ kind: 'refused', items: [] })),
actionScopeFor: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
selectForAction: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
availableDestinations: vi.fn(() => []),
moveToDestination: vi.fn(),
moveAvailability: () => availability,
moveInDirection,
} as unknown as SortableListsRoot);

function stubMenuRoot(el:HTMLElement, position:{ isFirst:boolean; isLast:boolean }) {
const actionScopeFor = vi.fn((item:HTMLElement):ActionScope => ({ kind: 'refused', items: [] }));
const actionScopeFor = vi.fn(():ActionScope => ({ kind: 'refused', items: [] }));
const availableDestinations = vi.fn((_scope:ActionScope, _candidates:DestinationIdentity[]):DestinationIdentity[] => []);
const root = { ...stubRoot(el, position), actionScopeFor, availableDestinations };

Expand Down Expand Up @@ -1546,7 +1546,7 @@ describe('Sortable lists item controller', () => {
expect(menu.showItem).toHaveBeenCalledWith(moveToInbox);
});

it('hides destination actions and the position submenu for a true multi-card scope', async () => {
it('shows available batch position directions when every destination action is hidden', async () => {
const { el, menu } = renderItemWithMenu(1, true);
document.body.appendChild(el);
const controller = await mountItemController(el);
Expand All @@ -1556,6 +1556,7 @@ describe('Sortable lists item controller', () => {
const { root, actionScopeFor, availableDestinations } = stubMenuRoot(el, { isFirst: false, isLast: false });
actionScopeFor.mockReturnValue(scope);
availableDestinations.mockReturnValue([]);
root.moveAvailability = () => ({ top: false, up: true, down: true, bottom: false });
controller.connectRoot(root);

const moveToSprint = destinationFor(el, [{ type: 'sprint', id: '1' }]);
Expand All @@ -1566,8 +1567,35 @@ describe('Sortable lists item controller', () => {
const divider = el.querySelector<HTMLElement>('li[data-sortable-lists--item-target="moveDivider"]')!;
expect(menu.hideItem).toHaveBeenCalledWith(moveToSprint);
expect(menu.hideItem).toHaveBeenCalledWith(moveToInbox);
expect(menu.showItem).toHaveBeenCalledWith(moveMenu);
expect(menu.hideItem).toHaveBeenCalledWith(liFor(el, 'top'));
expect(menu.showItem).toHaveBeenCalledWith(liFor(el, 'up'));
expect(menu.showItem).toHaveBeenCalledWith(liFor(el, 'down'));
expect(menu.hideItem).toHaveBeenCalledWith(liFor(el, 'bottom'));
expect(divider.hasAttribute('hidden')).toBe(false);
});

it('hides an all-unavailable batch position submenu and its directions', async () => {
const { el, menu } = renderItemWithMenu(1, true);
document.body.appendChild(el);
const controller = await mountItemController(el);
const scope:ActionScope = { kind: 'batch', items: [el] };
const { root, actionScopeFor, availableDestinations } = stubMenuRoot(el, { isFirst: false, isLast: false });
actionScopeFor.mockReturnValue(scope);
availableDestinations.mockReturnValue([]);
root.moveAvailability = () => ({ top: false, up: false, down: false, bottom: false });
controller.connectRoot(root);

const moveToSprint = destinationFor(el, [{ type: 'sprint', id: '1' }]);
await menuCtx!.nextFrame();

const moveMenu = el.querySelector<HTMLElement>('li[data-sortable-lists--item-target="moveMenu"]')!;
const divider = el.querySelector<HTMLElement>('li[data-sortable-lists--item-target="moveDivider"]')!;
expect(menu.hideItem).toHaveBeenCalledWith(moveToSprint);
expect(menu.hideItem).toHaveBeenCalledWith(moveMenu);
expect(menu.hideItem).not.toHaveBeenCalledWith(liFor(el, 'top'));
for (const direction of ['top', 'up', 'down', 'bottom']) {
expect(menu.hideItem).toHaveBeenCalledWith(liFor(el, direction));
}
expect(divider.hasAttribute('hidden')).toBe(true);
});

Expand Down Expand Up @@ -1909,8 +1937,8 @@ describe('Sortable lists item controller', () => {
const root:SortableListsRoot = {
element: item,
busy: false,
actionScopeFor: vi.fn((actionItem:HTMLElement):ActionScope => ({ kind: 'refused', items: [] })),
selectForAction: vi.fn((actionItem:HTMLElement):ActionScope => ({ kind: 'refused', items: [] })),
actionScopeFor: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
selectForAction: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
availableDestinations: vi.fn(() => []),
moveToDestination: vi.fn(),
moveInDirection: vi.fn(),
Expand Down Expand Up @@ -1942,8 +1970,8 @@ describe('Sortable lists item controller', () => {
const root:SortableListsRoot = {
element: item,
busy: false,
actionScopeFor: vi.fn((actionItem:HTMLElement):ActionScope => ({ kind: 'refused', items: [] })),
selectForAction: vi.fn((actionItem:HTMLElement):ActionScope => ({ kind: 'refused', items: [] })),
actionScopeFor: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
selectForAction: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
availableDestinations: vi.fn(() => []),
moveToDestination: vi.fn(),
moveInDirection: vi.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ export default class ItemController extends Controller<HTMLElement> implements R

const scope = root.actionScopeFor(this.element);
this.refreshDestinationAvailability(root, scope);
this.refreshMoveMenuAvailability(root, scope);
this.refreshMoveMenuAvailability(root);
this.refreshMoveDivider();
}

Expand All @@ -550,14 +550,7 @@ export default class ItemController extends Controller<HTMLElement> implements R
}
}

private refreshMoveMenuAvailability(root:SortableListsRoot, scope:ActionScope):void {
if (scope.kind === 'batch' && scope.items.length > 1) {
if (this.hasMoveMenuTarget) {
this.setAvailability(this.moveMenuTarget, false);
}
return;
}

private refreshMoveMenuAvailability(root:SortableListsRoot):void {
// Null availability means the item is not in a list yet; leave the menu
// alone until the outlet wiring settles.
const availability = root.moveAvailability(this.element);
Expand Down
Loading
Loading