From e220686fa366b86dda2647da25144eb45a4a193d Mon Sep 17 00:00:00 2001 From: JC Franco Date: Thu, 13 Apr 2023 11:34:17 -0700 Subject: [PATCH] fix(combobox, dropdown, input-date-picker, popover, tooltip): fix misplaced floating-ui elements when associated-components are closed (#6709) **Related Issue:** #6404 ## Summary This removes a utility added in https://github.com/Esri/calcite-components/pull/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. --- src/components/combobox/combobox.tsx | 11 +--- src/components/dropdown/dropdown.tsx | 9 +-- .../input-date-picker/input-date-picker.tsx | 8 +-- src/components/popover/popover.tsx | 5 +- src/components/tooltip/tooltip.tsx | 5 +- src/utils/floating-ui.spec.ts | 62 +------------------ src/utils/floating-ui.ts | 62 ++----------------- 7 files changed, 14 insertions(+), 148 deletions(-) diff --git a/src/components/combobox/combobox.tsx b/src/components/combobox/combobox.tsx index a7a8233dac0..71e14246304 100644 --- 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 index 538cf6ce875..ff57bb20989 100644 --- 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 index 1a53e6d501b..2d219b05c76 100644 --- 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 index b5f47fa37bc..32142a17b82 100644 --- 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 index d88d87d415c..fe224f04315 100644 --- 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 index d003773a580..21cc07d8bd9 100644 --- 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 index bf82621b6f3..05952e12fc8 100644 --- 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}]`); -}