fix: ensure beforeOpen/open and beforeClose/close events emit…

… properly (#9958) **Related Issue:** #9641, #9315 ## Summary This addresses some issues that prevented `beforeOpen`/`open` and `beforeClose`/`close` events from emitting as expected. ### Notable changes: * Removes redundant/unused transitions in favor of ones used for open/close transitions * Updates `input-date-picker` to set `open` via a prop instead of a attribute to avoid the transition starting before `onToggleOpenCloseComponent` was called. We could refactor this if `popover` and similar components use a different, internal, attribute to control transitions. * Updates tests that would now have a transition when opening/closing * Remove unnecessary `OpenCloseComponent` implementation from `input-time-picker` as it can delegate to the internal `popover`. * Updates `block` to use on margin transition to determine when it is open or closed. I will create a refactor issue to refactor the open/close transition, focusing on the content, and will update the implementation to use this instead. * Refactored `notice`'s styles to properly transition when opened/closed. * Added missing `openClose` tests to `sheet` and `tooltip` E2E test suites. * Added option to `openClose` test helper for tests that are expected to use the fallback due to the normal transition start/end events not being able to fire properly. * Improved `openClose` test helper to consider timing between beforeOpen/Close and open/close events. * Fixed `whenTransitionOrAnimationDone` issue that prevented the correct duration to be associated with the open/close transition.
Esri · Aug 14, 2024 · 7e2764f · 7e2764f
1 parent 259282b
commit 7e2764f
Show file tree

Hide file tree

Showing 14 changed files with 166 additions and 114 deletions.
diff --git a/packages/calcite-components/src/components/alert/alert.scss b/packages/calcite-components/src/components/alert/alert.scss
@@ -38,7 +38,6 @@ $border-style: 1px solid var(--calcite-color-border-3);
   inline-size: var(--calcite-alert-width);
   max-inline-size: calc(100% - (var(--calcite-alert-edge-distance) * 2));
   transition:
-    var(--calcite-internal-animation-timing-slow) $easing-function,
     opacity var(--calcite-internal-animation-timing-slow) $easing-function,
     all var(--calcite-animation-timing) ease-in-out;
 

diff --git a/packages/calcite-components/src/components/block/block.e2e.ts b/packages/calcite-components/src/components/block/block.e2e.ts
@@ -13,6 +13,7 @@ import {
 } from "../../tests/commonTests";
 import { html } from "../../../support/formatting";
 import { openClose } from "../../tests/commonTests";
+import { skipAnimations } from "../../tests/utils";
 import { CSS, SLOTS } from "./resources";
 
 describe("calcite-block", () => {
@@ -189,6 +190,7 @@ describe("calcite-block", () => {
     const heading = "heading";
     const page = await newE2EPage();
     await page.setContent(html`<calcite-block collapsible heading=${heading}></calcite-block>`);
+    await skipAnimations(page);
     const messages = await import(`./assets/block/t9n/messages.json`);
 
     const element = await page.find("calcite-block");
@@ -251,6 +253,7 @@ describe("calcite-block", () => {
           <div class="nested-control" tabindex="0" slot=${SLOTS.control}>fake space/enter-bubbling control</div>
         </calcite-block>
       `);
+      await skipAnimations(page);
       const control = await page.find(".nested-control");
       expect(await control.isVisible()).toBe(true);
 

diff --git a/packages/calcite-components/src/components/block/block.scss b/packages/calcite-components/src/components/block/block.scss
@@ -10,8 +10,9 @@
   @extend %component-host;
   @extend %component-spacing;
   @apply transition-margin ease-cubic border-color-3 flex flex-shrink-0 flex-grow-0
-    flex-col border-0 border-b border-solid p-0 duration-150;
+    flex-col border-0 border-b border-solid p-0;
   flex-basis: auto;
+  transition-duration: var(--calcite-animation-timing);
 }
 
 @include disabled();

diff --git a/packages/calcite-components/src/components/block/block.tsx b/packages/calcite-components/src/components/block/block.tsx
@@ -224,7 +224,7 @@ export class Block
 
   @State() hasEndActions = false;
 
-  openTransitionProp = "opacity";
+  openTransitionProp = "margin-top";
 
   transitionEl: HTMLElement;
 
@@ -239,6 +239,8 @@ export class Block
     connectInteractive(this);
     connectLocalized(this);
     connectMessages(this);
+
+    this.transitionEl = this.el;
   }
 
   disconnectedCallback(): void {
@@ -301,10 +303,6 @@ export class Block
     this.calciteBlockToggle.emit();
   };
 
-  private setTransitionEl = (el: HTMLElement): void => {
-    this.transitionEl = el;
-  };
-
   private actionsEndSlotChangeHandler = (event: Event): void => {
     this.hasEndActions = slotChangeHasAssignedElement(event);
   };
@@ -490,7 +488,6 @@ export class Block
               class={CSS.content}
               hidden={!open}
               id={IDS.content}
-              ref={this.setTransitionEl}
             >
               {this.renderScrim()}
             </section>

diff --git a/packages/calcite-components/src/components/dialog/dialog.e2e.ts b/packages/calcite-components/src/components/dialog/dialog.e2e.ts
@@ -29,12 +29,13 @@ describe("calcite-dialog", () => {
   });
 
   describe("openClose", () => {
-    const openCloseOptions = {
-      initialToggleValue: true,
-    };
-
     openClose("calcite-dialog");
-    openClose("calcite-dialog", openCloseOptions);
+
+    describe("initially open", () => {
+      openClose("calcite-dialog", {
+        initialToggleValue: true,
+      });
+    });
   });
 
   describe("slots", () => {

diff --git a/packages/calcite-components/src/components/input-time-picker/input-time-picker.e2e.ts b/packages/calcite-components/src/components/input-time-picker/input-time-picker.e2e.ts
@@ -101,6 +101,10 @@ describe("calcite-input-time-picker", () => {
 
   describe("openClose", () => {
     openClose("calcite-input-time-picker");
+
+    describe("initially open", () => {
+      openClose("calcite-input-time-picker", { initialToggleValue: true });
+    });
   });
 
   it("when set to readOnly, element still focusable but won't display the controls or allow for changing the value", async () => {
@@ -859,8 +863,8 @@ describe("calcite-input-time-picker", () => {
         html`<calcite-input-time-picker></calcite-input-time-picker>
           <div id="next-sibling" tabindex="0">next sibling</div>`,
       );
+      await skipAnimations(page);
       const popover = await page.find("calcite-input-time-picker >>> calcite-popover");
-      const stopgapDelayUntilOpenCloseEventsAreImplemented = 500;
 
       await page.keyboard.press("Tab");
       expect(await getFocusedElementProp(page, "tagName")).toBe("CALCITE-INPUT-TIME-PICKER");
@@ -876,7 +880,6 @@ describe("calcite-input-time-picker", () => {
 
       await page.keyboard.press("ArrowDown");
       await page.waitForChanges();
-      await page.waitForTimeout(stopgapDelayUntilOpenCloseEventsAreImplemented);
 
       expect(await popover.isVisible()).toBe(true);
       expect(await getFocusedElementProp(page, "tagName", { shadow: true })).toBe("CALCITE-TIME-PICKER");
@@ -891,7 +894,6 @@ describe("calcite-input-time-picker", () => {
 
       await page.keyboard.press("Escape");
       await page.waitForChanges();
-      await page.waitForTimeout(stopgapDelayUntilOpenCloseEventsAreImplemented);
 
       expect(await popover.isVisible()).toBe(false);
       expect(await getFocusedElementProp(page, "tagName")).toBe("CALCITE-INPUT-TIME-PICKER");

diff --git a/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx b/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx
@@ -75,7 +75,6 @@ import {
   updateMessages,
 } from "../../utils/t9n";
 import { getSupportedLocale } from "../../utils/locale";
-import { onToggleOpenCloseComponent, OpenCloseComponent } from "../../utils/openCloseComponent";
 import { decimalPlaces } from "../../utils/math";
 import { getIconScale } from "../../utils/component";
 import { Validation } from "../functional/Validation";
@@ -169,7 +168,6 @@ export class InputTimePicker
     LabelableComponent,
     LoadableComponent,
     LocalizedComponent,
-    OpenCloseComponent,
     T9nComponent
 {
   //--------------------------------------------------------------------------
@@ -179,19 +177,17 @@ export class InputTimePicker
   //--------------------------------------------------------------------------
 
   /** When `true`, displays the `calcite-time-picker` component. */
-
   @Prop({ reflect: true, mutable: true }) open = false;
 
   @Watch("open")
   openHandler(): void {
-    onToggleOpenCloseComponent(this);
-
     if (this.disabled || this.readOnly) {
       this.open = false;
       return;
     }
 
-    this.reposition(true);
+    // we set the property instead of the attribute to ensure popover's open/close events are emitted properly
+    this.popoverEl.open = this.open;
   }
 
   /** When `true`, interaction is prevented and the component is displayed with lower opacity. */
@@ -395,10 +391,6 @@ export class InputTimePicker
 
   private referenceElementId = `input-time-picker-${guid()}`;
 
-  openTransitionProp = "opacity";
-
-  transitionEl: HTMLCalciteInputElement;
-
   //--------------------------------------------------------------------------
   //
   //  State
@@ -559,21 +551,42 @@ export class InputTimePicker
   //
   // --------------------------------------------------------------------------
 
-  onBeforeOpen(): void {
+  private popoverBeforeOpenHandler = (event: CustomEvent<void>): void => {
+    event.stopPropagation();
     this.calciteInputTimePickerBeforeOpen.emit();
-  }
+  };
 
-  onOpen(): void {
+  private popoverOpenHandler = (event: CustomEvent<void>): void => {
+    event.stopPropagation();
     this.calciteInputTimePickerOpen.emit();
-  }
 
-  onBeforeClose(): void {
+    activateFocusTrap(this, {
+      onActivate: () => {
+        if (this.focusOnOpen) {
+          this.calciteTimePickerEl.setFocus();
+          this.focusOnOpen = false;
+        }
+      },
+    });
+  };
+
+  private popoverBeforeCloseHandler = (event: CustomEvent<void>): void => {
+    event.stopPropagation();
     this.calciteInputTimePickerBeforeClose.emit();
-  }
+  };
 
-  onClose(): void {
+  private popoverCloseHandler = (event: CustomEvent<void>): void => {
+    event.stopPropagation();
     this.calciteInputTimePickerClose.emit();
-  }
+
+    deactivateFocusTrap(this, {
+      onDeactivate: () => {
+        this.calciteInputEl.setFocus();
+        this.focusOnOpen = false;
+      },
+    });
+    this.open = false;
+  };
 
   syncHiddenFormInput(input: HTMLInputElement): void {
     syncHiddenFormInput("time", this, input);
@@ -685,27 +698,6 @@ export class InputTimePicker
     return timeString;
   }
 
-  private popoverCloseHandler = () => {
-    deactivateFocusTrap(this, {
-      onDeactivate: () => {
-        this.calciteInputEl.setFocus();
-        this.focusOnOpen = false;
-      },
-    });
-    this.open = false;
-  };
-
-  private popoverOpenHandler = () => {
-    activateFocusTrap(this, {
-      onActivate: () => {
-        if (this.focusOnOpen) {
-          this.calciteTimePickerEl.setFocus();
-          this.focusOnOpen = false;
-        }
-      },
-    });
-  };
-
   keyDownHandler = (event: KeyboardEvent): void => {
     const { defaultPrevented, key } = event;
 
@@ -867,9 +859,8 @@ export class InputTimePicker
     this.popoverEl = el;
   };
 
-  private setInputAndTransitionEl = (el: HTMLCalciteInputElement): void => {
+  private setInputEl = (el: HTMLCalciteInputElement): void => {
     this.calciteInputEl = el;
-    this.transitionEl = el;
   };
 
   private setCalciteTimePickerEl = (el: HTMLCalciteTimePickerElement): void => {
@@ -978,9 +969,6 @@ export class InputTimePicker
   async componentWillLoad(): Promise<void> {
     setUpLoadableComponent(this);
     await Promise.all([setUpMessages(this), this.loadDateTimeLocaleData()]);
-    if (this.open) {
-      onToggleOpenCloseComponent(this);
-    }
   }
 
   componentDidLoad() {
@@ -996,6 +984,8 @@ export class InputTimePicker
         }),
       );
     }
+
+    this.openHandler();
   }
 
   disconnectedCallback() {
@@ -1034,7 +1024,7 @@ export class InputTimePicker
               onCalciteInputTextInput={this.calciteInternalInputInputHandler}
               onCalciteInternalInputTextFocus={this.calciteInternalInputFocusHandler}
               readOnly={readOnly}
-              ref={this.setInputAndTransitionEl}
+              ref={this.setInputEl}
               role="combobox"
               scale={this.scale}
               status={this.status}
@@ -1046,9 +1036,10 @@ export class InputTimePicker
             id={dialogId}
             label={messages.chooseTime}
             lang={this.effectiveLocale}
+            onCalcitePopoverBeforeClose={this.popoverBeforeCloseHandler}
+            onCalcitePopoverBeforeOpen={this.popoverBeforeOpenHandler}
             onCalcitePopoverClose={this.popoverCloseHandler}
             onCalcitePopoverOpen={this.popoverOpenHandler}
-            open={this.open}
             overlayPositioning={this.overlayPositioning}
             placement={this.placement}
             ref={this.setCalcitePopoverEl}

diff --git a/packages/calcite-components/src/components/modal/modal.e2e.ts b/packages/calcite-components/src/components/modal/modal.e2e.ts
@@ -14,12 +14,13 @@ describe("calcite-modal", () => {
   });
 
   describe("openClose", () => {
-    const openCloseOptions = {
-      initialToggleValue: true,
-    };
-
     openClose("calcite-modal");
-    openClose("calcite-modal", openCloseOptions);
+
+    describe("initially open", () => {
+      openClose("calcite-modal", {
+        initialToggleValue: true,
+      });
+    });
   });
 
   describe("slots", () => {

diff --git a/packages/calcite-components/src/components/notice/notice.scss b/packages/calcite-components/src/components/notice/notice.scss
@@ -75,13 +75,14 @@
     pointer-events-none
     my-0
     box-border
-    hidden
+    flex
     w-full
-    opacity-0
-    transition-default;
+    opacity-0;
   max-block-size: 0;
+  transition-property: opacity, max-block-size;
+  transition-duration: var(--calcite-animation-timing);
   text-align: start;
-  border-inline-start: 0 solid;
+  border-inline-start: var(--calcite-border-width-md) solid;
   box-shadow: 0 0 0 0 transparent;
 }
 
@@ -100,10 +101,8 @@
 :host([open]) .container {
   @apply shadow-1
     pointer-events-auto
-    flex
     max-h-full
     items-center
-    border-2
     opacity-100;
 }
 

diff --git a/packages/calcite-components/src/components/sheet/sheet.e2e.ts b/packages/calcite-components/src/components/sheet/sheet.e2e.ts
@@ -1,6 +1,6 @@
 import { newE2EPage } from "@stencil/core/testing";
 import { html } from "../../../support/formatting";
-import { focusable, renders, hidden, defaults, accessible } from "../../tests/commonTests";
+import { accessible, defaults, focusable, hidden, openClose, renders } from "../../tests/commonTests";
 import { GlobalTestProps, newProgrammaticE2EPage, skipAnimations } from "../../tests/utils";
 import { CSS } from "./resources";
 
@@ -79,6 +79,16 @@ describe("calcite-sheet properties", () => {
     });
   });
 
+  describe("openClose", () => {
+    openClose("calcite-sheet");
+
+    describe("initially open", () => {
+      openClose("calcite-sheet", {
+        initialToggleValue: true,
+      });
+    });
+  });
+
   it("sets custom width correctly", async () => {
     const page = await newE2EPage();
     // set large page to ensure test sheet isn't becoming fullscreen