Skip to content

fix(list, block-group): update menu item labels and call canPut / canPull to filter items before opening a sort handle menu #12487

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

Merged
driskull merged 6 commits into from
Jul 22, 2025
Merged

fix(list, block-group): update menu item labels and call canPut / canPull to filter items before opening a sort handle menu #12487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cf24486
wip
driskull Jul 10, 2025
6818a62
wip
driskull Jul 10, 2025
c239381
update demo
driskull Jul 10, 2025
d73abe4
demo
driskull Jul 10, 2025
e704f49
fix(list, block-group): update menu item labels and call 'canPut/canP…
driskull Jul 10, 2025
30762df
add tests
driskull Jul 10, 2025
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
69 changes: 16 additions & 53 deletions packages/calcite-components/src/components/block-group/block-group.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { Reorder } from "../sort-handle/interfaces";
import { SLOTS as BLOCK_SLOTS } from "../block/resources";
import { Block } from "../block/block";
import { mockConsole } from "../../tests/utils/logging";
import { IDS } from "../sort-handle/resources";
import { BlockDragDetail } from "./interfaces";
import type { BlockGroup } from "./block-group";

Expand Down Expand Up @@ -149,9 +150,6 @@ describe("calcite-block-group", () => {
endOldIndex: number;
startNewIndex: number;
startOldIndex: number;
moveHaltNewIndex: number;
moveHaltOldIndex: number;
moveHaltCalledTimes: number;
}>;

it("works using a mouse", async () => {
Expand Down Expand Up @@ -356,82 +354,47 @@ describe("calcite-block-group", () => {
it("calls canPull and canPut for move items", async () => {
const page = await newE2EPage();
await page.setContent(html`
<calcite-block-group id="first-letters" drag-enabled group="letters">
<calcite-block-group label="First Letters" id="first-letters" drag-enabled group="letters">
<calcite-block id="a" heading="a" label="A"></calcite-block>
<calcite-block id="b" heading="b" label="B"></calcite-block>
</calcite-block-group>
<calcite-block-group id="second-letters" drag-enabled group="letters">
<calcite-block-group label="Second Letters" id="second-letters" drag-enabled group="letters">
<calcite-block id="c" heading="c" label="C"></calcite-block>
<calcite-block id="d" heading="d" label="D"></calcite-block>
</calcite-block-group>
`);

// Workaround for page.spyOnEvent() failing due to drag event payload being serialized and there being circular JSON structures from the payload elements. See: https://github.com/Esri/calcite-design-system/issues/7643
await page.evaluate(() => {
const testWindow = window as TestWindow;
testWindow.moveHaltCalledTimes = 0;
const firstLetters = document.getElementById("first-letters") as BlockGroup["el"];

firstLetters.addEventListener("calciteBlockGroupMoveHalt", (event: CustomEvent<BlockDragDetail>) => {
testWindow.moveHaltCalledTimes++;
testWindow.moveHaltNewIndex = event.detail.newIndex;
testWindow.moveHaltOldIndex = event.detail.oldIndex;
});

firstLetters.canPull = ({ dragEl }) => dragEl.id === "b";
firstLetters.canPut = ({ dragEl }) => dragEl.id === "c";
});
await page.waitForChanges();

async function clickMoveDropdownItem(id: string) {
async function getMoveItems(id: string) {
const component = await page.find(`#${id}`);
component.setProperty("sortHandleOpen", true);
await page.waitForChanges();

const dropdownItem = await page.find(`#${id} >>> calcite-dropdown-group:last-child calcite-dropdown-item`);
expect(dropdownItem).not.toBeNull();
await dropdownItem.click();

await page.waitForChanges();
}

async function getResults() {
return await page.evaluate(() => {
const testWindow = window as TestWindow;

return {
moveHaltCalledTimes: testWindow.moveHaltCalledTimes,
moveHaltOldIndex: testWindow.moveHaltOldIndex,
moveHaltNewIndex: testWindow.moveHaltNewIndex,
};
return await findAll(page, `#${id} >>> calcite-dropdown-group#${IDS.move} calcite-dropdown-item`, {
allowEmpty: true,
});
}

await clickMoveDropdownItem("a");
let results = await getResults();

expect(results.moveHaltCalledTimes).toBe(1);
expect(results.moveHaltNewIndex).toBe(0);
expect(results.moveHaltOldIndex).toBe(0);

await clickMoveDropdownItem("b");
results = await getResults();

expect(results.moveHaltCalledTimes).toBe(1);
expect(results.moveHaltNewIndex).toBe(0);
expect(results.moveHaltNewIndex).toBe(0);

await clickMoveDropdownItem("c");
results = await getResults();
const aMoveItems = await getMoveItems("a");
expect(aMoveItems.length).toBe(0);

expect(results.moveHaltCalledTimes).toBe(1);
const bMoveItems = await getMoveItems("b");
expect(bMoveItems.length).toBe(1);
expect(await bMoveItems[0].getProperty("label")).toBe("Second Letters");

await clickMoveDropdownItem("d");
results = await getResults();
const cMoveItems = await getMoveItems("c");
expect(cMoveItems.length).toBe(1);
expect(await cMoveItems[0].getProperty("label")).toBe("First Letters");

expect(results.moveHaltCalledTimes).toBe(2);
expect(results.moveHaltNewIndex).toBe(0);
expect(results.moveHaltOldIndex).toBe(1);
const dMoveItems = await getMoveItems("d");
expect(dMoveItems.length).toBe(0);
});

it("reorders using a keyboard", async () => {
Expand Down
138 changes: 87 additions & 51 deletions packages/calcite-components/src/components/block-group/block-group.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export class BlockGroup extends LitElement implements InteractiveComponent, Sort
handleSelector = "calcite-sort-handle";

mutationObserver = createObserver("mutation", () => {
this.updateBlockItems();
this.updateBlockItemsDebounced();
});

private parentBlockGroupEl: BlockGroup["el"];
Expand All @@ -59,23 +59,7 @@ export class BlockGroup extends LitElement implements InteractiveComponent, Sort

private cancelable = useCancelable<this>()(this);

private updateBlockItems = debounce((): void => {
this.updateGroupItems();
const { dragEnabled, el, moveToItems } = this;

const items = Array.from(this.el.querySelectorAll(blockSelector));

items.forEach((item) => {
if (item.closest(blockGroupSelector) === el) {
item.moveToItems = moveToItems.filter(
(moveToItem) => moveToItem.element !== el && !item.contains(moveToItem.element),
);
item.dragHandle = dragEnabled;
}
});

this.setUpSorting();
}, DEBOUNCE.nextTick);
private updateBlockItemsDebounced = debounce(this.updateBlockItems, DEBOUNCE.nextTick);

// #endregion

Expand Down Expand Up @@ -136,17 +120,6 @@ export class BlockGroup extends LitElement implements InteractiveComponent, Sort
focusFirstTabbable(this.el);
}

/**
* Emits a `calciteBlockGroupMoveHalt` event.
*
* @private
* @param dragDetail
*/
@method()
putFailed(dragDetail: BlockDragDetail): void {
this.calciteBlockGroupMoveHalt.emit(dragDetail);
}

// #endregion

// #region Events
Expand All @@ -160,7 +133,11 @@ export class BlockGroup extends LitElement implements InteractiveComponent, Sort
/** Fires when the component's item order changes. */
calciteBlockGroupOrderChange = createEvent<BlockDragDetail>({ cancelable: false });

/** Fires when a user attempts to move an element using the sort menu and 'canPut' or 'canPull' returns falsy. */
/**
* Fires when a user attempts to move an element using the sort menu and 'canPut' or 'canPull' returns falsy.
*
* @deprecated No longer necessary.
*/
Comment on lines +136 to +140
Copy link
Copy Markdown
Contributor

@eriklharper eriklharper Jul 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is now deprecated, shouldn't the calciteBlockGroupMoveHalt.emit calls still happen until the next breaking change? Same for the calciteListMoveHalt event.

Copy link
Copy Markdown
Member Author

@driskull driskull Jul 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, because there's no longer a reason to emit a halt since we prevent the invalid move now.

Copy link
Copy Markdown
Contributor

@eriklharper eriklharper Jul 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its an internal event then? We're not expecting anyone to be listening on it? If so, looks good.

Copy link
Copy Markdown
Member Author

@driskull driskull Jul 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, its not internal, its just that its no longer necessary. There's no need for it anymore since we don't allow clicking on items that will be halted.

calciteBlockGroupMoveHalt = createEvent<BlockDragDetail>({ cancelable: false });

// #endregion
Expand All @@ -176,22 +153,23 @@ export class BlockGroup extends LitElement implements InteractiveComponent, Sort
);
this.listen("calciteSortHandleReorder", this.handleSortReorder);
this.listen("calciteSortHandleMove", this.handleSortMove);
this.listen("calciteInternalBlockUpdateMoveToItems", this.handleUpdateMoveToItems);
}

override connectedCallback(): void {
this.connectObserver();
this.updateBlockItems();
this.updateBlockItemsDebounced();
this.setUpSorting();
this.setParentBlockGroup();
this.cancelable.add(this.updateBlockItems);
this.cancelable.add(this.updateBlockItemsDebounced);
}

override willUpdate(changes: PropertyValues<this>): void {
if (
changes.has("group") ||
(changes.has("dragEnabled") && (this.hasUpdated || this.dragEnabled !== false))
) {
this.updateBlockItems();
this.updateBlockItemsDebounced();
}
}

Expand All @@ -208,6 +186,24 @@ export class BlockGroup extends LitElement implements InteractiveComponent, Sort

// #region Private Methods

private updateBlockItems(): void {
this.updateGroupItems();
const { dragEnabled, el, moveToItems } = this;

const items = Array.from(this.el.querySelectorAll(blockSelector));

items.forEach((item) => {
if (item.closest(blockGroupSelector) === el) {
item.moveToItems = moveToItems.filter(
(moveToItem) => moveToItem.element !== el && !item.contains(moveToItem.element),
);
item.dragHandle = dragEnabled;
}
});

this.setUpSorting();
}

private updateGroupItems(): void {
const { el, group } = this;

Expand All @@ -231,6 +227,28 @@ export class BlockGroup extends LitElement implements InteractiveComponent, Sort
event.stopPropagation();
}

private async handleUpdateMoveToItems(event: CustomEvent): Promise<void> {
event.stopPropagation();

const fromEl = this.el;
const fromElItems = Array.from(fromEl.children).filter(isBlock);
const item = event.target as Block["el"];

await fromEl.componentOnReady();
await item.componentOnReady();
this.updateBlockItems();

item.moveToItems = item.moveToItems.filter((moveToItem) =>
this.validateMove({
fromEl,
toEl: moveToItem.element as BlockGroup["el"],
dragEl: item,
newIndex: 0,
oldIndex: fromElItems.indexOf(item),
}),
);
}

private handleSortReorder(event: CustomEvent<ReorderEventDetail>): void {
if (this.parentBlockGroupEl || event.defaultPrevented) {
return;
Expand Down Expand Up @@ -286,7 +304,7 @@ export class BlockGroup extends LitElement implements InteractiveComponent, Sort

onDragSort(detail: BlockDragDetail): void {
this.setParentBlockGroup();
this.updateBlockItems();
this.updateBlockItemsDebounced();

this.calciteBlockGroupOrderChange.emit(detail);
}
Expand All @@ -299,18 +317,21 @@ export class BlockGroup extends LitElement implements InteractiveComponent, Sort
updateBlockChildren(event.target as HTMLSlotElement);
}

private handleMove(event: CustomEvent<MoveEventDetail>): void {
const { moveTo } = event.detail;

const dragEl = event.target as Block["el"];
const fromEl = dragEl?.parentElement as BlockGroup["el"];
const toEl = moveTo.element as BlockGroup["el"];
const fromElItems = Array.from(fromEl.children).filter(isBlock);
const oldIndex = fromElItems.indexOf(dragEl);
const newIndex = 0;

if (!fromEl) {
return;
private validateMove({
fromEl,
toEl,
dragEl,
newIndex,
oldIndex,
}: {
fromEl?: BlockGroup["el"];
toEl?: BlockGroup["el"];
dragEl: Block["el"];
newIndex: number;
oldIndex: number;
}): boolean {
if (!fromEl || !toEl) {
return false;
}

if (
Expand All @@ -322,8 +343,7 @@ export class BlockGroup extends LitElement implements InteractiveComponent, Sort
oldIndex,
}) === false
) {
this.calciteBlockGroupMoveHalt.emit({ toEl, fromEl, dragEl, oldIndex, newIndex });
return;
return false;
}

if (
Expand All @@ -335,7 +355,23 @@ export class BlockGroup extends LitElement implements InteractiveComponent, Sort
oldIndex,
}) === false
) {
toEl.putFailed({ toEl, fromEl, dragEl, oldIndex, newIndex });
return false;
}

return true;
}

private handleMove(event: CustomEvent<MoveEventDetail>): void {
const { moveTo } = event.detail;

const dragEl = event.target as Block["el"];
const fromEl = dragEl?.parentElement as BlockGroup["el"];
const toEl = moveTo.element as BlockGroup["el"];
const fromElItems = Array.from(fromEl.children).filter(isBlock);
const oldIndex = fromElItems.indexOf(dragEl);
const newIndex = 0;

if (!this.validateMove({ fromEl, toEl, dragEl, newIndex, oldIndex })) {
return;
}

Expand All @@ -345,7 +381,7 @@ export class BlockGroup extends LitElement implements InteractiveComponent, Sort

toEl.prepend(dragEl);

this.updateBlockItems();
this.updateBlockItemsDebounced();
this.connectObserver();

this.calciteBlockGroupOrderChange.emit({
Expand Down Expand Up @@ -399,7 +435,7 @@ export class BlockGroup extends LitElement implements InteractiveComponent, Sort

parentEl.insertBefore(dragEl, referenceEl);

this.updateBlockItems();
this.updateBlockItemsDebounced();
this.connectObserver();

this.calciteBlockGroupOrderChange.emit({
Expand Down
7 changes: 7 additions & 0 deletions packages/calcite-components/src/components/block/block.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,12 @@ export class Block extends LitElement implements InteractiveComponent, OpenClose

//#region Events

/**
*
* @private
*/
calciteInternalBlockUpdateMoveToItems = createEvent({ cancelable: false });

/** Fires when the component is requested to be closed and before the closing transition begins. */
calciteBlockBeforeClose = createEvent({ cancelable: false });

Expand Down Expand Up @@ -313,6 +319,7 @@ export class Block extends LitElement implements InteractiveComponent, OpenClose
private handleSortHandleBeforeOpen(event: CustomEvent<void>): void {
event.stopPropagation();
this.calciteBlockSortHandleBeforeOpen.emit();
this.calciteInternalBlockUpdateMoveToItems.emit();
}

private handleSortHandleBeforeClose(event: CustomEvent<void>): void {
Expand Down
7 changes: 7 additions & 0 deletions packages/calcite-components/src/components/list-item/list-item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,12 @@ export class ListItem extends LitElement implements InteractiveComponent, Sortab
*/
calciteInternalListItemToggle = createEvent({ cancelable: false });

/**
*
* @private
*/
calciteInternalListItemUpdateMoveToItems = createEvent({ cancelable: false });

/** Fires when the close button is clicked. */
calciteListItemClose = createEvent({ cancelable: false });

Expand Down Expand Up @@ -455,6 +461,7 @@ export class ListItem extends LitElement implements InteractiveComponent, Sortab
private handleSortHandleBeforeOpen(event: CustomEvent<void>): void {
event.stopPropagation();
this.calciteListItemSortHandleBeforeOpen.emit();
this.calciteInternalListItemUpdateMoveToItems.emit();
}

private handleSortHandleBeforeClose(event: CustomEvent<void>): void {
Expand Down
Loading
Loading