Skip to content

Commit

Permalink
fix(modal): OpenCloseComponent emits when setting `--calcite-duration…
Browse files Browse the repository at this point in the history
…-factor` to 0 (#5326)

**Related Issue:** #5206

All (before)open/close events are now emitted on modal regardless of
transition duration.

Add the `onToggleUnanimatedComponent` function to the
`openCloseComponent Interface` to query props on the `transitionEl`. Use
that to get the `transition-duration` on the `openTransitionProp`, which
in the case of modal it’s the `opacity`. This will allow us to emit the
open/close and beforeOpen/beforeClose events in cases where `opacity`
transition duration is set to 0.

Because calling computed style forces a layout, we use `readTask()`
instead which schedules a DOM-read task. The provided callback will be
executed at the best moment to perform DOM reads without causing layout
thrashing as per Stencil docs.

Updated to rework `openCloseComponent Interface` to call one function
instead of 3 separate calls in every component by removing
connect/disconnect and event listener mapping logic and passing argument
`once` to `.addEventListener` on `transitionEl` instead.
  • Loading branch information
Elijbet committed Dec 6, 2022
1 parent dd6123b commit ff19420
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 27 deletions.
122 changes: 104 additions & 18 deletions src/components/modal/modal.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { newE2EPage } from "@stencil/core/testing";
import { focusable, renders, slots, hidden } from "../../tests/commonTests";
import { html } from "../../../support/formatting";
import { CSS, SLOTS } from "./resources";
import { CSS, SLOTS, DURATIONS } from "./resources";
import { newProgrammaticE2EPage, skipAnimations } from "../../tests/utils";

describe("calcite-modal properties", () => {
Expand Down Expand Up @@ -64,26 +64,34 @@ describe("calcite-modal properties", () => {
});

describe("opening and closing behavior", () => {
it.skip("opens and closes", async () => {
function getTransitionTransform(
modalSelector: string,
modalContainerSelector: string,
type: "none" | "matrix"
): boolean {
const modalContainer = document
.querySelector(modalSelector)
.shadowRoot.querySelector<HTMLElement>(modalContainerSelector);
return getComputedStyle(modalContainer).transform.startsWith(type);
}

const getTransitionDuration = (): { duration: string } => {
const modal = document.querySelector("calcite-modal");
const { transitionDuration } = window.getComputedStyle(modal);
return {
duration: transitionDuration
};
};

it("opens and closes", async () => {
const page = await newE2EPage();
await page.setContent(`<calcite-modal></calcite-modal>`);
await page.setContent(html`<calcite-modal style="transition: opacity ${DURATIONS.test}s"></calcite-modal>`);
const modal = await page.find("calcite-modal");
const beforeOpenSpy = await modal.spyOnEvent("calciteModalBeforeOpen");
const openSpy = await modal.spyOnEvent("calciteModalOpen");
const beforeCloseSpy = await modal.spyOnEvent("calciteModalBeforeClose");
const closeSpy = await modal.spyOnEvent("calciteModalClose");

function getTransitionTransform(
modalSelector: string,
modalContainerSelector: string,
type: "none" | "matrix"
): boolean {
const modalContainer = document
.querySelector(modalSelector)
.shadowRoot.querySelector<HTMLElement>(modalContainerSelector);
return getComputedStyle(modalContainer).transform.startsWith(type);
}

expect(beforeOpenSpy).toHaveReceivedEventTimes(0);
expect(openSpy).toHaveReceivedEventTimes(0);
expect(beforeCloseSpy).toHaveReceivedEventTimes(0);
Expand Down Expand Up @@ -117,6 +125,9 @@ describe("opening and closing behavior", () => {
await page.waitForChanges();
await waitForEvent;

const opacityTransition = await page.evaluate(getTransitionDuration);
expect(opacityTransition.duration).toEqual(`${DURATIONS.test}s`);

expect(beforeOpenSpy).toHaveReceivedEventTimes(1);
expect(openSpy).toHaveReceivedEventTimes(1);
expect(beforeCloseSpy).toHaveReceivedEventTimes(1);
Expand All @@ -135,22 +146,97 @@ describe("opening and closing behavior", () => {
expect(await modal.getProperty("open")).toBe(false);
});

it("emits when set up to open on initial render", async () => {
it("emits when set to open on initial render", async () => {
const page = await newProgrammaticE2EPage();
const waitForOpen = page.waitForEvent("calciteModalOpen");

const beforeOpenSpy = await page.spyOnEvent("calciteModalBeforeOpen");
const openSpy = await page.spyOnEvent("calciteModalOpen");

await page.evaluate(() => {
await page.evaluate((transitionDuration: string): void => {
const modal = document.createElement("calcite-modal");
modal.open = true;
modal.style.transition = `opacity ${transitionDuration}s`;
document.body.append(modal);
}, `${DURATIONS.test}`);

await page.waitForTimeout(DURATIONS.test);

const waitForBeforeOpenEvent = page.waitForEvent("calciteModalBeforeOpen");
const waitForOpenEvent = page.waitForEvent("calciteModalOpen");

await waitForBeforeOpenEvent;
await waitForOpenEvent;

expect(beforeOpenSpy).toHaveReceivedEventTimes(1);
expect(openSpy).toHaveReceivedEventTimes(1);
});

it("emits when set to open on initial render and duration is 0", async () => {
const page = await newProgrammaticE2EPage();
await skipAnimations(page);

const beforeOpenSpy = await page.spyOnEvent("calciteModalBeforeOpen");
const openSpy = await page.spyOnEvent("calciteModalOpen");

await page.evaluate((): void => {
const modal = document.createElement("calcite-modal");
modal.open = true;
document.body.append(modal);
});
await waitForOpen;

const opacityTransition = await page.evaluate(getTransitionDuration);
expect(opacityTransition.duration).toEqual("0s");

await page.waitForChanges;

const waitForOpenEvent = page.waitForEvent("calciteModalOpen");
const waitForBeforeOpenEvent = page.waitForEvent("calciteModalBeforeOpen");

await waitForBeforeOpenEvent;
await waitForOpenEvent;

expect(beforeOpenSpy).toHaveReceivedEventTimes(1);
expect(openSpy).toHaveReceivedEventTimes(1);
});

it("emits when duration is set to 0", async () => {
const page = await newProgrammaticE2EPage();
await skipAnimations(page);

const beforeOpenSpy = await page.spyOnEvent("calciteModalBeforeOpen");
const openSpy = await page.spyOnEvent("calciteModalOpen");

const beforeCloseSpy = await page.spyOnEvent("calciteModalBeforeClose");
const closeSpy = await page.spyOnEvent("calciteModalClose");

await page.evaluate((): void => {
const modal = document.createElement("calcite-modal");
modal.open = true;
document.body.append(modal);
});

const opacityTransition = await page.evaluate(getTransitionDuration);
expect(opacityTransition.duration).toEqual("0s");

await page.waitForChanges();
await beforeOpenSpy;
await openSpy;

expect(beforeOpenSpy).toHaveReceivedEventTimes(1);
expect(openSpy).toHaveReceivedEventTimes(1);

await page.evaluate(() => {
const modal = document.querySelector("calcite-modal");
modal.open = false;
});

await page.waitForChanges();
await beforeCloseSpy;
await closeSpy;

expect(beforeCloseSpy).toHaveReceivedEventTimes(1);
expect(closeSpy).toHaveReceivedEventTimes(1);
});
});

describe("calcite-modal accessibility checks", () => {
Expand Down
11 changes: 3 additions & 8 deletions src/components/modal/modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,7 @@ import {
connectConditionalSlotComponent,
disconnectConditionalSlotComponent
} from "../../utils/conditionalSlot";
import {
OpenCloseComponent,
connectOpenCloseComponent,
disconnectOpenCloseComponent
} from "../../utils/openCloseComponent";
import { OpenCloseComponent, onToggleOpenCloseComponent } from "../../utils/openCloseComponent";
import {
FocusTrapComponent,
FocusTrap,
Expand Down Expand Up @@ -134,6 +130,7 @@ export class Modal
setUpLoadableComponent(this);
// when modal initially renders, if active was set we need to open as watcher doesn't fire
if (this.open) {
onToggleOpenCloseComponent(this);
requestAnimationFrame(() => this.openModal());
}
}
Expand All @@ -146,7 +143,6 @@ export class Modal
this.mutationObserver?.observe(this.el, { childList: true, subtree: true });
this.updateFooterVisibility();
connectConditionalSlotComponent(this);
connectOpenCloseComponent(this);
if (this.open) {
this.active = this.open;
}
Expand All @@ -159,7 +155,6 @@ export class Modal
this.removeOverflowHiddenClass();
this.mutationObserver?.disconnect();
disconnectConditionalSlotComponent(this);
disconnectOpenCloseComponent(this);
deactivateFocusTrap(this);
}

Expand Down Expand Up @@ -397,7 +392,6 @@ export class Modal

private setTransitionEl = (el: HTMLDivElement): void => {
this.transitionEl = el;
connectOpenCloseComponent(this);
this.focusTrapEl = el;
connectFocusTrap(this);
};
Expand Down Expand Up @@ -431,6 +425,7 @@ export class Modal
@Watch("open")
async toggleModal(value: boolean): Promise<void> {
this.active = value;
onToggleOpenCloseComponent(this);
if (value) {
this.transitionEl?.classList.add(CSS.openingIdle);
this.openModal();
Expand Down
7 changes: 7 additions & 0 deletions src/components/modal/resources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ export const CSS = {
closingActive: "modal--closing-active"
};

export const DURATIONS = {
slow: 14000,
medium: 10000,
fast: 6000,
test: 300 / 1000
};

export const ICONS = {
close: "x"
};
Expand Down
39 changes: 38 additions & 1 deletion src/utils/openCloseComponent.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { readTask } from "@stencil/core";
/**
* Defines interface for components with open/close public emitter.
* All implementations of this interface must handle the following events: `beforeOpen`, `open`, `beforeClose`, `close`.
Expand Down Expand Up @@ -54,12 +55,48 @@ function transitionStart(event: TransitionEvent): void {
this.open ? this.onBeforeOpen() : this.onBeforeClose();
}
}

function transitionEnd(event: TransitionEvent): void {
if (event.propertyName === this.openTransitionProp && event.target === this.transitionEl) {
this.open ? this.onOpen() : this.onClose();
}
}

/**
* 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).
*
* @param component
*/
export function onToggleOpenCloseComponent(component: OpenCloseComponent): void {
readTask((): void => {
if (component.transitionEl) {
const allTransitionPropsArray = getComputedStyle(component.transitionEl).transition.split(" ");
const openTransitionPropIndex = allTransitionPropsArray.findIndex(
(item) => item === component.openTransitionProp
);
const transitionDuration = allTransitionPropsArray[openTransitionPropIndex + 1];
if (transitionDuration === "0s") {
component.open ? component.onBeforeOpen() : component.onBeforeClose();
component.open ? component.onOpen() : component.onClose();
} else {
component.transitionEl.addEventListener(
"transitionstart",
() => {
component.open ? component.onBeforeOpen() : component.onBeforeClose();
},
{ once: true }
);
component.transitionEl.addEventListener(
"transitionend",
() => {
component.open ? component.onOpen() : component.onClose();
},
{ once: true }
);
}
}
});
}
/**
* Helper to keep track of transition listeners on setTransitionEl and connectedCallback on OpenCloseComponent components.
*
Expand Down

0 comments on commit ff19420

Please sign in to comment.