diff --git a/packages/calcite-components/src/components/modal/modal.e2e.ts b/packages/calcite-components/src/components/modal/modal.e2e.ts index 30a4a205732..aa8068dc4a8 100644 --- a/packages/calcite-components/src/components/modal/modal.e2e.ts +++ b/packages/calcite-components/src/components/modal/modal.e2e.ts @@ -35,8 +35,8 @@ describe("calcite-modal properties", () => { const modal = await page.find("calcite-modal"); await modal.setProperty("open", true); await page.waitForChanges(); - const style = await page.$eval("calcite-modal", (elm) => { - const m = elm.shadowRoot.querySelector(".modal"); + const style = await page.$eval("calcite-modal", (el) => { + const m = el.shadowRoot.querySelector(".modal"); return window.getComputedStyle(m).getPropertyValue("width"); }); expect(style).toEqual("600px"); @@ -50,8 +50,8 @@ describe("calcite-modal properties", () => { const modal = await page.find("calcite-modal"); await modal.setProperty("open", true); await page.waitForChanges(); - const style = await page.$eval("calcite-modal", (elm) => { - const m = elm.shadowRoot.querySelector(".modal"); + const style = await page.$eval("calcite-modal", (el) => { + const m = el.shadowRoot.querySelector(".modal"); return window.getComputedStyle(m).getPropertyValue("height"); }); expect(style).toEqual("600px"); @@ -65,8 +65,8 @@ describe("calcite-modal properties", () => { const modal = await page.find("calcite-modal"); await modal.setProperty("open", true); await page.waitForChanges(); - const style = await page.$eval("calcite-modal", (elm) => { - const m = elm.shadowRoot.querySelector(".modal"); + const style = await page.$eval("calcite-modal", (el) => { + const m = el.shadowRoot.querySelector(".modal"); return window.getComputedStyle(m).getPropertyValue("width"); }); expect(style).not.toEqual("600px"); @@ -80,8 +80,8 @@ describe("calcite-modal properties", () => { const modal = await page.find("calcite-modal"); await modal.setProperty("open", true); await page.waitForChanges(); - const style = await page.$eval("calcite-modal", (elm) => { - const m = elm.shadowRoot.querySelector(".modal"); + const style = await page.$eval("calcite-modal", (el) => { + const m = el.shadowRoot.querySelector(".modal"); return window.getComputedStyle(m).getPropertyValue("height"); }); expect(style).not.toEqual("600px"); @@ -97,19 +97,19 @@ describe("calcite-modal properties", () => { const modal = await page.find("calcite-modal"); await modal.setProperty("open", true); await page.waitForChanges(); - const styleW = await page.$eval("calcite-modal", (elm) => { - const m = elm.shadowRoot.querySelector(".modal"); + const styleW = await page.$eval("calcite-modal", (el) => { + const m = el.shadowRoot.querySelector(".modal"); return window.getComputedStyle(m).getPropertyValue("width"); }); - const styleH = await page.$eval("calcite-modal", (elm) => { - const m = elm.shadowRoot.querySelector(".modal"); + const styleH = await page.$eval("calcite-modal", (el) => { + const m = el.shadowRoot.querySelector(".modal"); return window.getComputedStyle(m).getPropertyValue("height"); }); expect(styleW).toEqual("800px"); expect(styleH).toEqual("800px"); }); - it("calls the beforeClose method prior to closing", async () => { + it("calls the beforeClose method prior to closing via click", async () => { const page = await newE2EPage(); const mockCallBack = jest.fn(); await page.exposeFunction("beforeClose", mockCallBack); @@ -119,18 +119,74 @@ describe("calcite-modal properties", () => { const modal = await page.find("calcite-modal"); await page.$eval( "calcite-modal", - (elm: HTMLCalciteModalElement) => - (elm.beforeClose = (window as typeof window & Pick).beforeClose) + (el: HTMLCalciteModalElement) => + (el.beforeClose = ( + window as GlobalTestProps<{ beforeClose: HTMLCalciteModalElement["beforeClose"] }> + ).beforeClose) ); await page.waitForChanges(); - await modal.setProperty("open", true); + modal.setProperty("open", true); await page.waitForChanges(); - await modal.setProperty("open", false); + expect(await modal.getProperty("opened")).toBe(true); + const closeButton = await page.find(`calcite-modal >>> .${CSS.close}`); + await closeButton.click(); + await page.waitForChanges(); + expect(mockCallBack).toHaveBeenCalledTimes(1); + expect(await modal.getProperty("opened")).toBe(false); + }); + + it("calls the beforeClose method prior to closing via ESC key", async () => { + const page = await newE2EPage(); + const mockCallBack = jest.fn(); + await page.exposeFunction("beforeClose", mockCallBack); + await page.setContent(` + + `); + const modal = await page.find("calcite-modal"); + await page.$eval( + "calcite-modal", + (el: HTMLCalciteModalElement) => + (el.beforeClose = ( + window as GlobalTestProps<{ beforeClose: HTMLCalciteModalElement["beforeClose"] }> + ).beforeClose) + ); + await page.waitForChanges(); + modal.setProperty("open", true); + await page.waitForChanges(); + expect(await modal.getProperty("opened")).toBe(true); + await page.keyboard.press("Escape"); await page.waitForChanges(); - expect(mockCallBack).toHaveBeenCalled(); + await page.waitForChanges(); + expect(mockCallBack).toHaveBeenCalledTimes(1); + expect(await modal.getProperty("opened")).toBe(false); }); }); +it("calls the beforeClose method prior to closing via attribute", async () => { + const page = await newE2EPage(); + const mockCallBack = jest.fn(); + await page.exposeFunction("beforeClose", mockCallBack); + await page.setContent(` + + `); + const modal = await page.find("calcite-modal"); + await page.$eval( + "calcite-modal", + (el: HTMLCalciteModalElement) => + (el.beforeClose = ( + window as GlobalTestProps<{ beforeClose: HTMLCalciteModalElement["beforeClose"] }> + ).beforeClose) + ); + await page.waitForChanges(); + modal.setProperty("open", true); + await page.waitForChanges(); + expect(await modal.getProperty("opened")).toBe(true); + modal.removeAttribute("open"); + await page.waitForChanges(); + expect(mockCallBack).toHaveBeenCalledTimes(1); + expect(await modal.getProperty("opened")).toBe(false); +}); + describe("opening and closing behavior", () => { it("opens and closes", async () => { const page = await newE2EPage(); @@ -462,7 +518,7 @@ describe("calcite-modal accessibility checks", () => { modal.setProperty("open", true); await page.waitForChanges(); expect(modal).toHaveAttribute("open"); - await page.$eval("calcite-modal", (elm) => elm.shadowRoot.querySelector("calcite-scrim").click()); + await page.$eval("calcite-modal", (el) => el.shadowRoot.querySelector("calcite-scrim").click()); await page.waitForChanges(); expect(await modal.getProperty("open")).toBe(false); }); @@ -474,7 +530,7 @@ describe("calcite-modal accessibility checks", () => { modal.setProperty("open", true); await page.waitForChanges(); expect(modal).toHaveAttribute("open"); - await page.$eval("calcite-modal", (elm) => elm.shadowRoot.querySelector("calcite-scrim").click()); + await page.$eval("calcite-modal", (el) => el.shadowRoot.querySelector("calcite-scrim").click()); await page.waitForChanges(); expect(await modal.getProperty("open")).toBe(true); }); @@ -536,11 +592,11 @@ describe("calcite-modal accessibility checks", () => { TEST `); - let footer = await page.$eval("calcite-modal", (elm) => elm.shadowRoot.querySelector(".footer")); + let footer = await page.$eval("calcite-modal", (el) => el.shadowRoot.querySelector(".footer")); expect(footer).toBeDefined(); - await page.$eval("calcite-button", (elm) => elm.parentElement.removeChild(elm)); + await page.$eval("calcite-button", (el) => el.parentElement.removeChild(el)); await page.waitForChanges(); - footer = await page.$eval("calcite-modal", (elm) => elm.shadowRoot.querySelector(".footer")); + footer = await page.$eval("calcite-modal", (el) => el.shadowRoot.querySelector(".footer")); expect(footer).toBeFalsy(); }); diff --git a/packages/calcite-components/src/components/modal/modal.scss b/packages/calcite-components/src/components/modal/modal.scss index 728f961665b..00cb5216cbf 100644 --- a/packages/calcite-components/src/components/modal/modal.scss +++ b/packages/calcite-components/src/components/modal/modal.scss @@ -108,7 +108,7 @@ } } -:host([open]) { +:host([opened]) { @apply opacity-100; visibility: visible !important; transition-delay: 0ms; @@ -310,7 +310,7 @@ slot[name="primary"] { } } -:host([open][fullscreen]) { +:host([opened][fullscreen]) { .header, .footer, .content-top, diff --git a/packages/calcite-components/src/components/modal/modal.tsx b/packages/calcite-components/src/components/modal/modal.tsx index 69cb37eb9ab..cc6fd44d864 100644 --- a/packages/calcite-components/src/components/modal/modal.tsx +++ b/packages/calcite-components/src/components/modal/modal.tsx @@ -86,9 +86,15 @@ export class Modal /** When `true`, displays and positions the component. */ @Prop({ mutable: true, reflect: true }) open = false; + /** + * We use an internal property to handle styles for when a modal is actually opened, not just when the open attribute is applied. This is a property because we need to apply styles to the host element and to keep the styles present while beforeClose is. + * + * @internal + */ + @Prop({ mutable: true, reflect: true }) opened = false; + /** Passes a function to run before the component closes. */ - @Prop() - beforeClose: (el: HTMLElement) => Promise = () => Promise.resolve(); + @Prop() beforeClose: (el: HTMLCalciteModalElement) => Promise; /** When `true`, disables the component's close button. */ @Prop({ reflect: true }) closeButtonDisabled = false; @@ -208,7 +214,7 @@ export class Modal
@@ -284,7 +290,7 @@ export class Modal aria-label={this.messages.close} class={CSS.close} key="button" - onClick={this.close} + onClick={this.closeModal} title={this.messages.close} // eslint-disable-next-line react/jsx-sort-props -- ref should be last so node attrs/props are in sync (see https://github.com/Esri/calcite-design-system/pull/6530) ref={(el) => (this.closeButtonEl = el)} @@ -343,6 +349,8 @@ export class Modal // //-------------------------------------------------------------------------- + ignoreOpenChange = false; + @Element() el: HTMLCalciteModalElement; modalContent: HTMLDivElement; @@ -379,13 +387,6 @@ export class Modal @State() hasContentBottom = false; - /** - * We use internal variable to make sure initially open modal can transition from closed state when rendered - * - * @private - */ - @State() isOpen = false; - @State() effectiveLocale: string; @Watch("effectiveLocale") @@ -404,7 +405,7 @@ export class Modal @Listen("keydown", { target: "window" }) handleEscape(event: KeyboardEvent): void { if (this.open && !this.escapeDisabled && event.key === "Escape" && !event.defaultPrevented) { - this.close(); + this.closeModal(); event.preventDefault(); } } @@ -508,7 +509,7 @@ export class Modal this.openModal(); } else { this.transitionEl?.classList.add(CSS.closingIdle); - this.close(); + this.closeModal(); } } @@ -519,9 +520,14 @@ export class Modal /** Open the modal */ private openModal() { + if (this.ignoreOpenChange) { + return; + } + + this.ignoreOpenChange = true; this.el.addEventListener("calciteModalOpen", this.openEnd); this.open = true; - this.isOpen = true; + this.opened = true; const titleEl = getSlotted(this.el, SLOTS.header); const contentEl = getSlotted(this.el, SLOTS.content); @@ -533,23 +539,32 @@ export class Modal // use an inline style instead of a utility class to avoid global class declarations. document.documentElement.style.setProperty("overflow", "hidden"); } + this.ignoreOpenChange = false; } - handleOutsideClose = (): void => { + private handleOutsideClose = (): void => { if (this.outsideCloseDisabled) { return; } - this.close(); + this.closeModal(); }; /** Close the modal, first running the `beforeClose` method */ - close = (): Promise => { - return this.beforeClose(this.el).then(() => { - this.open = false; - this.isOpen = false; - this.removeOverflowHiddenClass(); - }); + closeModal = async (): Promise => { + if (this.ignoreOpenChange) { + return; + } + + if (this.beforeClose) { + await this.beforeClose(this.el); + } + + this.ignoreOpenChange = true; + this.open = false; + this.opened = false; + this.removeOverflowHiddenClass(); + this.ignoreOpenChange = false; }; private removeOverflowHiddenClass(): void {