From 12da9b7a6091582d034cc3944d5f6c4437828cc6 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Tue, 14 Jan 2025 16:44:17 -0800 Subject: [PATCH 1/4] fix: fix eventing of initially open components * use Web Animations API for better transition/animation handling --- .../src/components/alert/alert.tsx | 3 - .../components/autocomplete/autocomplete.tsx | 11 - .../src/components/block/block.tsx | 4 - .../src/components/button/button.spec.ts | 0 .../src/components/combobox/combobox.tsx | 6 - .../src/components/dropdown/dropdown.tsx | 4 - .../input-date-picker/input-date-picker.tsx | 4 - .../src/components/notice/notice.tsx | 3 - .../src/components/popover/popover.tsx | 3 - .../src/components/tooltip/tooltip.tsx | 9 - .../calcite-components/src/demos/flow.html | 4 + .../src/tests/spec-helpers/animationEvents.ts | 24 -- .../src/tests/spec-helpers/computedStyle.ts | 17 - .../tests/spec-helpers/transitionEvents.ts | 24 -- .../calcite-components/src/utils/dom.spec.ts | 291 ++++++------------ packages/calcite-components/src/utils/dom.ts | 118 ++++--- .../src/utils/openCloseComponent.spec.ts | 55 ++-- .../src/utils/openCloseComponent.ts | 18 +- 18 files changed, 194 insertions(+), 404 deletions(-) create mode 100644 packages/calcite-components/src/components/button/button.spec.ts delete mode 100644 packages/calcite-components/src/tests/spec-helpers/animationEvents.ts delete mode 100644 packages/calcite-components/src/tests/spec-helpers/computedStyle.ts delete mode 100644 packages/calcite-components/src/tests/spec-helpers/transitionEvents.ts diff --git a/packages/calcite-components/src/components/alert/alert.tsx b/packages/calcite-components/src/components/alert/alert.tsx index ce8b1a5e58c..f543a7f2f68 100644 --- a/packages/calcite-components/src/components/alert/alert.tsx +++ b/packages/calcite-components/src/components/alert/alert.tsx @@ -219,9 +219,6 @@ export class Alert extends LitElement implements OpenCloseComponent, LoadableCom async load(): Promise { setUpLoadableComponent(this); - if (this.open) { - onToggleOpenCloseComponent(this); - } } override willUpdate(changes: PropertyValues): void { diff --git a/packages/calcite-components/src/components/autocomplete/autocomplete.tsx b/packages/calcite-components/src/components/autocomplete/autocomplete.tsx index 54a1a5d0b70..96b2a3ef806 100644 --- a/packages/calcite-components/src/components/autocomplete/autocomplete.tsx +++ b/packages/calcite-components/src/components/autocomplete/autocomplete.tsx @@ -421,24 +421,13 @@ export class Autocomplete connectLabel(this); connectForm(this); this.defaultInputValue = this.inputValue || ""; - this.getAllItemsDebounced(); - - if (this.open) { - this.openHandler(); - onToggleOpenCloseComponent(this); - } - connectFloatingUI(this); } async load(): Promise { setUpLoadableComponent(this); this.getAllItemsDebounced(); - - if (this.open) { - onToggleOpenCloseComponent(this); - } } override willUpdate(changes: PropertyValues): void { diff --git a/packages/calcite-components/src/components/block/block.tsx b/packages/calcite-components/src/components/block/block.tsx index ed8c643d506..faf5009993d 100644 --- a/packages/calcite-components/src/components/block/block.tsx +++ b/packages/calcite-components/src/components/block/block.tsx @@ -195,10 +195,6 @@ export class Block load(): void { setUpLoadableComponent(this); - if (this.open) { - onToggleOpenCloseComponent(this); - } - if (!this.heading && !this.label) { logger.warn( `${this.el.tagName} is missing both heading & label. Please provide a heading or label for the component to be accessible.`, diff --git a/packages/calcite-components/src/components/button/button.spec.ts b/packages/calcite-components/src/components/button/button.spec.ts new file mode 100644 index 00000000000..e69de29bb2d diff --git a/packages/calcite-components/src/components/combobox/combobox.tsx b/packages/calcite-components/src/components/combobox/combobox.tsx index 6b235bf7f5b..07641062ead 100644 --- a/packages/calcite-components/src/components/combobox/combobox.tsx +++ b/packages/calcite-components/src/components/combobox/combobox.tsx @@ -556,12 +556,6 @@ export class Combobox this.mutationObserver?.observe(this.el, { childList: true, subtree: true }); this.setFilteredPlacements(); - - if (this.open) { - this.openHandler(); - onToggleOpenCloseComponent(this); - } - connectFloatingUI(this); } diff --git a/packages/calcite-components/src/components/dropdown/dropdown.tsx b/packages/calcite-components/src/components/dropdown/dropdown.tsx index b028ca0fd41..875d1a806e7 100644 --- a/packages/calcite-components/src/components/dropdown/dropdown.tsx +++ b/packages/calcite-components/src/components/dropdown/dropdown.tsx @@ -236,10 +236,6 @@ export class Dropdown override connectedCallback(): void { this.mutationObserver?.observe(this.el, { childList: true, subtree: true }); this.setFilteredPlacements(); - if (this.open) { - this.openHandler(); - onToggleOpenCloseComponent(this); - } this.updateItems(); connectFloatingUI(this); } diff --git a/packages/calcite-components/src/components/input-date-picker/input-date-picker.tsx b/packages/calcite-components/src/components/input-date-picker/input-date-picker.tsx index bfee415bc7d..4a374b7fed4 100644 --- a/packages/calcite-components/src/components/input-date-picker/input-date-picker.tsx +++ b/packages/calcite-components/src/components/input-date-picker/input-date-picker.tsx @@ -435,10 +435,6 @@ export class InputDatePicker useGrouping: false, }; - if (this.open) { - onToggleOpenCloseComponent(this); - } - connectFloatingUI(this); } diff --git a/packages/calcite-components/src/components/notice/notice.tsx b/packages/calcite-components/src/components/notice/notice.tsx index 944d2f8025c..3eb9787a535 100644 --- a/packages/calcite-components/src/components/notice/notice.tsx +++ b/packages/calcite-components/src/components/notice/notice.tsx @@ -152,9 +152,6 @@ export class Notice extends LitElement implements LoadableComponent, OpenCloseCo async load(): Promise { setUpLoadableComponent(this); this.requestedIcon = setRequestedIcon(KindIcons, this.icon, this.kind); - if (this.open) { - onToggleOpenCloseComponent(this); - } } override willUpdate(changes: PropertyValues): void { diff --git a/packages/calcite-components/src/components/popover/popover.tsx b/packages/calcite-components/src/components/popover/popover.tsx index 8346af8565e..f15172ecaf0 100644 --- a/packages/calcite-components/src/components/popover/popover.tsx +++ b/packages/calcite-components/src/components/popover/popover.tsx @@ -341,9 +341,6 @@ export class Popover this.setUpReferenceElement(); } - if (this.open) { - onToggleOpenCloseComponent(this); - } this.hasLoaded = true; } diff --git a/packages/calcite-components/src/components/tooltip/tooltip.tsx b/packages/calcite-components/src/components/tooltip/tooltip.tsx index 8176a1d6512..e0c51a6f378 100644 --- a/packages/calcite-components/src/components/tooltip/tooltip.tsx +++ b/packages/calcite-components/src/components/tooltip/tooltip.tsx @@ -174,15 +174,6 @@ export class Tooltip extends LitElement implements FloatingUIComponent, OpenClos override connectedCallback(): void { this.setUpReferenceElement(true); - if (this.open) { - onToggleOpenCloseComponent(this); - } - } - - load(): void { - if (this.open) { - onToggleOpenCloseComponent(this); - } } override willUpdate(changes: PropertyValues): void { diff --git a/packages/calcite-components/src/demos/flow.html b/packages/calcite-components/src/demos/flow.html index 03b40d57cb3..3b432e7ffe5 100644 --- a/packages/calcite-components/src/demos/flow.html +++ b/packages/calcite-components/src/demos/flow.html @@ -27,6 +27,10 @@ text-align: right; flex: 0 0 15%; } + + :root { + --calcite-duration-factor: 1; + } diff --git a/packages/calcite-components/src/tests/spec-helpers/animationEvents.ts b/packages/calcite-components/src/tests/spec-helpers/animationEvents.ts deleted file mode 100644 index 0ac5b13aa23..00000000000 --- a/packages/calcite-components/src/tests/spec-helpers/animationEvents.ts +++ /dev/null @@ -1,24 +0,0 @@ -// @ts-strict-ignore -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 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 })); - }; -} diff --git a/packages/calcite-components/src/tests/spec-helpers/computedStyle.ts b/packages/calcite-components/src/tests/spec-helpers/computedStyle.ts deleted file mode 100644 index 646c7ec4886..00000000000 --- a/packages/calcite-components/src/tests/spec-helpers/computedStyle.ts +++ /dev/null @@ -1,17 +0,0 @@ -// @ts-strict-ignore -import { vi } from "vitest"; - -/** - * Mocks `getComputedStyle` to return the provided values for the provided element. - * This is needed due to JSDOM issue with getComputedStyle - https://github.com/jsdom/jsdom/issues/3090 - * - * @param element - * @param fakeComputedStyle - */ -export function mockGetComputedStyleFor(element: Element, fakeComputedStyle: Partial): void { - vi.spyOn(window, "getComputedStyle").mockImplementation((el: Element): CSSStyleDeclaration => { - if (el === element) { - return fakeComputedStyle as CSSStyleDeclaration; - } - }); -} diff --git a/packages/calcite-components/src/tests/spec-helpers/transitionEvents.ts b/packages/calcite-components/src/tests/spec-helpers/transitionEvents.ts deleted file mode 100644 index 268907504f1..00000000000 --- a/packages/calcite-components/src/tests/spec-helpers/transitionEvents.ts +++ /dev/null @@ -1,24 +0,0 @@ -// @ts-strict-ignore -export interface TransitionEventDispatcher { - (element: HTMLElement, type: "transitionstart" | "transitionend", propertyName: string): void; -} - -/** Must be called in a `beforeEach` block to create a transition event dispatcher. */ -export function createTransitionEventDispatcher(): TransitionEventDispatcher { - // 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; - } - } - - return (element: HTMLElement, type: "transitionstart" | "transitionend", propertyName: string): void => { - element.dispatchEvent(new TransitionEvent(type, { propertyName })); - }; -} diff --git a/packages/calcite-components/src/utils/dom.spec.ts b/packages/calcite-components/src/utils/dom.spec.ts index 3f0abd225d6..517154e77c6 100644 --- a/packages/calcite-components/src/utils/dom.spec.ts +++ b/packages/calcite-components/src/utils/dom.spec.ts @@ -2,9 +2,6 @@ import { beforeAll, beforeEach, describe, expect, it, Mock, vi } from "vitest"; import { ModeName } from "../components/interfaces"; import { html } from "../../support/formatting"; -import { createTransitionEventDispatcher, TransitionEventDispatcher } from "../tests/spec-helpers/transitionEvents"; -import { AnimationEventDispatcher, createAnimationEventDispatcher } from "../tests/spec-helpers/animationEvents"; -import { mockGetComputedStyleFor } from "../tests/spec-helpers/computedStyle"; import { waitForAnimationFrame } from "../tests/utils"; import { guidPattern } from "./guid.spec"; import { @@ -436,205 +433,119 @@ describe("dom", () => { ); } - describe("whenTransitionDone", () => { - const testProp = "opacity"; - const testDuration = "0.5s"; - - let element: HTMLDivElement; - let dispatchTransitionEvent: TransitionEventDispatcher; - let onStartCallback: Mock; - let onEndCallback: Mock; - - beforeEach(() => { - dispatchTransitionEvent = createTransitionEventDispatcher(); - element = window.document.createElement("div"); - onStartCallback = vi.fn(); - onEndCallback = vi.fn(); - }); - - it("should return a promise that resolves after the transition", async () => { - const testTransition = `${testProp} ${testDuration} ease 0s`; - - element.style.transition = testTransition; - window.document.body.append(element); - mockGetComputedStyleFor(element, { - transition: testTransition, - transitionDuration: testDuration, - transitionProperty: testProp, - }); - - const promise = whenTransitionDone(element, testProp, onStartCallback, onEndCallback); - element.style.opacity = "0"; - - expect(await promiseState(promise)).toHaveProperty("status", "pending"); - expect(onStartCallback).not.toHaveBeenCalled(); - expect(onEndCallback).not.toHaveBeenCalled(); - - dispatchTransitionEvent(element, "transitionstart", testProp); - - expect(await promiseState(promise)).toHaveProperty("status", "pending"); - expect(onStartCallback).toHaveBeenCalled(); - expect(onEndCallback).not.toHaveBeenCalled(); - - dispatchTransitionEvent(element, "transitionend", testProp); - - expect(await promiseState(promise)).toHaveProperty("status", "pending"); - expect(onStartCallback).toHaveBeenCalled(); - expect(onEndCallback).toHaveBeenCalled(); - - expect(await promiseState(promise)).toHaveProperty("status", "fulfilled"); - expect(onStartCallback).toHaveBeenCalled(); - expect(onEndCallback).toHaveBeenCalled(); - }); - - it("should return a promise that resolves after 0s transition", async () => { - const testDuration = "0s"; // shadows the outer testDuration - const testTransition = `${testProp} ${testDuration} ease 0s`; - - element.style.transition = testTransition; - window.document.body.append(element); - mockGetComputedStyleFor(element, { - transition: testTransition, - transitionDuration: testDuration, - transitionProperty: testProp, - }); - - const promise = whenTransitionDone(element, testProp, onStartCallback, onEndCallback); - element.style.opacity = "0"; - expect(await promiseState(promise)).toHaveProperty("status", "pending"); - await waitForAnimationFrame(); - expect(onStartCallback).toHaveBeenCalled(); - await waitForAnimationFrame(); - expect(onEndCallback).toHaveBeenCalled(); - - expect(await promiseState(promise)).toHaveProperty("status", "fulfilled"); - }); - - it("should return a promise that resolves when called and transition has not started when expected", async () => { - const testTransition = `${testProp} ${testDuration} ease 0s`; - - element.style.transition = testTransition; - window.document.body.append(element); - mockGetComputedStyleFor(element, { - transition: testTransition, - transitionDuration: testDuration, - transitionProperty: testProp, - }); - - const promise = whenTransitionDone(element, testProp, onStartCallback, onEndCallback); - element.style.opacity = "0"; - expect(await promiseState(promise)).toHaveProperty("status", "pending"); - expect(onStartCallback).not.toHaveBeenCalled(); - expect(onEndCallback).not.toHaveBeenCalled(); - - await new Promise((resolve) => setTimeout(resolve, 500)); - - expect(await promiseState(promise)).toHaveProperty("status", "fulfilled"); - await waitForAnimationFrame(); - expect(onStartCallback).toHaveBeenCalled(); - await waitForAnimationFrame(); - expect(onEndCallback).toHaveBeenCalled(); - }); - }); - - describe("whenAnimationDone", () => { - const testAnimationName = "fade"; - const testDuration = "0.5s"; - + /* + * These tests depend on the `getAnimations` method which is not available in happy-dom, + * so we try to mock it as close to the real thing as possible. + */ + describe("transition/animation helpers", () => { let element: HTMLDivElement; - let dispatchAnimationEvent: AnimationEventDispatcher; let onStartCallback: Mock; let onEndCallback: Mock; beforeEach(() => { - dispatchAnimationEvent = createAnimationEventDispatcher(); element = window.document.createElement("div"); onStartCallback = vi.fn(); onEndCallback = vi.fn(); }); - it("should return a promise that resolves after the animation", async () => { - const testAnimation = `${testAnimationName} ${testDuration} ease 0s`; - - element.style.animation = testAnimation; - window.document.body.append(element); - mockGetComputedStyleFor(element, { - animation: testAnimation, - animationDuration: testDuration, - animationName: testAnimationName, - }); - - const promise = whenAnimationDone(element, testAnimationName, onStartCallback, onEndCallback); - element.style.animationName = "none"; - - expect(await promiseState(promise)).toHaveProperty("status", "pending"); - expect(onStartCallback).not.toHaveBeenCalled(); - expect(onEndCallback).not.toHaveBeenCalled(); - - dispatchAnimationEvent(element, "animationstart", testAnimationName); - - expect(await promiseState(promise)).toHaveProperty("status", "pending"); - expect(onStartCallback).toHaveBeenCalled(); - expect(onEndCallback).not.toHaveBeenCalled(); - - dispatchAnimationEvent(element, "animationend", testAnimationName); - - expect(await promiseState(promise)).toHaveProperty("status", "pending"); - expect(onStartCallback).toHaveBeenCalled(); - expect(onEndCallback).toHaveBeenCalled(); - - expect(await promiseState(promise)).toHaveProperty("status", "fulfilled"); - expect(onStartCallback).toHaveBeenCalled(); - expect(onEndCallback).toHaveBeenCalled(); - }); - - it("should return a promise that resolves after 0s animation", async () => { - const testDuration = "0s"; // shadows the outer testDuration - const testAnimation = `${testAnimationName} ${testDuration} ease 0s`; - - element.style.animation = testAnimation; - window.document.body.append(element); - mockGetComputedStyleFor(element, { - animation: testAnimation, - animationDuration: testDuration, - animationName: testAnimationName, - }); - - const promise = whenAnimationDone(element, testAnimationName, onStartCallback, onEndCallback); - element.style.animationName = "none"; - expect(await promiseState(promise)).toHaveProperty("status", "pending"); - await waitForAnimationFrame(); - expect(onStartCallback).toHaveBeenCalled(); - await waitForAnimationFrame(); - expect(onEndCallback).toHaveBeenCalled(); - - expect(await promiseState(promise)).toHaveProperty("status", "fulfilled"); - }); - - it("should return a promise that resolves when called and animation has not started when expected", async () => { - const testAnimation = `${testAnimationName} ${testDuration} ease 0s`; + function createFinishedPromise(): [Promise, () => void] { + let resolver: () => void; + const finishedPromise = new Promise((resolve) => (resolver = resolve)); + return [finishedPromise, resolver]; + } - element.style.animation = testAnimation; - window.document.body.append(element); - mockGetComputedStyleFor(element, { - animation: testAnimation, - animationDuration: testDuration, - animationName: testAnimationName, + const helpers = [whenTransitionDone, whenAnimationDone] as const; + + helpers.forEach((helper) => { + const type = helper === whenTransitionDone ? "transition" : "animation"; + const animationPropName = helper === whenTransitionDone ? "transitionProperty" : "animationName"; + const testTransitionOrAnimationName = helper === whenTransitionDone ? "opacity" : "fade"; + + describe(`${helper.name}`, () => { + it(`should return a promise that resolves after the ${type} (running at call time)`, async () => { + const [finishedPromise, resolver] = createFinishedPromise(); + const animationsPerCall = [ + [ + { + [animationPropName]: testTransitionOrAnimationName, + finished: finishedPromise, + } as unknown as Animation | CSSTransition, + ], + ]; + element.getAnimations = () => animationsPerCall.shift(); + + const promise = helper(element, testTransitionOrAnimationName, onStartCallback, onEndCallback); + expect(await promiseState(promise)).toHaveProperty("status", "pending"); + expect(onStartCallback).toHaveBeenCalled(); + expect(onEndCallback).not.toHaveBeenCalled(); + + resolver(); + + expect(await promiseState(promise)).toHaveProperty("status", "pending"); + expect(onStartCallback).toHaveBeenCalled(); + expect(onEndCallback).toHaveBeenCalled(); + + expect(await promiseState(promise)).toHaveProperty("status", "fulfilled"); + expect(onStartCallback).toHaveBeenCalled(); + await expect(onEndCallback).toHaveBeenCalled(); + }); + + it(`should return a promise that resolves after the ${type} (running frame after call time)`, async () => { + const [finishedPromise, resolver] = createFinishedPromise(); + const animationsPerCall = [ + [], + [ + { + [animationPropName]: testTransitionOrAnimationName, + finished: finishedPromise, + } as unknown as Animation | CSSTransition, + ], + ]; + element.getAnimations = () => animationsPerCall.shift(); + + const promise = helper(element, testTransitionOrAnimationName, onStartCallback, onEndCallback); + + expect(await promiseState(promise)).toHaveProperty("status", "pending"); + expect(onStartCallback).not.toHaveBeenCalled(); + expect(onEndCallback).not.toHaveBeenCalled(); + + await waitForAnimationFrame(); + + expect(await promiseState(promise)).toHaveProperty("status", "pending"); + expect(onStartCallback).toHaveBeenCalled(); + expect(onEndCallback).not.toHaveBeenCalled(); + + resolver(); + + expect(await promiseState(promise)).toHaveProperty("status", "pending"); + expect(onStartCallback).toHaveBeenCalled(); + expect(onEndCallback).toHaveBeenCalled(); + + expect(await promiseState(promise)).toHaveProperty("status", "fulfilled"); + expect(onStartCallback).toHaveBeenCalled(); + await expect(onEndCallback).toHaveBeenCalled(); + }); + + it(`should return a promise that resolves after 0s ${type} or has not started when expected (fallback cases)`, async () => { + const animationsPerCall = [[], []]; + element.getAnimations = () => animationsPerCall.shift(); + + const promise = helper(element, testTransitionOrAnimationName, onStartCallback, onEndCallback); + expect(await promiseState(promise)).toHaveProperty("status", "pending"); + + await waitForAnimationFrame(); + expect(onStartCallback).not.toHaveBeenCalled(); + expect(onEndCallback).not.toHaveBeenCalled(); + + await waitForAnimationFrame(); + expect(onStartCallback).toHaveBeenCalled(); + + await waitForAnimationFrame(); + expect(onEndCallback).toHaveBeenCalled(); + + expect(await promiseState(promise)).toHaveProperty("status", "fulfilled"); + }); }); - - const promise = whenAnimationDone(element, testAnimationName, onStartCallback, onEndCallback); - element.style.animationName = "none"; - expect(await promiseState(promise)).toHaveProperty("status", "pending"); - expect(onStartCallback).not.toHaveBeenCalled(); - expect(onEndCallback).not.toHaveBeenCalled(); - - await new Promise((resolve) => setTimeout(resolve, 500)); - - expect(await promiseState(promise)).toHaveProperty("status", "fulfilled"); - await waitForAnimationFrame(); - expect(onStartCallback).toHaveBeenCalled(); - await waitForAnimationFrame(); - expect(onEndCallback).toHaveBeenCalled(); }); }); diff --git a/packages/calcite-components/src/utils/dom.ts b/packages/calcite-components/src/utils/dom.ts index 48a0e4ec543..44835558ca2 100644 --- a/packages/calcite-components/src/utils/dom.ts +++ b/packages/calcite-components/src/utils/dom.ts @@ -123,6 +123,7 @@ export function getTextWidth(text: string, font: string): number { context.font = font; return context.measureText(text).width; } + /** * This helper returns the host of a ShadowRoot. * @@ -615,7 +616,34 @@ export async function whenTransitionDone( } type TransitionOrAnimation = "transition" | "animation"; -type TransitionOrAnimationEvent = TransitionEvent | AnimationEvent; +type TransitionOrAnimationInstance = CSSTransition | Animation; + +const activeAnimationsFinishPromises = new WeakMap>(); + +async function triggerFallbackStartEnd(start: () => void, end: () => void): Promise { + // offset callbacks by a frame to simulate event counterparts + await nextFrame(); + start?.(); + + await nextFrame(); + end?.(); +} + +function triggerEndActionAndClearAnim(animation: Animation, end: () => void): void { + activeAnimationsFinishPromises.delete(animation); + end?.(); +} + +function findAnimation( + targetEl: HTMLElement, + type: TransitionOrAnimation, + transitionPropOrAnimationName: string, +): TransitionOrAnimationInstance { + const targetProp = type === "transition" ? "transitionProperty" : "animationName"; + return targetEl + .getAnimations() + .find((anim: Animation | CSSTransition) => anim[targetProp] === transitionPropOrAnimationName); +} /** * This util helps determine when a transition has completed. @@ -633,78 +661,36 @@ export async function whenTransitionOrAnimationDone( onStart?: () => void, onEnd?: () => void, ): Promise { - const style = window.getComputedStyle(targetEl); - const allDurations = type === "transition" ? style.transitionDuration : style.animationDuration; - const allProps = type === "transition" ? style.transitionProperty : style.animationName; - - const allDurationsArray = allDurations.split(","); - const allPropsArray = allProps.split(",").map((prop) => prop.trim()); - const propIndex = allPropsArray.indexOf(transitionPropOrAnimationName); - const duration = - allDurationsArray[propIndex] ?? - /* Safari will have a single duration value for the shorthand prop when multiple, separate names/props are defined, - so we fall back to it if there's no matching prop duration */ - allDurationsArray[0]; - - function triggerFallbackStartEnd(): void { - // offset callbacks by a frame to simulate event counterparts - requestAnimationFrame(() => { - onStart?.(); - - requestAnimationFrame(() => onEnd?.()); - }); - } + let anim = findAnimation(targetEl, type, transitionPropOrAnimationName); - if (duration === "0s") { - triggerFallbackStartEnd(); - return; + if (!anim) { + // we try again in the next frame in case the browser hasn't yet initiated the transition/animation + await nextFrame(); + anim = findAnimation(targetEl, type, transitionPropOrAnimationName); } - const startEvent = type === "transition" ? "transitionstart" : "animationstart"; - const endEvent = type === "transition" ? "transitionend" : "animationend"; - const cancelEvent = type === "transition" ? "transitioncancel" : "animationcancel"; - - return new Promise((resolve) => { - const fallbackTimeoutId = window.setTimeout( - (): void => { - targetEl.removeEventListener(startEvent, onTransitionOrAnimationStart); - targetEl.removeEventListener(endEvent, onTransitionOrAnimationEndOrCancel); - targetEl.removeEventListener(cancelEvent, onTransitionOrAnimationEndOrCancel); - triggerFallbackStartEnd(); - resolve(); - }, - parseFloat(duration) * 1000, - ); - - targetEl.addEventListener(startEvent, onTransitionOrAnimationStart); - targetEl.addEventListener(endEvent, onTransitionOrAnimationEndOrCancel); - targetEl.addEventListener(cancelEvent, onTransitionOrAnimationEndOrCancel); - - function onTransitionOrAnimationStart(event: TransitionOrAnimationEvent): void { - if (event.target === targetEl && getTransitionOrAnimationName(event) === transitionPropOrAnimationName) { - window.clearTimeout(fallbackTimeoutId); - targetEl.removeEventListener(startEvent, onTransitionOrAnimationStart); - onStart?.(); - } - } + if (!anim) { + return triggerFallbackStartEnd(onStart, onEnd); + } - function onTransitionOrAnimationEndOrCancel(event: TransitionOrAnimationEvent): void { - if (event.target === targetEl && getTransitionOrAnimationName(event) === transitionPropOrAnimationName) { - targetEl.removeEventListener(endEvent, onTransitionOrAnimationEndOrCancel); - targetEl.removeEventListener(cancelEvent, onTransitionOrAnimationEndOrCancel); - onEnd?.(); - resolve(); - } - } - }); -} + if (activeAnimationsFinishPromises.has(anim)) { + return activeAnimationsFinishPromises.get(anim); + } -function isTransitionEvent(event: TransitionOrAnimationEvent): event is TransitionEvent { - return "propertyName" in event; + onStart?.(); + activeAnimationsFinishPromises.set( + anim, + anim.finished + .then(() => {}) + .catch(() => { + // swallow error if canceled + }) + .finally(() => triggerEndActionAndClearAnim(anim, onEnd)), + ); } -function getTransitionOrAnimationName(event: TransitionOrAnimationEvent): string { - return isTransitionEvent(event) ? event.propertyName : event.animationName; +function nextFrame(): Promise { + return new Promise((resolve) => requestAnimationFrame(() => resolve())); } /** diff --git a/packages/calcite-components/src/utils/openCloseComponent.spec.ts b/packages/calcite-components/src/utils/openCloseComponent.spec.ts index d3e90d16a7c..0a7b6d89c5c 100644 --- a/packages/calcite-components/src/utils/openCloseComponent.spec.ts +++ b/packages/calcite-components/src/utils/openCloseComponent.spec.ts @@ -1,20 +1,16 @@ -import { describe, expect, it, afterEach, beforeEach, vi } from "vitest"; -import { createTransitionEventDispatcher, TransitionEventDispatcher } from "../tests/spec-helpers/transitionEvents"; -import { mockGetComputedStyleFor } from "../tests/spec-helpers/computedStyle"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { waitForAnimationFrame } from "../tests/utils"; import * as openCloseComponent from "./openCloseComponent"; const { onToggleOpenCloseComponent } = openCloseComponent; describe("openCloseComponent", () => { describe("toggleOpenCloseComponent", () => { - let dispatchTransitionEvent: TransitionEventDispatcher; - beforeEach(() => { vi.spyOn(global, "requestAnimationFrame").mockImplementation((cb) => { cb(0); return 0; }); - dispatchTransitionEvent = createTransitionEventDispatcher(); }); afterEach(() => { @@ -23,18 +19,6 @@ describe("openCloseComponent", () => { 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; - window.document.body.append(transitionEl); - mockGetComputedStyleFor(transitionEl, { - transition: testTransition, - transitionDuration: testDuration, - transitionProperty: testProp, - }); - const emittedEvents: string[] = []; const fakeOpenCloseComponent = { el: document.createElement("div"), @@ -48,24 +32,43 @@ describe("openCloseComponent", () => { onClose: vi.fn(() => emittedEvents.push("close")), }; - onToggleOpenCloseComponent(fakeOpenCloseComponent); - expect(emittedEvents).toEqual([]); + function createFinishedPromise(): [Promise, () => void] { + let resolver: () => void; + const finishedPromise = new Promise((resolve) => (resolver = resolve)); + return [finishedPromise, resolver]; + } + + const [finishedPromise, resolveFinishedPromise] = createFinishedPromise(); - dispatchTransitionEvent(transitionEl, "transitionstart", fakeOpenCloseComponent.transitionProp); + fakeOpenCloseComponent.transitionEl.getAnimations = () => [ + { + transitionProperty: "opacity", + finished: finishedPromise, + } as unknown as CSSTransition, + ]; + + onToggleOpenCloseComponent(fakeOpenCloseComponent); expect(emittedEvents).toEqual(["beforeOpen"]); - dispatchTransitionEvent(transitionEl, "transitionend", fakeOpenCloseComponent.transitionProp); + resolveFinishedPromise(); + await waitForAnimationFrame(); expect(emittedEvents).toEqual(["beforeOpen", "open"]); - fakeOpenCloseComponent.open = false; + fakeOpenCloseComponent.transitionEl.getAnimations = () => [ + { + transitionProperty: "opacity", + finished: finishedPromise, + } as unknown as CSSTransition, + ]; + fakeOpenCloseComponent.open = false; onToggleOpenCloseComponent(fakeOpenCloseComponent); - expect(emittedEvents).toEqual(["beforeOpen", "open"]); - dispatchTransitionEvent(transitionEl, "transitionstart", fakeOpenCloseComponent.transitionProp); expect(emittedEvents).toEqual(["beforeOpen", "open", "beforeClose"]); - dispatchTransitionEvent(transitionEl, "transitionend", fakeOpenCloseComponent.transitionProp); + resolveFinishedPromise(); + await waitForAnimationFrame(); + expect(emittedEvents).toEqual(["beforeOpen", "open", "beforeClose", "close"]); }); }); diff --git a/packages/calcite-components/src/utils/openCloseComponent.ts b/packages/calcite-components/src/utils/openCloseComponent.ts index 64a65bec863..f92daa0f1ea 100644 --- a/packages/calcite-components/src/utils/openCloseComponent.ts +++ b/packages/calcite-components/src/utils/openCloseComponent.ts @@ -41,20 +41,18 @@ function isOpen(component: OpenCloseComponent): boolean { } /** - * Helper to determine globally set transition duration on the given openTransitionProp, which is imported and set in the `@Watch`("open"). - * Used to emit (before)open/close events both for when the opacity transition is present and when there is none (transition-duration is set to 0). + * This util helps emit (before)open/close events consistently based on the associated CSS transition property. + * + * Note: this should be called whenever the component's toggling property changes and would trigger a transition. * * @example * import { onToggleOpenCloseComponent, OpenCloseComponent } from "../../utils/openCloseComponent"; * - * async componentWillLoad() { - * // When component initially renders, if `open` was set we need to trigger on load as watcher doesn't fire. - * if (this.open) { - * onToggleOpenCloseComponent(this); - * } - * @Watch ("open") - * async toggleModal(value: boolean): Promise { - * onToggleOpenCloseComponent(this); + * override willUpdate(changes: PropertyValues): void { + * if (changes.has("open") && (this.hasUpdated || this.open !== false)) { + * onToggleOpenCloseComponent(this); + * } + * // ... * } * @param component - OpenCloseComponent uses `open` prop to emit (before)open/close. */ From a044e8351dfcee059d7b9718bd9a2bb46dcb9b3a Mon Sep 17 00:00:00 2001 From: JC Franco Date: Wed, 15 Jan 2025 00:05:26 -0800 Subject: [PATCH 2/4] fix tests --- packages/calcite-components/src/utils/dom.ts | 28 ++++++-------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/packages/calcite-components/src/utils/dom.ts b/packages/calcite-components/src/utils/dom.ts index 44835558ca2..be510909fea 100644 --- a/packages/calcite-components/src/utils/dom.ts +++ b/packages/calcite-components/src/utils/dom.ts @@ -618,8 +618,6 @@ export async function whenTransitionDone( type TransitionOrAnimation = "transition" | "animation"; type TransitionOrAnimationInstance = CSSTransition | Animation; -const activeAnimationsFinishPromises = new WeakMap>(); - async function triggerFallbackStartEnd(start: () => void, end: () => void): Promise { // offset callbacks by a frame to simulate event counterparts await nextFrame(); @@ -629,11 +627,6 @@ async function triggerFallbackStartEnd(start: () => void, end: () => void): Prom end?.(); } -function triggerEndActionAndClearAnim(animation: Animation, end: () => void): void { - activeAnimationsFinishPromises.delete(animation); - end?.(); -} - function findAnimation( targetEl: HTMLElement, type: TransitionOrAnimation, @@ -673,20 +666,15 @@ export async function whenTransitionOrAnimationDone( return triggerFallbackStartEnd(onStart, onEnd); } - if (activeAnimationsFinishPromises.has(anim)) { - return activeAnimationsFinishPromises.get(anim); - } - onStart?.(); - activeAnimationsFinishPromises.set( - anim, - anim.finished - .then(() => {}) - .catch(() => { - // swallow error if canceled - }) - .finally(() => triggerEndActionAndClearAnim(anim, onEnd)), - ); + + try { + await anim.finished; + } catch { + // swallow error if canceled + } finally { + onEnd?.(); + } } function nextFrame(): Promise { From 88ac4ceb53758bc48ec2008be3fce434d71fc6d4 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Wed, 15 Jan 2025 00:15:10 -0800 Subject: [PATCH 3/4] fix alert test --- .../calcite-components/src/components/alert/alert.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/calcite-components/src/components/alert/alert.tsx b/packages/calcite-components/src/components/alert/alert.tsx index f543a7f2f68..dba0cda8d95 100644 --- a/packages/calcite-components/src/components/alert/alert.tsx +++ b/packages/calcite-components/src/components/alert/alert.tsx @@ -226,14 +226,14 @@ export class Alert extends LitElement implements OpenCloseComponent, LoadableCom To account for this semantics change, the checks for (this.hasUpdated || value != defaultValue) was added in this method Please refactor your code to reduce the need for this check. Docs: https://qawebgis.esri.com/arcgis-components/?path=/docs/lumina-transition-from-stencil--docs#watching-for-property-changes */ - if (changes.has("active") && (this.hasUpdated || this.active !== false)) { - this.handleActiveChange(); - } - if (changes.has("open") && (this.hasUpdated || this.open !== false)) { this.openHandler(); } + if (changes.has("active") && (this.hasUpdated || this.active !== false)) { + this.handleActiveChange(); + } + if ( changes.has("autoCloseDuration") && (this.hasUpdated || this.autoCloseDuration !== "medium") @@ -269,6 +269,7 @@ export class Alert extends LitElement implements OpenCloseComponent, LoadableCom // #region Private Methods private handleActiveChange(): void { + onToggleOpenCloseComponent(this); this.clearAutoCloseTimeout(); if (this.active && this.autoClose && !this.autoCloseTimeoutId) { this.initialOpenTime = Date.now(); @@ -280,7 +281,6 @@ export class Alert extends LitElement implements OpenCloseComponent, LoadableCom } private openHandler(): void { - onToggleOpenCloseComponent(this); if (this.open) { manager.registerElement(this.el); } else { From 83c8d3753f69b94feb21a430b15985ef6f8bff7a Mon Sep 17 00:00:00 2001 From: JC Franco Date: Wed, 15 Jan 2025 00:58:33 -0800 Subject: [PATCH 4/4] refactor: add useOpenClose controller --- .../src/controllers/useOpenClose | 14 ++++++++++++++ .../src/utils/openCloseComponent.ts | 5 +---- 2 files changed, 15 insertions(+), 4 deletions(-) create mode 100644 packages/calcite-components/src/controllers/useOpenClose diff --git a/packages/calcite-components/src/controllers/useOpenClose b/packages/calcite-components/src/controllers/useOpenClose new file mode 100644 index 00000000000..9e876c7a167 --- /dev/null +++ b/packages/calcite-components/src/controllers/useOpenClose @@ -0,0 +1,14 @@ +import { makeGenericController } from "@arcgis/components-controllers"; +import { onToggleOpenCloseComponent, OpenCloseComponent } from "../utils/openCloseComponent"; + +export interface UseOpenClose { + afterToggle: () => void; +} + +export const useOpenClose = makeGenericController((component, controller) => { + return { + afterToggle: () => { + onToggleOpenCloseComponent(component); + }, + }; +}); diff --git a/packages/calcite-components/src/utils/openCloseComponent.ts b/packages/calcite-components/src/utils/openCloseComponent.ts index f92daa0f1ea..0fdc38f220a 100644 --- a/packages/calcite-components/src/utils/openCloseComponent.ts +++ b/packages/calcite-components/src/utils/openCloseComponent.ts @@ -7,9 +7,6 @@ import { whenTransitionDone } from "./dom"; * All implementations of this interface must handle the following events: `beforeOpen`, `open`, `beforeClose`, `close`. */ export interface OpenCloseComponent { - /** The host element. */ - readonly el: HTMLElement; - /** * Specifies property on which active transition is watched for. * @@ -18,7 +15,7 @@ export interface OpenCloseComponent { openProp?: string; /** Specifies the name of CSS transition property. */ - transitionProp?: KebabCase>; + transitionProp: KebabCase>; /** Specifies element that the transition is allowed to emit on. */ transitionEl: HTMLElement;