From fd9c90e5c60cbb90a74307ecbddb0515a5e4de57 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Wed, 22 May 2024 10:39:48 -0700 Subject: [PATCH 1/4] fix: defer floating-ui updating until component is connected and open --- .../src/components/combobox/combobox.tsx | 6 +- .../src/components/dropdown/dropdown.tsx | 6 +- .../input-date-picker/input-date-picker.tsx | 8 +- .../src/components/popover/popover.tsx | 18 +-- .../src/components/tooltip/tooltip.tsx | 19 +-- .../src/utils/floating-ui.spec.ts | 127 +++++++++++------- .../src/utils/floating-ui.ts | 94 ++++++++++--- 7 files changed, 177 insertions(+), 101 deletions(-) diff --git a/packages/calcite-components/src/components/combobox/combobox.tsx b/packages/calcite-components/src/components/combobox/combobox.tsx index ab7a18a61df..a56f49ceada 100644 --- a/packages/calcite-components/src/components/combobox/combobox.tsx +++ b/packages/calcite-components/src/components/combobox/combobox.tsx @@ -452,7 +452,7 @@ export class Combobox // // -------------------------------------------------------------------------- - connectedCallback(): void { + async connectedCallback(): Promise { connectInteractive(this); connectLocalized(this); connectMessages(this); @@ -472,7 +472,9 @@ export class Combobox onToggleOpenCloseComponent(this); } + await componentOnReady(this.el); connectFloatingUI(this, this.referenceEl, this.floatingEl); + afterConnectDefaultValueSet(this, this.getValue()); } async componentWillLoad(): Promise { @@ -482,8 +484,6 @@ export class Combobox } componentDidLoad(): void { - afterConnectDefaultValueSet(this, this.getValue()); - connectFloatingUI(this, this.referenceEl, this.floatingEl); setComponentLoaded(this); } diff --git a/packages/calcite-components/src/components/dropdown/dropdown.tsx b/packages/calcite-components/src/components/dropdown/dropdown.tsx index 6d501d1a42d..d9fcdcb81e3 100644 --- a/packages/calcite-components/src/components/dropdown/dropdown.tsx +++ b/packages/calcite-components/src/components/dropdown/dropdown.tsx @@ -43,6 +43,7 @@ import { createObserver } from "../../utils/observers"; import { onToggleOpenCloseComponent, OpenCloseComponent } from "../../utils/openCloseComponent"; import { RequestedItem } from "../dropdown-group/interfaces"; import { Scale } from "../interfaces"; +import { componentOnReady } from "../../utils/component"; import { ItemKeyboardEvent } from "./interfaces"; import { SLOTS } from "./resources"; @@ -196,7 +197,7 @@ export class Dropdown // //-------------------------------------------------------------------------- - connectedCallback(): void { + async connectedCallback(): Promise { this.mutationObserver?.observe(this.el, { childList: true, subtree: true }); this.setFilteredPlacements(); if (this.open) { @@ -205,6 +206,8 @@ export class Dropdown } connectInteractive(this); this.updateItems(); + + await componentOnReady(this.el); connectFloatingUI(this, this.referenceEl, this.floatingEl); } @@ -214,7 +217,6 @@ export class Dropdown componentDidLoad(): void { setComponentLoaded(this); - connectFloatingUI(this, this.referenceEl, this.floatingEl); } componentDidRender(): void { diff --git a/packages/calcite-components/src/components/input-date-picker/input-date-picker.tsx b/packages/calcite-components/src/components/input-date-picker/input-date-picker.tsx index f75b9f47964..01674e4f7e7 100644 --- a/packages/calcite-components/src/components/input-date-picker/input-date-picker.tsx +++ b/packages/calcite-components/src/components/input-date-picker/input-date-picker.tsx @@ -79,7 +79,7 @@ import { FocusTrapComponent, } from "../../utils/focusTrapComponent"; import { guid } from "../../utils/guid"; -import { getIconScale } from "../../utils/component"; +import { componentOnReady, getIconScale } from "../../utils/component"; import { Status } from "../interfaces"; import { Validation } from "../functional/Validation"; import { normalizeToCurrentCentury, isTwoDigitYear } from "./utils"; @@ -459,7 +459,7 @@ export class InputDatePicker // // -------------------------------------------------------------------------- - connectedCallback(): void { + async connectedCallback(): Promise { connectInteractive(this); connectLocalized(this); @@ -506,7 +506,9 @@ export class InputDatePicker onToggleOpenCloseComponent(this); } + await componentOnReady(this.el); connectFloatingUI(this, this.referenceEl, this.floatingEl); + this.localizeInputValues(); } async componentWillLoad(): Promise { @@ -518,8 +520,6 @@ export class InputDatePicker componentDidLoad(): void { setComponentLoaded(this); - this.localizeInputValues(); - connectFloatingUI(this, this.referenceEl, this.floatingEl); } disconnectedCallback(): void { diff --git a/packages/calcite-components/src/components/popover/popover.tsx b/packages/calcite-components/src/components/popover/popover.tsx index aeb55416ef2..3761ddbe25e 100644 --- a/packages/calcite-components/src/components/popover/popover.tsx +++ b/packages/calcite-components/src/components/popover/popover.tsx @@ -55,7 +55,7 @@ import { } from "../../utils/loadable"; import { createObserver } from "../../utils/observers"; import { FloatingArrow } from "../functional/FloatingArrow"; -import { getIconScale } from "../../utils/component"; +import { componentOnReady, getIconScale } from "../../utils/component"; import PopoverManager from "./PopoverManager"; import { PopoverMessages } from "./assets/popover/t9n"; import { ARIA_CONTROLS, ARIA_EXPANDED, CSS, defaultPopoverPlacement } from "./resources"; @@ -278,8 +278,6 @@ export class Popover transitionEl: HTMLDivElement; - hasLoaded = false; - focusTrap: FocusTrap; // -------------------------------------------------------------------------- @@ -288,16 +286,19 @@ export class Popover // // -------------------------------------------------------------------------- - connectedCallback(): void { + async connectedCallback(): Promise { this.setFilteredPlacements(); connectLocalized(this); connectMessages(this); - this.setUpReferenceElement(this.hasLoaded); + + await componentOnReady(this.el); + this.setUpReferenceElement(); connectFocusTrap(this); + connectFloatingUI(this, this.effectiveReferenceElement, this.el); + if (this.open) { onToggleOpenCloseComponent(this); } - connectFloatingUI(this, this.effectiveReferenceElement, this.el); } async componentWillLoad(): Promise { @@ -307,11 +308,6 @@ export class Popover componentDidLoad(): void { setComponentLoaded(this); - if (this.referenceElement && !this.effectiveReferenceElement) { - this.setUpReferenceElement(); - } - connectFloatingUI(this, this.effectiveReferenceElement, this.el); - this.hasLoaded = true; } disconnectedCallback(): void { diff --git a/packages/calcite-components/src/components/tooltip/tooltip.tsx b/packages/calcite-components/src/components/tooltip/tooltip.tsx index caeae89d968..0d2015a92dd 100644 --- a/packages/calcite-components/src/components/tooltip/tooltip.tsx +++ b/packages/calcite-components/src/components/tooltip/tooltip.tsx @@ -27,6 +27,7 @@ import { import { guid } from "../../utils/guid"; import { onToggleOpenCloseComponent, OpenCloseComponent } from "../../utils/openCloseComponent"; import { FloatingArrow } from "../functional/FloatingArrow"; +import { componentOnReady } from "../../utils/component"; import { ARIA_DESCRIBED_BY, CSS } from "./resources"; import TooltipManager from "./TooltipManager"; import { getEffectiveReferenceElement } from "./utils"; @@ -146,8 +147,6 @@ export class Tooltip implements FloatingUIComponent, OpenCloseComponent { guid = `calcite-tooltip-${guid()}`; - hasLoaded = false; - openTransitionProp = "opacity"; transitionEl: HTMLDivElement; @@ -158,12 +157,14 @@ export class Tooltip implements FloatingUIComponent, OpenCloseComponent { // // -------------------------------------------------------------------------- - connectedCallback(): void { - this.setUpReferenceElement(this.hasLoaded); + async connectedCallback(): Promise { + await componentOnReady(this.el); + this.setUpReferenceElement(true); + connectFloatingUI(this, this.effectiveReferenceElement, this.el); + if (this.open) { onToggleOpenCloseComponent(this); } - connectFloatingUI(this, this.effectiveReferenceElement, this.el); } async componentWillLoad(): Promise { @@ -172,14 +173,6 @@ export class Tooltip implements FloatingUIComponent, OpenCloseComponent { } } - componentDidLoad(): void { - if (this.referenceElement && !this.effectiveReferenceElement) { - this.setUpReferenceElement(); - } - connectFloatingUI(this, this.effectiveReferenceElement, this.el); - this.hasLoaded = true; - } - disconnectedCallback(): void { this.removeReferences(); disconnectFloatingUI(this, this.effectiveReferenceElement, this.el); diff --git a/packages/calcite-components/src/utils/floating-ui.spec.ts b/packages/calcite-components/src/utils/floating-ui.spec.ts index dd65a4bf420..6069b8f3350 100644 --- a/packages/calcite-components/src/utils/floating-ui.spec.ts +++ b/packages/calcite-components/src/utils/floating-ui.spec.ts @@ -3,7 +3,7 @@ import * as floatingUI from "./floating-ui"; import { FloatingUIComponent } from "./floating-ui"; const { - cleanupMap, + autoUpdatingComponentMap, connectFloatingUI, defaultOffsetDistance, disconnectFloatingUI, @@ -36,28 +36,40 @@ it("should set calcite placement to FloatingUI placement", () => { expect(getEffectivePlacement(el, "trailing-end")).toBe("left-end"); }); +function createFakeFloatingUiComponent(referenceEl: HTMLElement, floatingEl: HTMLElement): FloatingUIComponent { + const fake: FloatingUIComponent = { + open: false, + reposition: async () => { + await reposition(fake, { + floatingEl, + referenceEl, + overlayPositioning: fake.overlayPositioning, + placement: "top", + flipPlacements: [], + type: "menu", + }); + }, + overlayPositioning: "absolute", + placement: "auto", + }; + + return fake; +} + describe("repositioning", () => { let fakeFloatingUiComponent: FloatingUIComponent; let floatingEl: HTMLDivElement; let referenceEl: HTMLButtonElement; let positionOptions: Parameters[1]; - function createFakeFloatingUiComponent(): FloatingUIComponent { - return { - open: false, - reposition: async () => { - /* noop */ - }, - overlayPositioning: "absolute", - placement: "auto", - }; - } - beforeEach(() => { - fakeFloatingUiComponent = createFakeFloatingUiComponent(); - - floatingEl = document.createElement("div"); referenceEl = document.createElement("button"); + floatingEl = document.createElement("div"); + + document.body.append(floatingEl); + document.body.append(referenceEl); + + fakeFloatingUiComponent = createFakeFloatingUiComponent(referenceEl, floatingEl); positionOptions = { floatingEl, @@ -66,6 +78,8 @@ describe("repositioning", () => { placement: fakeFloatingUiComponent.placement, type: "popover", }; + + connectFloatingUI(fakeFloatingUiComponent, referenceEl, floatingEl); }); function assertPreOpenPositioning(floatingEl: HTMLElement): void { @@ -112,55 +126,70 @@ describe("repositioning", () => { assertOpenPositioning(floatingEl); }); - describe("connect/disconnect helpers", () => { - it("has connectedCallback and disconnectedCallback helpers", () => { - expect(cleanupMap.has(fakeFloatingUiComponent)).toBe(false); - expect(floatingEl.style.position).toBe(""); - expect(floatingEl.style.visibility).toBe(""); - expect(floatingEl.style.pointerEvents).toBe(""); + it("debounces positioning per instance", async () => { + const positionSpy = jest.spyOn(floatingUI, "positionFloatingUI"); + fakeFloatingUiComponent.open = true; - connectFloatingUI(fakeFloatingUiComponent, referenceEl, floatingEl); + const anotherFakeFloatingUiComponent = createFakeFloatingUiComponent(referenceEl, floatingEl); + anotherFakeFloatingUiComponent.open = true; - expect(cleanupMap.has(fakeFloatingUiComponent)).toBe(true); - expect(floatingEl.style.position).toBe("absolute"); - expect(floatingEl.style.visibility).toBe("hidden"); - expect(floatingEl.style.pointerEvents).toBe("none"); + floatingUI.reposition(fakeFloatingUiComponent, positionOptions, true); + expect(positionSpy).toHaveBeenCalledTimes(1); - disconnectFloatingUI(fakeFloatingUiComponent, referenceEl, floatingEl); + floatingUI.reposition(anotherFakeFloatingUiComponent, positionOptions, true); + expect(positionSpy).toHaveBeenCalledTimes(2); - expect(cleanupMap.has(fakeFloatingUiComponent)).toBe(false); - expect(floatingEl.style.position).toBe("absolute"); + await new Promise((resolve) => setTimeout(resolve, repositionDebounceTimeout)); + expect(positionSpy).toHaveBeenCalledTimes(2); + }); +}); - fakeFloatingUiComponent.overlayPositioning = "fixed"; - connectFloatingUI(fakeFloatingUiComponent, referenceEl, floatingEl); +describe("connect/disconnect helpers", () => { + let fakeFloatingUiComponent: FloatingUIComponent; + let floatingEl: HTMLDivElement; + let referenceEl: HTMLButtonElement; - expect(cleanupMap.has(fakeFloatingUiComponent)).toBe(true); - expect(floatingEl.style.position).toBe("fixed"); - expect(floatingEl.style.visibility).toBe("hidden"); - expect(floatingEl.style.pointerEvents).toBe("none"); + beforeEach(() => { + referenceEl = document.createElement("button"); + floatingEl = document.createElement("div"); - disconnectFloatingUI(fakeFloatingUiComponent, referenceEl, floatingEl); + document.body.append(floatingEl); + document.body.append(referenceEl); - expect(cleanupMap.has(fakeFloatingUiComponent)).toBe(false); - expect(floatingEl.style.position).toBe("fixed"); - }); + fakeFloatingUiComponent = createFakeFloatingUiComponent(referenceEl, floatingEl); }); - it("debounces positioning per instance", async () => { - const positionSpy = jest.spyOn(floatingUI, "positionFloatingUI"); + it("has connectedCallback and disconnectedCallback helpers", async () => { fakeFloatingUiComponent.open = true; + expect(autoUpdatingComponentMap.has(fakeFloatingUiComponent)).toBe(false); + expect(floatingEl.style.position).toBe(""); + expect(floatingEl.style.visibility).toBe(""); + expect(floatingEl.style.pointerEvents).toBe(""); - const anotherFakeFloatingUiComponent = createFakeFloatingUiComponent(); - anotherFakeFloatingUiComponent.open = true; + await connectFloatingUI(fakeFloatingUiComponent, referenceEl, floatingEl); - floatingUI.reposition(fakeFloatingUiComponent, positionOptions, true); - expect(positionSpy).toHaveBeenCalledTimes(1); + expect(autoUpdatingComponentMap.has(fakeFloatingUiComponent)).toBe(true); + expect(floatingEl.style.position).toBe("absolute"); + expect(floatingEl.style.visibility).toBe("hidden"); + expect(floatingEl.style.pointerEvents).toBe("none"); - floatingUI.reposition(anotherFakeFloatingUiComponent, positionOptions, true); - expect(positionSpy).toHaveBeenCalledTimes(2); + disconnectFloatingUI(fakeFloatingUiComponent, referenceEl, floatingEl); - await new Promise((resolve) => setTimeout(resolve, repositionDebounceTimeout)); - expect(positionSpy).toHaveBeenCalledTimes(2); + expect(autoUpdatingComponentMap.has(fakeFloatingUiComponent)).toBe(false); + expect(floatingEl.style.position).toBe("absolute"); + + fakeFloatingUiComponent.overlayPositioning = "fixed"; + await connectFloatingUI(fakeFloatingUiComponent, referenceEl, floatingEl); + + expect(autoUpdatingComponentMap.has(fakeFloatingUiComponent)).toBe(true); + expect(floatingEl.style.position).toBe("fixed"); + expect(floatingEl.style.visibility).toBe("hidden"); + expect(floatingEl.style.pointerEvents).toBe("none"); + + disconnectFloatingUI(fakeFloatingUiComponent, referenceEl, floatingEl); + + expect(autoUpdatingComponentMap.has(fakeFloatingUiComponent)).toBe(false); + expect(floatingEl.style.position).toBe("fixed"); }); }); diff --git a/packages/calcite-components/src/utils/floating-ui.ts b/packages/calcite-components/src/utils/floating-ui.ts index 2687c46e999..c10f44c1d85 100644 --- a/packages/calcite-components/src/utils/floating-ui.ts +++ b/packages/calcite-components/src/utils/floating-ui.ts @@ -427,13 +427,19 @@ export async function reposition( options: Parameters[1], delayed = false, ): Promise { - if (!component.open) { + if (!component.open || !options.floatingEl || !options.referenceEl) { return; } + const trackedState = autoUpdatingComponentMap.get(component); + + if (!trackedState) { + return runAutoUpdate(component, options.referenceEl, options.floatingEl); + } + const positionFunction = delayed ? getDebouncedReposition(component) : positionFloatingUI; - return positionFunction(component, options); + await positionFunction(component, options); } function getDebouncedReposition(component: FloatingUIComponent): DebouncedFunc { @@ -460,15 +466,68 @@ const ARROW_CSS_TRANSFORM = { right: "rotate(90deg)", }; +type PendingFloatingUIState = { + state: "pending"; +}; + +type ActiveFloatingUIState = { + state: "active"; + cleanUp: () => void; +}; + +type TrackedFloatingUIState = PendingFloatingUIState | ActiveFloatingUIState; + /** * Exported for testing purposes only * * @internal */ -export const cleanupMap = new WeakMap void>(); +export const autoUpdatingComponentMap = new WeakMap(); const componentToDebouncedRepositionMap = new WeakMap>(); +async function runAutoUpdate( + component: FloatingUIComponent, + referenceEl: ReferenceElement, + floatingEl: HTMLElement, +): Promise { + if (!floatingEl.isConnected) { + return; + } + + const effectiveAutoUpdate = Build.isBrowser + ? autoUpdate + : (_refEl: HTMLElement, _floatingEl: HTMLElement, updateCallback: () => void): (() => void) => { + updateCallback(); + return () => { + /* noop */ + }; + }; + + // we set initial state here to make it available for `reposition` calls + autoUpdatingComponentMap.set(component, { state: "pending" }); + + const rep = component.reposition(); + + // define callback function that return s `rep` on the first call and then returns component.reposition(); + const callback = () => { + let first = true; + return () => { + if (first) { + first = false; + return rep; + } + + return component.reposition(); + }; + }; + + const cleanUp = effectiveAutoUpdate(referenceEl, floatingEl, callback); + autoUpdatingComponentMap.set(component, { state: "active", cleanUp }); + + return rep; +} + /** * Helper to set up floating element interactions on connectedCallback. * @@ -476,11 +535,11 @@ const componentToDebouncedRepositionMap = new WeakMap { if (!floatingEl || !referenceEl) { return; } @@ -495,19 +554,11 @@ export function connectFloatingUI( position: component.overlayPositioning, }); - const runAutoUpdate = Build.isBrowser - ? autoUpdate - : (_refEl: HTMLElement, _floatingEl: HTMLElement, updateCallback: () => void): (() => void) => { - updateCallback(); - return () => { - /* noop */ - }; - }; + if (!component.open) { + return; + } - cleanupMap.set( - component, - runAutoUpdate(referenceEl, floatingEl, () => component.reposition()), - ); + return runAutoUpdate(component, referenceEl, floatingEl); } /** @@ -526,8 +577,13 @@ export function disconnectFloatingUI( return; } - cleanupMap.get(component)?.(); - cleanupMap.delete(component); + const trackedState = autoUpdatingComponentMap.get(component); + + if (trackedState?.state === "active") { + trackedState.cleanUp(); + } + + autoUpdatingComponentMap.delete(component); componentToDebouncedRepositionMap.get(component)?.cancel(); componentToDebouncedRepositionMap.delete(component); From c5de05608c2ba546146f9d996454f494a1e15e9b Mon Sep 17 00:00:00 2001 From: JC Franco Date: Thu, 23 May 2024 15:04:13 -0700 Subject: [PATCH 2/4] fix autoUpdate callback not being invoked properly --- .../src/utils/floating-ui.ts | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/packages/calcite-components/src/utils/floating-ui.ts b/packages/calcite-components/src/utils/floating-ui.ts index c10f44c1d85..9d921f4f100 100644 --- a/packages/calcite-components/src/utils/floating-ui.ts +++ b/packages/calcite-components/src/utils/floating-ui.ts @@ -507,25 +507,24 @@ async function runAutoUpdate( // we set initial state here to make it available for `reposition` calls autoUpdatingComponentMap.set(component, { state: "pending" }); - const rep = component.reposition(); - - // define callback function that return s `rep` on the first call and then returns component.reposition(); - const callback = () => { - let first = true; - return () => { - if (first) { - first = false; - return rep; - } + let repositionPromise: Promise; - return component.reposition(); - }; - }; + const cleanUp = effectiveAutoUpdate( + referenceEl, + floatingEl, + // callback is invoked immediately + () => { + const promise = component.reposition(); + + if (!repositionPromise) { + repositionPromise = promise; + } + }, + ); - const cleanUp = effectiveAutoUpdate(referenceEl, floatingEl, callback); autoUpdatingComponentMap.set(component, { state: "active", cleanUp }); - return rep; + return repositionPromise; } /** From 95273b9b24dd58dde8ebd0a771c2e20906e2a42c Mon Sep 17 00:00:00 2001 From: JC Franco Date: Thu, 23 May 2024 16:47:50 -0700 Subject: [PATCH 3/4] tidy up --- packages/calcite-components/src/components/popover/popover.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/calcite-components/src/components/popover/popover.tsx b/packages/calcite-components/src/components/popover/popover.tsx index 3761ddbe25e..076313e31a6 100644 --- a/packages/calcite-components/src/components/popover/popover.tsx +++ b/packages/calcite-components/src/components/popover/popover.tsx @@ -294,7 +294,6 @@ export class Popover await componentOnReady(this.el); this.setUpReferenceElement(); connectFocusTrap(this); - connectFloatingUI(this, this.effectiveReferenceElement, this.el); if (this.open) { onToggleOpenCloseComponent(this); From 6e18671040732601974103860532e740db8b9bd9 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Tue, 28 May 2024 13:53:02 -0700 Subject: [PATCH 4/4] tidy up --- packages/calcite-components/src/components/tooltip/tooltip.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/calcite-components/src/components/tooltip/tooltip.tsx b/packages/calcite-components/src/components/tooltip/tooltip.tsx index 0d2015a92dd..9961d2feb20 100644 --- a/packages/calcite-components/src/components/tooltip/tooltip.tsx +++ b/packages/calcite-components/src/components/tooltip/tooltip.tsx @@ -160,7 +160,6 @@ export class Tooltip implements FloatingUIComponent, OpenCloseComponent { async connectedCallback(): Promise { await componentOnReady(this.el); this.setUpReferenceElement(true); - connectFloatingUI(this, this.effectiveReferenceElement, this.el); if (this.open) { onToggleOpenCloseComponent(this);