fix: fix regression causing open/close events from emitting in proper order

packages/calcite-components/src/utils/dom.spec.ts

@@ -1,4 +1,3 @@
-import { JSDOM } from "jsdom";
 import { ModeName } from "../../src/components/interfaces";
 import { html } from "../../support/formatting";
 import {
@@ -24,6 +23,8 @@ import {
   whenTransitionDone,
 } from "./dom";
 import { guidPattern } from "./guid.spec";
+import { createTransitionEventDispatcher, TransitionEventDispatcher } from "./spec-helpers/transitionEvents";
+import { AnimationEventDispatcher, createAnimationEventDispatcher } from "./spec-helpers/animationEvents";
 
 describe("dom", () => {
   describe("getElementProp()", () => {
@@ -623,39 +624,10 @@ describe("dom", () => {
   }
 
   describe("whenTransitionDone", () => {
-    let dispatchTransitionEvent: (
-      element: HTMLElement,
-      type: "transitionstart" | "transitionend",
-      propertyName: string,
-    ) => void;
+    let dispatchTransitionEvent: TransitionEventDispatcher;
 
     beforeEach(() => {
-      // we clobber Stencil's custom Mock document implementation
-      const { window: win } = new JSDOM();
-
-      // eslint-disable-next-line no-global-assign -- overriding to make window references use JSDOM (which is a subset, hence the type cast)
-      window = win as any as Window & typeof globalThis;
-
-      // we define TransitionEvent since JSDOM doesn't support it yet - https://github.com/jsdom/jsdom/issues/1781
-      class TransitionEvent extends window.Event {
-        elapsedTime: number;
-
-        propertyName: string;
-
-        constructor(type: string, eventInitDict: EventInit & Partial<{ elapsedTime: number; propertyName: string }>) {
-          super(type, eventInitDict);
-          this.elapsedTime = eventInitDict.elapsedTime;
-          this.propertyName = eventInitDict.propertyName;
-        }
-      }
-
-      dispatchTransitionEvent = (
-        element: HTMLElement,
-        type: "transitionstart" | "transitionend",
-        propertyName: string,
-      ): void => {
-        element.dispatchEvent(new TransitionEvent(type, { propertyName }));
-      };
+      dispatchTransitionEvent = createTransitionEventDispatcher();
     });
 
     it("should return a promise that resolves after the transition", async () => {
@@ -689,40 +661,10 @@ describe("dom", () => {
   });
 
   describe("whenAnimationDone", () => {
-    let dispatchAnimationEvent: (
-      element: HTMLElement,
-      type: "animationstart" | "animationend",
-      animationName: string,
-    ) => void;
+    let dispatchAnimationEvent: AnimationEventDispatcher;
 
     beforeEach(() => {
-      // we clobber Stencil's custom Mock document implementation
-      const { window: win } = new JSDOM();
-
-      // eslint-disable-next-line no-global-assign -- overriding to make window references use JSDOM (which is a subset, hence the type cast)
-      window = win as any as Window & typeof globalThis;
-
-      // we define AnimationEvent since JSDOM doesn't support it yet -
-
-      class AnimationEvent extends window.Event {
-        elapsedTime: number;
-
-        animationName: string;
-
-        constructor(type: string, eventInitDict: EventInit & Partial<{ elapsedTime: number; animationName: string }>) {
-          super(type, eventInitDict);
-          this.elapsedTime = eventInitDict.elapsedTime;
-          this.animationName = eventInitDict.animationName;
-        }
-      }
-
-      dispatchAnimationEvent = (
-        element: HTMLElement,
-        type: "animationstart" | "animationend",
-        animationName: string,
-      ): void => {
-        element.dispatchEvent(new AnimationEvent(type, { animationName }));
-      };
+      dispatchAnimationEvent = createAnimationEventDispatcher();
     });
 
     it("should return a promise that resolves after the animation", async () => {

packages/calcite-components/src/utils/dom.ts

@@ -659,19 +659,33 @@ export function isBefore(a: HTMLElement, b: HTMLElement): boolean {
  *
  * @param targetEl The element to watch for the animation to complete.
  * @param animationName The name of the animation to watch for completion.
+ * @param onStart A callback to run when the animation starts.
+ * @param onEnd A callback to run when the animation ends or is canceled.
  */
-export async function whenAnimationDone(targetEl: HTMLElement, animationName: string): Promise<void> {
-  return whenTransitionOrAnimationDone(targetEl, animationName, "animation");
+export async function whenAnimationDone(
+  targetEl: HTMLElement,
+  animationName: string,
+  onStart?: () => void,
+  onEnd?: () => void,
+): Promise<void> {
+  return whenTransitionOrAnimationDone(targetEl, animationName, "animation", onStart, onEnd);
 }
 
 /**
  * This util helps determine when a transition has completed.
  *
  * @param targetEl The element to watch for the transition to complete.
  * @param transitionProp The name of the transition to watch for completion.
+ * @param onStart A callback to run when the transition starts.
+ * @param onEnd A callback to run when the transition ends or is canceled.
  */
-export async function whenTransitionDone(targetEl: HTMLElement, transitionProp: string): Promise<void> {
-  return whenTransitionOrAnimationDone(targetEl, transitionProp, "transition");
+export async function whenTransitionDone(
+  targetEl: HTMLElement,
+  transitionProp: string,
+  onStart?: () => void,
+  onEnd?: () => void,
+): Promise<void> {
+  return whenTransitionOrAnimationDone(targetEl, transitionProp, "transition", onStart, onEnd);
 }
 
 type TransitionOrAnimation = "transition" | "animation";
@@ -683,11 +697,15 @@ type TransitionOrAnimationEvent = TransitionEvent | AnimationEvent;
  * @param targetEl The element to watch for the transition or animation to complete.
  * @param transitionPropOrAnimationName The transition or animation property to watch for completion.
  * @param type The type of property to watch for completion. Defaults to "transition".
+ * @param onStart A callback to run when the transition or animation starts.
+ * @param onEnd A callback to run when the transition or animation ends or is canceled.
  */
 export async function whenTransitionOrAnimationDone(
   targetEl: HTMLElement,
   transitionPropOrAnimationName: string,
   type: TransitionOrAnimation,
+  onStart?: () => void,
+  onEnd?: () => void,
 ): Promise<void> {
   const style = window.getComputedStyle(targetEl);
   const allDurations = type === "transition" ? style.transitionDuration : style.animationDuration;
@@ -702,8 +720,14 @@ export async function whenTransitionOrAnimationDone(
             so we fall back to it if there's no matching prop duration */
     allDurationsArray[0];
 
+  function startEndImmediately(): void {
+    onStart?.();
+    onEnd?.();
+  }
+
   if (duration === "0s") {
-    return Promise.resolve();
+    startEndImmediately();
+    return;
   }
 
   const startEvent = type === "transition" ? "transitionstart" : "animationstart";
@@ -713,29 +737,32 @@ export async function whenTransitionOrAnimationDone(
   return new Promise<void>((resolve) => {
     const fallbackTimeoutId = window.setTimeout(
       (): void => {
-        targetEl.removeEventListener(startEvent, onStart);
-        targetEl.removeEventListener(endEvent, onEndOrCancel);
-        targetEl.removeEventListener(cancelEvent, onEndOrCancel);
+        targetEl.removeEventListener(startEvent, onTransitionOrAnimationStart);
+        targetEl.removeEventListener(endEvent, onTransitionOrAnimationEndOrCancel);
+        targetEl.removeEventListener(cancelEvent, onTransitionOrAnimationEndOrCancel);
+        startEndImmediately();
         resolve();
       },
       parseFloat(duration) * 1000,
     );
 
-    targetEl.addEventListener(startEvent, onStart);
-    targetEl.addEventListener(endEvent, onEndOrCancel);
-    targetEl.addEventListener(cancelEvent, onEndOrCancel);
+    targetEl.addEventListener(startEvent, onTransitionOrAnimationStart);
+    targetEl.addEventListener(endEvent, onTransitionOrAnimationEndOrCancel);
+    targetEl.addEventListener(cancelEvent, onTransitionOrAnimationEndOrCancel);
 
-    function onStart(event: TransitionOrAnimationEvent): void {
+    function onTransitionOrAnimationStart(event: TransitionOrAnimationEvent): void {
       if (event.target === targetEl && getTransitionOrAnimationName(event) === transitionPropOrAnimationName) {
         window.clearTimeout(fallbackTimeoutId);
-        targetEl.removeEventListener(startEvent, onStart);
+        targetEl.removeEventListener(startEvent, onTransitionOrAnimationStart);
+        onStart?.();
       }
     }
 
-    function onEndOrCancel(event: TransitionOrAnimationEvent): void {
+    function onTransitionOrAnimationEndOrCancel(event: TransitionOrAnimationEvent): void {
       if (event.target === targetEl && getTransitionOrAnimationName(event) === transitionPropOrAnimationName) {
-        targetEl.removeEventListener(endEvent, onEndOrCancel);
-        targetEl.removeEventListener(cancelEvent, onEndOrCancel);
+        targetEl.removeEventListener(endEvent, onTransitionOrAnimationEndOrCancel);
+        targetEl.removeEventListener(cancelEvent, onTransitionOrAnimationEndOrCancel);
+        onEnd?.();
         resolve();
       }
     }

packages/calcite-components/src/utils/openCloseComponent.spec.ts

@@ -0,0 +1,68 @@
+import * as openCloseComponent from "./openCloseComponent";
+import { createTransitionEventDispatcher, TransitionEventDispatcher } from "./spec-helpers/transitionEvents";
+
+const { onToggleOpenCloseComponent } = openCloseComponent;
+
+describe("openCloseComponent", () => {
+  describe("toggleOpenCloseComponent", () => {
+    let dispatchTransitionEvent: TransitionEventDispatcher;
+
+    beforeEach(() => {
+      jest.spyOn(openCloseComponent, "internalReadTask").mockImplementation((task) => task(1337));
+      dispatchTransitionEvent = createTransitionEventDispatcher();
+    });
+
+    afterEach(() => {
+      jest.resetAllMocks();
+    });
+
+    it("emits beforeOpen/beforeClose events when the transition starts and open/close events when the transition is done", async () => {
+      const transitionEl = window.document.createElement("div");
+      const testProp = "opacity";
+      const testDuration = "0.5s";
+      const testTransition = `${testProp} ${testDuration} ease 0s`;
+
+      transitionEl.style.transition = testTransition;
+
+      // need to mock due to JSDOM issue with getComputedStyle - https://github.com/jsdom/jsdom/issues/3090
+      window.getComputedStyle = jest.fn().mockReturnValue({
+        transition: testTransition,
+        transitionDuration: testDuration,
+        transitionProperty: testProp,
+      });
+      window.document.body.append(transitionEl);
+
+      const emittedEvents: string[] = [];
+      const fakeOpenCloseComponent = {
+        el: document.createElement("div"),
+        open: true,
+        openTransitionProp: "opacity",
+        transitionEl,
+        onBeforeOpen: jest.fn(() => emittedEvents.push("beforeOpen")),
+        onOpen: jest.fn(() => emittedEvents.push("open")),
+        onBeforeClose: jest.fn(() => emittedEvents.push("beforeClose")),
+        onClose: jest.fn(() => emittedEvents.push("close")),
+      };
+
+      onToggleOpenCloseComponent(fakeOpenCloseComponent);
+      expect(emittedEvents).toEqual([]);
+
+      dispatchTransitionEvent(transitionEl, "transitionstart", fakeOpenCloseComponent.openTransitionProp);
+      expect(emittedEvents).toEqual(["beforeOpen"]);
+
+      dispatchTransitionEvent(transitionEl, "transitionend", fakeOpenCloseComponent.openTransitionProp);
+      expect(emittedEvents).toEqual(["beforeOpen", "open"]);
+
+      fakeOpenCloseComponent.open = false;
+
+      onToggleOpenCloseComponent(fakeOpenCloseComponent);
+      expect(emittedEvents).toEqual(["beforeOpen", "open"]);
+
+      dispatchTransitionEvent(transitionEl, "transitionstart", fakeOpenCloseComponent.openTransitionProp);
+      expect(emittedEvents).toEqual(["beforeOpen", "open", "beforeClose"]);
+
+      dispatchTransitionEvent(transitionEl, "transitionend", fakeOpenCloseComponent.openTransitionProp);
+      expect(emittedEvents).toEqual(["beforeOpen", "open", "beforeClose", "close"]);
+    });
+  });
+});

packages/calcite-components/src/utils/openCloseComponent.ts

@@ -1,6 +1,11 @@
 import { readTask } from "@stencil/core";
 import { whenTransitionDone } from "./dom";
 
+/**
+ * Exported for testing purposes only
+ */
+export const internalReadTask = readTask;
+
 /**
  * Defines interface for components with open/close public emitter.
  * All implementations of this interface must handle the following events: `beforeOpen`, `open`, `beforeClose`, `close`.
@@ -81,20 +86,28 @@ function isOpen(component: OpenCloseComponent): boolean {
  * @param component - OpenCloseComponent uses `open` prop to emit (before)open/close.
  */
 export function onToggleOpenCloseComponent(component: OpenCloseComponent): void {
-  readTask(async (): Promise<void> => {
+  internalReadTask((): void => {
     if (!component.transitionEl) {
       return;
     }
 
-    await whenTransitionDone(component.transitionEl, component.openTransitionProp);
-
-    if (isOpen(component)) {
-      component.onBeforeOpen();
-      component.onOpen();
-      return;
-    }
-
-    component.onBeforeClose();
-    component.onClose();
+    whenTransitionDone(
+      component.transitionEl,
+      component.openTransitionProp,
+      () => {
+        if (isOpen(component)) {
+          component.onBeforeOpen();
+        } else {
+          component.onBeforeClose();
+        }
+      },
+      () => {
+        if (isOpen(component)) {
+          component.onOpen();
+        } else {
+          component.onClose();
+        }
+      },
+    );
   });
 }

packages/calcite-components/src/utils/spec-helpers/animationEvents.ts

@@ -0,0 +1,33 @@
+import { JSDOM } from "jsdom";
+
+export interface AnimationEventDispatcher {
+  (element: HTMLElement, type: "animationstart" | "animationend", animationName: string): void;
+}
+
+/**
+ * Must be called in a `beforeEach` block to create a animation event dispatcher.
+ */
+export function createAnimationEventDispatcher(): AnimationEventDispatcher {
+  // we clobber Stencil's custom Mock document implementation
+  const { window: win } = new JSDOM();
+
+  // eslint-disable-next-line no-global-assign -- overriding to make window references use JSDOM (which is a subset, hence the type cast)
+  window = win as any as Window & typeof globalThis;
+
+  // we define AnimationEvent since JSDOM doesn't support it yet -
+  class AnimationEvent extends window.Event {
+    elapsedTime: number;
+
+    animationName: string;
+
+    constructor(type: string, eventInitDict: EventInit & Partial<{ elapsedTime: number; animationName: string }>) {
+      super(type, eventInitDict);
+      this.elapsedTime = eventInitDict.elapsedTime;
+      this.animationName = eventInitDict.animationName;
+    }
+  }
+
+  return (element: HTMLElement, type: "animationstart" | "animationend", animationName: string): void => {
+    element.dispatchEvent(new AnimationEvent(type, { animationName }));
+  };
+}