fix(list, sortable-list, value-list): Emit calciteListOrderChange whe…

…n dragging between lists (#7614)

**Related Issue:** #7046

## Summary

TLDR: Basically, we need to pause draggable component
connected/disconnected lifecycle events while a component is being
dragged. Having these lifecycle methods kick off during a drag causes
SortableJS errors.

- SortableComponent
- Refactors logic to prevent any SortableJS component from doing its
lifecycle logic when a component is being dragged
- Previously, it was preventing all SortableJS components Sortable from
being destroyed or created but that was causing issues by not emitting
events when an item was moved from one list to another.
- The nested component check that was previously being used isn't ideal
because two different lists don't have to be nested to drag items
between each other.
- We need all lists to still continue emitting events when necessary, we
just don't want their lifecycle methods to kick off when an item is
being dragged. Otherwise, JS errors are thrown.
- Components
- Updates SortableComponent components to not do any lifecycle callbacks
when an item is being dragged to prevent any JS errors that SortableJS
was throwing.
- This was because in connectedCallback, sortable components were
setting up the sortable instance, connecting the observer, modifying
items, etc. We don't want the component to do this while an item is
being dragged.
  - The same thing as above was happening on disconnectedCallback.
- This fix stops all those errors that occurred while dragging an item
from one list to another.

## Assumptions

It is reasonable to not do any lifecycle events for any draggable
component while a component is being dragged

packages/calcite-components/src/components/list-item/list-item.tsx

@@ -236,7 +236,7 @@ export class ListItem
 
   @Listen("calciteInternalListItemGroupDefaultSlotChange")
   @Listen("calciteInternalListDefaultSlotChange")
-  handleCalciteInternalListDefaultSlotChanges(event: CustomEvent): void {
+  handleCalciteInternalListDefaultSlotChanges(event: CustomEvent<void>): void {
     event.stopPropagation();
     this.handleOpenableChange(this.defaultSlotEl);
   }

packages/calcite-components/src/components/list/list.e2e.ts

@@ -5,7 +5,7 @@ import { E2EPage, newE2EPage } from "@stencil/core/testing";
 import { debounceTimeout } from "./resources";
 import { CSS } from "../list-item/resources";
 import { DEBOUNCE_TIMEOUT as FILTER_DEBOUNCE_TIMEOUT } from "../filter/resources";
-import { dragAndDrop, isElementFocused } from "../../tests/utils";
+import { GlobalTestProps, dragAndDrop, isElementFocused } from "../../tests/utils";
 
 const placeholder = placeholderImage({
   width: 140,
@@ -472,9 +472,21 @@ describe("calcite-list", () => {
       return page;
     }
 
+    type TestWindow = GlobalTestProps<{
+      calledTimes: number;
+    }>;
+
     it("works using a mouse", async () => {
       const page = await createSimpleList();
 
+      // 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.$eval("calcite-list", (list: HTMLCalciteListElement) => {
+        (window as TestWindow).calledTimes = 0;
+        list.addEventListener("calciteListOrderChange", () => {
+          (window as TestWindow).calledTimes++;
+        });
+      });
+
       await dragAndDrop(
         page,
         {
@@ -490,6 +502,9 @@ describe("calcite-list", () => {
       const [first, second] = await page.findAll("calcite-list-item");
       expect(await first.getProperty("value")).toBe("two");
       expect(await second.getProperty("value")).toBe("one");
+      await page.waitForChanges();
+
+      expect(await page.evaluate(() => (window as TestWindow).calledTimes)).toBe(1);
     });
 
     it("supports dragging items between lists", async () => {
@@ -517,6 +532,19 @@ describe("calcite-list", () => {
         </calcite-list>
       `);
 
+      await page.waitForChanges();
+
+      // 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(() => {
+        (window as TestWindow).calledTimes = 0;
+        const lists = document.querySelectorAll("calcite-list");
+        lists.forEach((list) =>
+          list.addEventListener("calciteListOrderChange", () => {
+            (window as TestWindow).calledTimes++;
+          })
+        );
+      });
+
       await dragAndDrop(
         page,
         {
@@ -571,6 +599,8 @@ describe("calcite-list", () => {
       expect(await seventh.getProperty("value")).toBe("c");
       expect(await eight.getProperty("value")).toBe("e");
       expect(await ninth.getProperty("value")).toBe("f");
+
+      expect(await page.evaluate(() => (window as TestWindow).calledTimes)).toBe(2);
     });
 
     it("works using a keyboard", async () => {
@@ -586,7 +616,7 @@ describe("calcite-list", () => {
 
       let totalMoves = 0;
 
-      const listOrderChangeSpy = await page.spyOnEvent("calciteListOrderChange");
+      const eventSpy = await page.spyOnEvent("calciteListOrderChange");
 
       async function assertKeyboardMove(
         arrowKey: "ArrowDown" | "ArrowUp",
@@ -603,7 +633,7 @@ describe("calcite-list", () => {
           expect(await itemsAfter[i].getProperty("value")).toBe(expectedValueOrder[i]);
         }
 
-        expect(listOrderChangeSpy).toHaveReceivedEventTimes(++totalMoves);
+        expect(eventSpy).toHaveReceivedEventTimes(++totalMoves);
       }
 
       await assertKeyboardMove("ArrowDown", ["two", "one", "three"]);

packages/calcite-components/src/components/list/list.tsx

@@ -11,7 +11,7 @@ import {
   VNode,
   Watch,
 } from "@stencil/core";
-import Sortable, { SortableEvent } from "sortablejs";
+import Sortable from "sortablejs";
 import { debounce } from "lodash-es";
 import { slotChangeHasAssignedElement, toAriaBoolean } from "../../utils/dom";
 import {
@@ -27,10 +27,11 @@ import { MAX_COLUMNS } from "../list-item/resources";
 import { getListItemChildren, updateListItemChildren } from "../list-item/utils";
 import { CSS, debounceTimeout, SelectionAppearance, SLOTS } from "./resources";
 import {
-  DragEvent,
+  DragDetail,
   connectSortableComponent,
   disconnectSortableComponent,
   SortableComponent,
+  dragActive,
 } from "../../utils/sortableComponent";
 import { SLOTS as STACK_SLOTS } from "../stack/resources";
 
@@ -73,12 +74,12 @@ export class List implements InteractiveComponent, LoadableComponent, SortableCo
   /**
    * When provided, the method will be called to determine whether the element can  move from the list.
    */
-  @Prop() canPull: (event: DragEvent) => boolean;
+  @Prop() canPull: (detail: DragDetail) => boolean;
 
   /**
    * When provided, the method will be called to determine whether the element can be added from another list.
    */
-  @Prop() canPut: (event: DragEvent) => boolean;
+  @Prop() canPut: (detail: DragDetail) => boolean;
 
   /**
    * When `true`, `calcite-list-item`s are sortable via a draggable button.
@@ -191,12 +192,12 @@ export class List implements InteractiveComponent, LoadableComponent, SortableCo
   /**
    * Emitted when the order of the list has changed.
    */
-  @Event({ cancelable: false }) calciteListOrderChange: EventEmitter<DragEvent>;
+  @Event({ cancelable: false }) calciteListOrderChange: EventEmitter<DragDetail>;
 
   /**
    * Emitted when the default slot has changes in order to notify parent lists.
    */
-  @Event({ cancelable: false }) calciteInternalListDefaultSlotChange: EventEmitter<DragEvent>;
+  @Event({ cancelable: false }) calciteInternalListDefaultSlotChange: EventEmitter<void>;
 
   @Listen("calciteInternalFocusPreviousItem")
   handleCalciteInternalFocusPreviousItem(event: CustomEvent): void {
@@ -294,14 +295,22 @@ export class List implements InteractiveComponent, LoadableComponent, SortableCo
   //--------------------------------------------------------------------------
 
   connectedCallback(): void {
+    if (dragActive(this)) {
+      return;
+    }
+
     this.connectObserver();
     this.updateListItems();
     this.setUpSorting();
     connectInteractive(this);
-    this.parentListEl = this.el.parentElement.closest("calcite-list");
+    this.setParentList();
   }
 
   disconnectedCallback(): void {
+    if (dragActive(this)) {
+      return;
+    }
+
     this.disconnectObserver();
     disconnectSortableComponent(this);
     disconnectInteractive(this);
@@ -476,16 +485,15 @@ export class List implements InteractiveComponent, LoadableComponent, SortableCo
     this.connectObserver();
   }
 
-  onDragSort(event: SortableEvent): void {
+  onDragSort(detail: DragDetail): void {
+    this.setParentList();
     this.updateListItems();
 
-    const { from, item, to } = event;
+    this.calciteListOrderChange.emit(detail);
+  }
 
-    this.calciteListOrderChange.emit({
-      dragEl: item,
-      fromEl: from,
-      toEl: to,
-    });
+  private setParentList(): void {
+    this.parentListEl = this.el.parentElement?.closest("calcite-list");
   }
 
   private handleDefaultSlotChange = (event: Event): void => {

packages/calcite-components/src/components/sortable-list/sortable-list.tsx

@@ -11,10 +11,11 @@ import { HandleNudge } from "../handle/interfaces";
 import { Layout } from "../interfaces";
 import { CSS } from "./resources";
 import {
-  DragEvent,
+  DragDetail,
   connectSortableComponent,
   disconnectSortableComponent,
   SortableComponent,
+  dragActive,
 } from "../../utils/sortableComponent";
 import { focusElement } from "../../utils/dom";
 
@@ -36,12 +37,12 @@ export class SortableList implements InteractiveComponent, SortableComponent {
   /**
    * When provided, the method will be called to determine whether the element can  move from the list.
    */
-  @Prop() canPull: (event: DragEvent) => boolean;
+  @Prop() canPull: (detail: DragDetail) => boolean;
 
   /**
    * When provided, the method will be called to determine whether the element can be added from another list.
    */
-  @Prop() canPut: (event: DragEvent) => boolean;
+  @Prop() canPut: (detail: DragDetail) => boolean;
 
   /**
    * Specifies which items inside the element should be draggable.
@@ -100,12 +101,20 @@ export class SortableList implements InteractiveComponent, SortableComponent {
   // --------------------------------------------------------------------------
 
   connectedCallback(): void {
+    if (dragActive(this)) {
+      return;
+    }
+
     this.setUpSorting();
     this.beginObserving();
     connectInteractive(this);
   }
 
   disconnectedCallback(): void {
+    if (dragActive(this)) {
+      return;
+    }
+
     disconnectInteractive(this);
     disconnectSortableComponent(this);
     this.endObserving();

packages/calcite-components/src/components/value-list/value-list.tsx

@@ -62,10 +62,11 @@ import { ValueListMessages } from "./assets/value-list/t9n";
 import { CSS, ICON_TYPES } from "./resources";
 import { getHandleAndItemElement, getScreenReaderText } from "./utils";
 import {
-  DragEvent,
+  DragDetail,
   connectSortableComponent,
   disconnectSortableComponent,
   SortableComponent,
+  dragActive,
 } from "../../utils/sortableComponent";
 import { focusElement } from "../../utils/dom";
 
@@ -103,12 +104,12 @@ export class ValueList<
   /**
    * When provided, the method will be called to determine whether the element can  move from the list.
    */
-  @Prop() canPull: (event: DragEvent) => boolean;
+  @Prop() canPull: (detail: DragDetail) => boolean;
 
   /**
    * When provided, the method will be called to determine whether the element can be added from another list.
    */
-  @Prop() canPut: (event: DragEvent) => boolean;
+  @Prop() canPut: (detail: DragDetail) => boolean;
 
   /**
    * When `true`, `calcite-value-list-item`s are sortable via a draggable button.
@@ -238,6 +239,10 @@ export class ValueList<
   // --------------------------------------------------------------------------
 
   connectedCallback(): void {
+    if (dragActive(this)) {
+      return;
+    }
+
     connectInteractive(this);
     connectLocalized(this);
     connectMessages(this);
@@ -261,6 +266,10 @@ export class ValueList<
   }
 
   disconnectedCallback(): void {
+    if (dragActive(this)) {
+      return;
+    }
+
     disconnectInteractive(this);
     disconnectSortableComponent(this);
     disconnectLocalized(this);