fix(combobox, dropdown, input-date-picker, popover, tooltip): fix mis…

…placed floating-ui elements when associated-components are closed (#6709) **Related Issue:** #6404 ## Summary This removes a utility added in #5484 that would reset position of floating-UI elements using the absolute strategy to avoid scroll bars when hidden. Unfortunately, the util would lead to undesired position resets in some scenarios (e.g., quick toggling between transitions). Moving forward, it is recommended to use `overlay-positioning='fixed'`to escaping clipping containers and prevent hidden floating-ui elements from affecting layout/creating scrollbars.
Esri · Apr 13, 2023 · e220686 · e220686
1 parent 40bb711
commit e220686
Show file tree

Hide file tree

Showing 7 changed files with 14 additions and 148 deletions.
diff --git a/src/components/combobox/combobox.tsx b/src/components/combobox/combobox.tsx
@@ -26,8 +26,7 @@ import {
   FloatingUIComponent,
   LogicalPlacement,
   OverlayPositioning,
-  reposition,
-  updateAfterClose
+  reposition
 } from "../../utils/floating-ui";
 import {
   afterConnectDefaultValueSet,
@@ -116,11 +115,7 @@ export class Combobox
   @Prop({ reflect: true, mutable: true }) open = false;
 
   @Watch("open")
-  openHandler(value: boolean): void {
-    if (!value) {
-      updateAfterClose(this.floatingEl);
-    }
-
+  openHandler(): void {
     if (this.disabled) {
       this.open = false;
       return;
@@ -398,7 +393,7 @@ export class Combobox
     this.setFilteredPlacements();
     this.reposition(true);
     if (this.open) {
-      this.openHandler(this.open);
+      this.openHandler();
     }
   }
 

diff --git a/src/components/dropdown/dropdown.tsx b/src/components/dropdown/dropdown.tsx
@@ -29,8 +29,7 @@ import {
   FloatingUIComponent,
   MenuPlacement,
   OverlayPositioning,
-  reposition,
-  updateAfterClose
+  reposition
 } from "../../utils/floating-ui";
 import { guid } from "../../utils/guid";
 import { InteractiveComponent, updateHostInteraction } from "../../utils/interactive";
@@ -89,16 +88,10 @@ export class Dropdown
     if (!this.disabled) {
       if (value) {
         this.reposition(true);
-      } else {
-        updateAfterClose(this.floatingEl);
       }
       return;
     }
 
-    if (!value) {
-      updateAfterClose(this.floatingEl);
-    }
-
     this.open = false;
   }
 

diff --git a/src/components/input-date-picker/input-date-picker.tsx b/src/components/input-date-picker/input-date-picker.tsx
@@ -32,8 +32,7 @@ import {
   FloatingUIComponent,
   MenuPlacement,
   OverlayPositioning,
-  reposition,
-  updateAfterClose
+  reposition
 } from "../../utils/floating-ui";
 import {
   connectForm,
@@ -214,17 +213,12 @@ export class InputDatePicker
   @Watch("open")
   openHandler(value: boolean): void {
     if (this.disabled || this.readOnly) {
-      if (!value) {
-        updateAfterClose(this.floatingEl);
-      }
       this.open = false;
       return;
     }
 
     if (value) {
       this.reposition(true);
-    } else {
-      updateAfterClose(this.floatingEl);
     }
   }
 

diff --git a/src/components/popover/popover.tsx b/src/components/popover/popover.tsx
@@ -23,8 +23,7 @@ import {
   LogicalPlacement,
   OverlayPositioning,
   ReferenceElement,
-  reposition,
-  updateAfterClose
+  reposition
 } from "../../utils/floating-ui";
 import {
   activateFocusTrap,
@@ -197,8 +196,6 @@ export class Popover
   openHandler(value: boolean): void {
     if (value) {
       this.reposition(true);
-    } else {
-      updateAfterClose(this.el);
     }
 
     this.setExpandedAttr();

diff --git a/src/components/tooltip/tooltip.tsx b/src/components/tooltip/tooltip.tsx
@@ -21,8 +21,7 @@ import {
   LogicalPlacement,
   OverlayPositioning,
   ReferenceElement,
-  reposition,
-  updateAfterClose
+  reposition
 } from "../../utils/floating-ui";
 import { guid } from "../../utils/guid";
 import {
@@ -89,8 +88,6 @@ export class Tooltip implements FloatingUIComponent, OpenCloseComponent {
   openHandler(value: boolean): void {
     if (value) {
       this.reposition(true);
-    } else {
-      updateAfterClose(this.el);
     }
   }
 

diff --git a/src/utils/floating-ui.spec.ts b/src/utils/floating-ui.spec.ts
@@ -6,15 +6,12 @@ import {
   disconnectFloatingUI,
   effectivePlacements,
   filterComputedPlacements,
-  FloatingCSS,
   FloatingUIComponent,
   getEffectivePlacement,
-  placementDataAttribute,
   placements,
   positionFloatingUI,
   reposition,
-  repositionDebounceTimeout,
-  updateAfterClose
+  repositionDebounceTimeout
 } from "./floating-ui";
 
 import * as floatingUIDOM from "@floating-ui/dom";
@@ -158,63 +155,6 @@ describe("repositioning", () => {
       expect(floatingEl.style.position).toBe("fixed");
     });
   });
-
-  describe("afterClose helper", () => {
-    beforeAll(() => {
-      class TransitionEvent extends globalThis.Event {
-        readonly elapsedTime: number;
-
-        readonly propertyName: string;
-
-        readonly pseudoElement: string;
-
-        constructor(type: string, transitionEventInitDict: TransitionEventInit = {}) {
-          super(type, transitionEventInitDict);
-
-          this.elapsedTime = transitionEventInitDict.elapsedTime || 0.0;
-          this.propertyName = transitionEventInitDict.propertyName || "";
-          this.pseudoElement = transitionEventInitDict.pseudoElement || "";
-        }
-      }
-
-      // polyfilled as it is not available via JSDOM
-      globalThis.TransitionEvent = TransitionEvent;
-    });
-
-    describe("resets positioning when closed", () => {
-      function emitTransitionEnd() {
-        const closingFloatingUITransitionEvent = new TransitionEvent("transitionend", { propertyName: "opacity" });
-        floatingEl.dispatchEvent(closingFloatingUITransitionEvent);
-      }
-
-      beforeEach(() => {
-        floatingEl.setAttribute(placementDataAttribute, "fake-placement");
-        floatingEl.classList.add(FloatingCSS.animation);
-      });
-
-      it("resets for absolute positioning strategy", async () => {
-        floatingEl.style.position = "absolute";
-
-        updateAfterClose(floatingEl);
-        emitTransitionEnd();
-
-        expect(floatingEl.style.transform).toBe("");
-        expect(floatingEl.style.top).toBe("0");
-        expect(floatingEl.style.left).toBe("0");
-      });
-
-      it("does not reset for fixed positioning strategy", async () => {
-        floatingEl.style.position = "fixed";
-
-        updateAfterClose(floatingEl);
-        emitTransitionEnd();
-
-        expect(floatingEl.style.transform).toBe("");
-        expect(floatingEl.style.top).not.toBe("0");
-        expect(floatingEl.style.left).not.toBe("0");
-      });
-    });
-  });
 });
 
 it("should have correct value for defaultOffsetDistance", () => {

diff --git a/src/utils/floating-ui.ts b/src/utils/floating-ui.ts
@@ -16,7 +16,7 @@ import {
 import { Build } from "@stencil/core";
 import { debounce } from "lodash-es";
 import { config } from "./config";
-import { closestElementCrossShadowBoundary, getElementDir } from "./dom";
+import { getElementDir } from "./dom";
 
 const floatingUIBrowserCheck = patchFloatingUiForNonChromiumBrowsers();
 
@@ -451,18 +451,15 @@ export function connectFloatingUI(
 
   disconnectFloatingUI(component, referenceEl, floatingEl);
 
-  const position = component.overlayPositioning;
-
-  // ensure position matches for initial positioning
   Object.assign(floatingEl.style, {
     visibility: "hidden",
     pointerEvents: "none",
-    position
-  });
 
-  if (position === "absolute") {
-    resetPosition(floatingEl);
-  }
+    // initial positioning based on https://floating-ui.com/docs/computePosition#initial-layout
+    position: component.overlayPositioning,
+    top: "0",
+    left: "0"
+  });
 
   const runAutoUpdate = Build.isBrowser
     ? autoUpdate
@@ -495,8 +492,6 @@ export function disconnectFloatingUI(
     return;
   }
 
-  getTransitionTarget(floatingEl).removeEventListener("transitionend", handleTransitionElTransitionEnd);
-
   const cleanup = cleanupMap.get(component);
 
   if (cleanup) {
@@ -514,48 +509,3 @@ const visiblePointerSize = 4;
  * @default 6
  */
 export const defaultOffsetDistance = Math.ceil(Math.hypot(visiblePointerSize, visiblePointerSize));
-
-/**
- * This utils applies floating element styles to avoid affecting layout when closed.
- *
- * This should be called when the closing transition will start.
- *
- * @param floatingEl
- */
-export function updateAfterClose(floatingEl: HTMLElement): void {
-  if (!floatingEl || floatingEl.style.position !== "absolute") {
-    return;
-  }
-
-  getTransitionTarget(floatingEl).addEventListener("transitionend", handleTransitionElTransitionEnd);
-}
-
-function getTransitionTarget(floatingEl: HTMLElement): ShadowRoot | HTMLElement {
-  // assumes floatingEl w/ shadowRoot is a FloatingUIComponent
-  return floatingEl.shadowRoot || floatingEl;
-}
-
-function handleTransitionElTransitionEnd(event: TransitionEvent): void {
-  const floatingTransitionEl = event.target as HTMLElement;
-
-  if (
-    // using any prop from floating-ui transition
-    event.propertyName === "opacity" &&
-    floatingTransitionEl.classList.contains(FloatingCSS.animation)
-  ) {
-    const floatingEl = getFloatingElFromTransitionTarget(floatingTransitionEl);
-    resetPosition(floatingEl);
-    getTransitionTarget(floatingEl).removeEventListener("transitionend", handleTransitionElTransitionEnd);
-  }
-}
-
-function resetPosition(floatingEl: HTMLElement): void {
-  // resets position to better match https://floating-ui.com/docs/computePosition#initial-layout
-  floatingEl.style.transform = "";
-  floatingEl.style.top = "0";
-  floatingEl.style.left = "0";
-}
-
-function getFloatingElFromTransitionTarget(floatingTransitionEl: HTMLElement): HTMLElement {
-  return closestElementCrossShadowBoundary(floatingTransitionEl, `[${placementDataAttribute}]`);
-}