Skip to content

Commit

Permalink
fix(dialog, panel): prevent beforeClose from being invoked during i…
Browse files Browse the repository at this point in the history
…nitialization (#11038)

**Related Issue:** #10731 

## Summary

`willUpdate` was invoking toggling logic that was meant to be called
after initialization (compare with [Stencil
version](https://github.com/Esri/calcite-design-system/blame/bcc19be314a45ff584793e473e5851e15cefa83f/packages/calcite-components/src/components/panel/panel.tsx#L92)).
  • Loading branch information
jcfranco authored Dec 12, 2024
1 parent 797b03f commit 1a0bf3b
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 26 deletions.
15 changes: 14 additions & 1 deletion packages/calcite-components/src/components/dialog/dialog.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
themed,
} from "../../tests/commonTests";
import { html } from "../../../support/formatting";
import { GlobalTestProps, isElementFocused, skipAnimations } from "../../tests/utils";
import { GlobalTestProps, isElementFocused, newProgrammaticE2EPage, skipAnimations } from "../../tests/utils";
import { IDS as PanelIDS } from "../panel/resources";
import { CSS, dialogDragStep, dialogResizeStep, SLOTS } from "./resources";
import type { Dialog } from "./dialog";
Expand Down Expand Up @@ -512,6 +512,19 @@ describe("calcite-dialog", () => {
expect(await page.find(`calcite-dialog >>> .${CSS.containerOpen}`)).toBeDefined();
expect(dialog.getAttribute("open")).toBe(""); // Makes sure attribute is added back
});

it("does not invoke beforeClose when initially open", async () => {
const page = await newProgrammaticE2EPage();
await page.evaluate(async () => {
const dialog = document.createElement("calcite-dialog");
dialog.open = true;
dialog.beforeClose = () => new Promise(() => document.body.removeChild(dialog));
document.body.append(dialog);
});
await page.waitForChanges();

expect(await page.find("calcite-dialog")).not.toBeNull();
});
});

describe("calcite-dialog accessibility checks", () => {
Expand Down
63 changes: 39 additions & 24 deletions packages/calcite-components/src/components/panel/panel.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
themed,
handlesActionMenuPlacements,
} from "../../tests/commonTests";
import { GlobalTestProps } from "../../tests/utils";
import { GlobalTestProps, newProgrammaticE2EPage } from "../../tests/utils";
import { defaultEndMenuPlacement } from "../../utils/floating-ui";
import { CSS, IDS, SLOTS } from "./resources";
import type { Panel } from "./panel";
Expand Down Expand Up @@ -255,39 +255,54 @@ describe("calcite-panel", () => {
expect(await container.isVisible()).toBe(false);
});

it("should handle rejected 'beforeClose' promise'", async () => {
const page = await newE2EPage();
describe("beforeClose", () => {
it("should handle rejected 'beforeClose' promise'", async () => {
const page = await newE2EPage();

const mockCallBack = vi.fn().mockReturnValue(() => Promise.reject());
await page.exposeFunction("beforeClose", mockCallBack);
const mockCallBack = vi.fn().mockReturnValue(() => Promise.reject());
await page.exposeFunction("beforeClose", mockCallBack);

await page.setContent(`<calcite-panel closable></calcite-panel>`);
await page.setContent(`<calcite-panel closable></calcite-panel>`);

await page.$eval("calcite-panel", (el: Panel["el"]) => (el.beforeClose = (window as TestWindow).beforeClose));
await page.waitForChanges();
await page.$eval("calcite-panel", (el: Panel["el"]) => (el.beforeClose = (window as TestWindow).beforeClose));
await page.waitForChanges();

const panel = await page.find("calcite-panel");
expect(await panel.getProperty("closed")).toBe(false);
panel.setProperty("closed", true);
await page.waitForChanges();
const panel = await page.find("calcite-panel");
expect(await panel.getProperty("closed")).toBe(false);
panel.setProperty("closed", true);
await page.waitForChanges();

expect(mockCallBack).toHaveBeenCalledTimes(1);
});
expect(mockCallBack).toHaveBeenCalledTimes(1);
});

it("should remain open with rejected 'beforeClose' promise'", async () => {
const page = await newE2EPage();
it("should remain open with rejected 'beforeClose' promise'", async () => {
const page = await newE2EPage();

await page.exposeFunction("beforeClose", () => Promise.reject());
await page.setContent(`<calcite-panel closable></calcite-panel>`);
await page.exposeFunction("beforeClose", () => Promise.reject());
await page.setContent(`<calcite-panel closable></calcite-panel>`);

await page.$eval("calcite-panel", (el: Panel["el"]) => (el.beforeClose = (window as TestWindow).beforeClose));
await page.$eval("calcite-panel", (el: Panel["el"]) => (el.beforeClose = (window as TestWindow).beforeClose));

const panel = await page.find("calcite-panel");
panel.setProperty("closed", true);
await page.waitForChanges();
const panel = await page.find("calcite-panel");
panel.setProperty("closed", true);
await page.waitForChanges();

expect(await panel.getProperty("closed")).toBe(false);
expect(panel.getAttribute("closed")).toBe(null); // Makes sure attribute is added back
});

expect(await panel.getProperty("closed")).toBe(false);
expect(panel.getAttribute("closed")).toBe(null); // Makes sure attribute is added back
it("does not invoke beforeClose when initially closed", async () => {
const page = await newProgrammaticE2EPage();
await page.evaluate(async () => {
const panel = document.createElement("calcite-panel");
panel.closed = true;
panel.beforeClose = () => new Promise(() => document.body.removeChild(panel));
document.body.append(panel);
});
await page.waitForChanges();

expect(await page.find("calcite-panel")).not.toBeNull();
});
});

it("honors collapsed & collapsible properties", async () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/calcite-components/src/components/panel/panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ export class Panel extends LitElement implements InteractiveComponent, LoadableC
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("closed") && (this.hasUpdated || this.closed !== false)) {
if (changes.has("closed") && this.hasUpdated) {
if (this.closed) {
this.close();
} else {
Expand Down

0 comments on commit 1a0bf3b

Please sign in to comment.