fix(sheet): calciteSheetClose should emit on scrim click. #7681

Esri · Sep 6, 2023 · d7b45d8 · d7b45d8
1 parent 9797977
commit d7b45d8
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 28 deletions.
diff --git a/packages/calcite-components/src/components/sheet/sheet.e2e.ts b/packages/calcite-components/src/components/sheet/sheet.e2e.ts
@@ -411,23 +411,47 @@ describe("calcite-sheet properties", () => {
 
     it("emits when closing on click", async () => {
       const page = await newE2EPage();
-      await page.setContent(html`<calcite-sheet open></calcite-sheet>`);
+      await page.setContent(html`<calcite-sheet></calcite-sheet>`);
       const sheet = await page.find("calcite-sheet");
 
+      const beforeOpenSpy = await sheet.spyOnEvent("calciteSheetBeforeOpen");
+      const openSpy = await sheet.spyOnEvent("calciteSheetOpen");
+      const beforeCloseSpy = await sheet.spyOnEvent("calciteSheetBeforeClose");
+      const closeSpy = await sheet.spyOnEvent("calciteSheetClose");
+
+      expect(beforeOpenSpy).toHaveReceivedEventTimes(0);
+      expect(openSpy).toHaveReceivedEventTimes(0);
+      expect(beforeCloseSpy).toHaveReceivedEventTimes(0);
+      expect(closeSpy).toHaveReceivedEventTimes(0);
+
+      expect(await sheet.isVisible()).toBe(false);
+
+      const sheetBeforeOpen = page.waitForEvent("calciteSheetBeforeOpen");
+      const sheetOpen = page.waitForEvent("calciteSheetOpen");
+      sheet.setProperty("open", true);
       await page.waitForChanges();
+
+      await sheetBeforeOpen;
+      await sheetOpen;
+
+      expect(beforeOpenSpy).toHaveReceivedEventTimes(1);
+      expect(openSpy).toHaveReceivedEventTimes(1);
+      expect(beforeCloseSpy).toHaveReceivedEventTimes(0);
+      expect(closeSpy).toHaveReceivedEventTimes(0);
+
       expect(await sheet.isVisible()).toBe(true);
 
-      const beforeCloseSpy = await sheet.spyOnEvent("calciteSheetBeforeClose");
-      const closeSpy = await sheet.spyOnEvent("calciteSheetClose");
       const sheetBeforeClose = page.waitForEvent("calciteSheetBeforeClose");
       const sheetClose = page.waitForEvent("calciteSheetClose");
-
       const scrim = await page.find(`calcite-sheet >>> .${CSS.scrim}`);
       await scrim.click();
+      await page.waitForChanges();
 
       await sheetBeforeClose;
       await sheetClose;
 
+      expect(beforeOpenSpy).toHaveReceivedEventTimes(1);
+      expect(openSpy).toHaveReceivedEventTimes(1);
       expect(beforeCloseSpy).toHaveReceivedEventTimes(1);
       expect(closeSpy).toHaveReceivedEventTimes(1);
 

diff --git a/packages/calcite-components/src/components/sheet/sheet.tsx b/packages/calcite-components/src/components/sheet/sheet.tsx
@@ -89,19 +89,23 @@ export class Sheet implements OpenCloseComponent, FocusTrapComponent, LoadableCo
   @Prop({ mutable: true, reflect: true }) open = false;
 
   @Watch("open")
-  async toggleSheet(value: boolean): Promise<void> {
+  toggleSheet(value: boolean): void {
     if (this.ignoreOpenChange) {
       return;
     }
 
-    onToggleOpenCloseComponent(this);
     if (value) {
-      this.openSheet(true);
+      this.openSheet();
     } else {
-      this.closeSheet(true);
+      this.closeSheet();
     }
   }
 
+  @Watch("opened")
+  handleOpenedChange(): void {
+    onToggleOpenCloseComponent(this);
+  }
+
   /**
    * 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 .
    *
@@ -138,7 +142,6 @@ export class Sheet implements OpenCloseComponent, FocusTrapComponent, LoadableCo
     setUpLoadableComponent(this);
     // when sheet initially renders, if active was set we need to open as watcher doesn't fire
     if (this.open) {
-      onToggleOpenCloseComponent(this);
       requestAnimationFrame(() => this.openSheet());
     }
   }
@@ -224,7 +227,7 @@ export class Sheet implements OpenCloseComponent, FocusTrapComponent, LoadableCo
   @Listen("keydown", { target: "window" })
   handleEscape(event: KeyboardEvent): void {
     if (this.open && !this.escapeDisabled && event.key === "Escape" && !event.defaultPrevented) {
-      this.closeSheet();
+      this.open = false;
       event.preventDefault();
     }
   }
@@ -305,37 +308,25 @@ export class Sheet implements OpenCloseComponent, FocusTrapComponent, LoadableCo
     this.el.removeEventListener("calciteSheetOpen", this.openEnd);
   };
 
-  private openSheet(ignoreOpenChange = false): void {
-    if (this.ignoreOpenChange) {
-      return;
-    }
-
-    this.ignoreOpenChange = ignoreOpenChange;
+  private openSheet(): void {
     this.el.addEventListener("calciteSheetOpen", this.openEnd);
-    this.open = true;
     this.opened = true;
     if (!this.slottedInShell) {
       this.initialOverflowCSS = document.documentElement.style.overflow;
       // use an inline style instead of a utility class to avoid global class declarations.
       document.documentElement.style.setProperty("overflow", "hidden");
     }
-
-    this.ignoreOpenChange = false;
   }
 
   private handleOutsideClose = (): void => {
     if (this.outsideCloseDisabled) {
       return;
     }
 
-    this.closeSheet();
+    this.open = false;
   };
 
-  private closeSheet = async (ignoreOpenChange = false): Promise<void> => {
-    if (this.ignoreOpenChange) {
-      return;
-    }
-
+  private closeSheet = async (): Promise<void> => {
     if (this.beforeClose) {
       try {
         await this.beforeClose(this.el);
@@ -350,11 +341,8 @@ export class Sheet implements OpenCloseComponent, FocusTrapComponent, LoadableCo
       }
     }
 
-    this.ignoreOpenChange = ignoreOpenChange;
-    this.open = false;
     this.opened = false;
     this.removeOverflowHiddenClass();
-    this.ignoreOpenChange = false;
   };
 
   private removeOverflowHiddenClass(): void {