fix(dialog, flow-item): slotted closable panels should not close the …

…component (#10130) **Related Issue:** #10129 ## Summary - updates `dialog` and `flow-item` to only listen to panel events for the internal panel. Excluding any other bubbling events. - uses `IDS` for unique ids of elements instead of `data-test` attributes. - stops propagation of internal panel events on dialog. - adds tests
Esri · Aug 22, 2024 · ce2513d · ce2513d
1 parent 7989b29
commit ce2513d
Show file tree

Hide file tree

Showing 7 changed files with 85 additions and 24 deletions.
diff --git a/packages/calcite-components/src/components/dialog/dialog.e2e.ts b/packages/calcite-components/src/components/dialog/dialog.e2e.ts
@@ -12,6 +12,7 @@ import {
 } from "../../tests/commonTests";
 import { html } from "../../../support/formatting";
 import { GlobalTestProps, isElementFocused, skipAnimations } from "../../tests/utils";
+import { IDS as PanelIDS } from "../panel/resources";
 import { DialogMessages } from "./assets/dialog/t9n";
 import { CSS, SLOTS } from "./resources";
 
@@ -367,7 +368,7 @@ describe("calcite-dialog", () => {
       await page.waitForChanges();
       expect(await page.find(`calcite-dialog >>> .${CSS.containerOpen}`)).toBeDefined();
 
-      const closeButton = await page.find(`calcite-dialog >>> calcite-panel >>> [data-test="close"]`);
+      const closeButton = await page.find(`calcite-dialog >>> calcite-panel >>> #${PanelIDS.close}`);
       await closeButton.click();
       await page.waitForChanges();
       expect(mockCallBack).toHaveBeenCalledTimes(2);
@@ -686,7 +687,7 @@ describe("calcite-dialog", () => {
     await page.waitForChanges();
     expect(await container.isVisible()).toBe(true);
 
-    const closeButton = await page.find(`calcite-dialog >>> calcite-panel >>> [data-test="close"]`);
+    const closeButton = await page.find(`calcite-dialog >>> calcite-panel >>> #${PanelIDS.close}`);
     await closeButton.click();
     await page.waitForChanges();
     expect(await container.isVisible()).toBe(false);
@@ -865,4 +866,21 @@ describe("calcite-dialog", () => {
 
     expect(await alert.getProperty("embedded")).toBe(true);
   });
+
+  it("should not close when slotted panels are closed", async () => {
+    const page = await newE2EPage({
+      html: html`<calcite-dialog open>
+        <calcite-panel closable heading="test"></calcite-panel>
+      </calcite-dialog>`,
+    });
+    await page.waitForChanges();
+
+    const closeButton = await page.find(`calcite-panel >>> #${PanelIDS.close}`);
+
+    await closeButton.click();
+    await page.waitForChanges();
+
+    const dialog = await page.find("calcite-dialog");
+    expect(await dialog.getProperty("open")).toBe(true);
+  });
 });
diff --git a/packages/calcite-components/src/components/dialog/dialog.tsx b/packages/calcite-components/src/components/dialog/dialog.tsx
@@ -256,8 +256,8 @@ export class Dialog
                 loading={this.loading}
                 menuOpen={this.menuOpen}
                 messageOverrides={this.messageOverrides}
-                onCalcitePanelClose={this.handleCloseClick}
-                onCalcitePanelScroll={this.handleScroll}
+                onCalcitePanelClose={this.handleInternalPanelCloseClick}
+                onCalcitePanelScroll={this.handleInternalPanelScroll}
                 onKeyDown={this.handlePanelKeyDown}
                 overlayPositioning={this.overlayPositioning}
                 ref={(el) => (this.panelEl = el)}
@@ -456,11 +456,21 @@ export class Dialog
     this.el.removeEventListener("calciteDialogOpen", this.openEnd);
   };
 
-  private handleScroll = (): void => {
+  private handleInternalPanelScroll = (event: CustomEvent<void>): void => {
+    if (event.target !== this.panelEl) {
+      return;
+    }
+
+    event.stopPropagation();
     this.calciteDialogScroll.emit();
   };
 
-  private handleCloseClick = (): void => {
+  private handleInternalPanelCloseClick = (event: CustomEvent<void>): void => {
+    if (event.target !== this.panelEl) {
+      return;
+    }
+
+    event.stopPropagation();
     this.open = false;
   };
 

diff --git a/packages/calcite-components/src/components/flow-item/flow-item.e2e.ts b/packages/calcite-components/src/components/flow-item/flow-item.e2e.ts
@@ -13,6 +13,7 @@ import {
 } from "../../tests/commonTests";
 import { html } from "../../../support/formatting";
 import { GlobalTestProps } from "../../tests/utils";
+import { IDS as PanelIDS } from "../panel/resources";
 import { CSS, SLOTS } from "./resources";
 
 type TestWindow = GlobalTestProps<{
@@ -237,11 +238,9 @@ describe("calcite-flow-item", () => {
     expect(await panel.getProperty("collapsed")).toBe(true);
     expect(await panel.getProperty("collapsible")).toBe(true);
 
-    await page.$eval("calcite-flow-item", (flowItem: HTMLCalciteFlowItemElement) => {
-      const panel = flowItem.shadowRoot.querySelector("calcite-panel");
-      const toggleButton = panel.shadowRoot.querySelector("[data-test=collapse]") as HTMLCalciteActionElement;
-      toggleButton.click();
-    });
+    const collapseButton = await page.find(`calcite-flow-item >>> calcite-panel >>> #${PanelIDS.collapse}`);
+    await collapseButton.click();
+    await page.waitForChanges();
 
     await page.waitForChanges();
 
@@ -348,4 +347,21 @@ describe("calcite-flow-item", () => {
 
     expect(await alert.getProperty("embedded")).toBe(true);
   });
+
+  it("should not close when slotted panels are closed", async () => {
+    const page = await newE2EPage({
+      html: html`<calcite-flow-item closable>
+        <calcite-panel closable heading="test"></calcite-panel>
+      </calcite-flow-item>`,
+    });
+    await page.waitForChanges();
+
+    const closeButton = await page.find(`calcite-panel >>> #${PanelIDS.close}`);
+
+    await closeButton.click();
+    await page.waitForChanges();
+
+    const flowItem = await page.find("calcite-flow-item");
+    expect(await flowItem.getProperty("closed")).toBe(false);
+  });
 });
diff --git a/packages/calcite-components/src/components/flow-item/flow-item.tsx b/packages/calcite-components/src/components/flow-item/flow-item.tsx
@@ -295,18 +295,30 @@ export class FlowItem
   //
   // --------------------------------------------------------------------------
 
-  handlePanelScroll = (event: CustomEvent<void>): void => {
+  handleInternalPanelScroll = (event: CustomEvent<void>): void => {
+    if (event.target !== this.containerEl) {
+      return;
+    }
+
     event.stopPropagation();
     this.calciteFlowItemScroll.emit();
   };
 
-  handlePanelClose = (event: CustomEvent<void>): void => {
+  handleInternalPanelClose = (event: CustomEvent<void>): void => {
+    if (event.target !== this.containerEl) {
+      return;
+    }
+
     event.stopPropagation();
     this.closed = true;
     this.calciteFlowItemClose.emit();
   };
 
-  handlePanelToggle = (event: CustomEvent<void>): void => {
+  handleInternalPanelToggle = (event: CustomEvent<void>): void => {
+    if (event.target !== this.containerEl) {
+      return;
+    }
+
     event.stopPropagation();
     this.collapsed = (event.target as HTMLCalcitePanelElement).collapsed;
     this.calciteFlowItemToggle.emit();
@@ -388,9 +400,9 @@ export class FlowItem
             loading={loading}
             menuOpen={menuOpen}
             messageOverrides={messages}
-            onCalcitePanelClose={this.handlePanelClose}
-            onCalcitePanelScroll={this.handlePanelScroll}
-            onCalcitePanelToggle={this.handlePanelToggle}
+            onCalcitePanelClose={this.handleInternalPanelClose}
+            onCalcitePanelScroll={this.handleInternalPanelScroll}
+            onCalcitePanelToggle={this.handleInternalPanelToggle}
             overlayPositioning={overlayPositioning}
             ref={this.setContainerRef}
             scale={this.scale}

diff --git a/packages/calcite-components/src/components/panel/panel.e2e.ts b/packages/calcite-components/src/components/panel/panel.e2e.ts
@@ -14,7 +14,7 @@ import {
   themed,
 } from "../../tests/commonTests";
 import { GlobalTestProps } from "../../tests/utils";
-import { CSS, SLOTS } from "./resources";
+import { CSS, IDS, SLOTS } from "./resources";
 
 type TestWindow = GlobalTestProps<{
   beforeClose: () => Promise<void>;
@@ -187,7 +187,7 @@ describe("calcite-panel", () => {
 
     const element = await page.find("calcite-panel");
     const container = await page.find(`calcite-panel >>> .${CSS.contentWrapper}`);
-    const collapseButtonSelector = `calcite-panel >>> [data-test="collapse"]`;
+    const collapseButtonSelector = `calcite-panel >>> #${IDS.collapse}`;
     expect(await page.find(collapseButtonSelector)).toBeNull();
 
     await page.waitForChanges();
@@ -208,7 +208,7 @@ describe("calcite-panel", () => {
 
     const calcitePanelClose = await page.spyOnEvent("calcitePanelClose", "window");
 
-    const closeButton = await page.find("calcite-panel >>> calcite-action[data-test=close]");
+    const closeButton = await page.find(`calcite-panel >>> #${IDS.close}`);
 
     await closeButton.click();
 
@@ -222,7 +222,7 @@ describe("calcite-panel", () => {
 
     const calcitePanelToggle = await page.spyOnEvent("calcitePanelToggle", "window");
 
-    const toggleButton = await page.find("calcite-panel >>> [data-test=collapse]");
+    const toggleButton = await page.find(`calcite-panel >>> #${IDS.collapse}`);
 
     await toggleButton.click();
 

diff --git a/packages/calcite-components/src/components/panel/panel.tsx b/packages/calcite-components/src/components/panel/panel.tsx
@@ -45,7 +45,7 @@ import { OverlayPositioning } from "../../utils/floating-ui";
 import { CollapseDirection } from "../interfaces";
 import { Scale } from "../interfaces";
 import { PanelMessages } from "./assets/panel/t9n";
-import { CSS, ICONS, SLOTS } from "./resources";
+import { CSS, ICONS, IDS, SLOTS } from "./resources";
 
 /**
  * @slot - A slot for adding custom content.
@@ -503,8 +503,8 @@ export class Panel
       <calcite-action
         aria-expanded={toAriaBoolean(!collapsed)}
         aria-label={collapse}
-        data-test="collapse"
         icon={collapsed ? icons[0] : icons[1]}
+        id={IDS.collapse}
         onClick={this.collapse}
         scale={this.scale}
         text={collapse}
@@ -515,8 +515,8 @@ export class Panel
     const closeNode = closable ? (
       <calcite-action
         aria-label={close}
-        data-test="close"
         icon={ICONS.close}
+        id={IDS.close}
         onClick={this.handleUserClose}
         scale={this.scale}
         text={close}

diff --git a/packages/calcite-components/src/components/panel/resources.ts b/packages/calcite-components/src/components/panel/resources.ts
@@ -23,6 +23,11 @@ export const CSS = {
   footerEnd: "footer-end",
 };
 
+export const IDS = {
+  close: "close",
+  collapse: "collapse",
+};
+
 export const ICONS = {
   close: "x",
   menu: "ellipsis",