From c84949a08a13e9607a049ba1c99dfa4a1e72da11 Mon Sep 17 00:00:00 2001 From: Matt Driscoll Date: Thu, 12 Oct 2023 15:26:54 -0700 Subject: [PATCH 1/7] fix: floating components will now get an initial position even if they are not opened. #7979 --- .../src/utils/floating-ui.spec.ts | 10 ---- .../src/utils/floating-ui.ts | 46 +++++++++---------- 2 files changed, 22 insertions(+), 34 deletions(-) diff --git a/packages/calcite-components/src/utils/floating-ui.spec.ts b/packages/calcite-components/src/utils/floating-ui.spec.ts index 7d9931e33ed..692874d54c5 100644 --- a/packages/calcite-components/src/utils/floating-ui.spec.ts +++ b/packages/calcite-components/src/utils/floating-ui.spec.ts @@ -80,16 +80,6 @@ describe("repositioning", () => { expect(floatingEl.style.left).toBe("0"); } - it("repositions only for open components", async () => { - await reposition(fakeFloatingUiComponent, positionOptions); - assertPreOpenPositioning(floatingEl); - - fakeFloatingUiComponent.open = true; - - await reposition(fakeFloatingUiComponent, positionOptions); - assertOpenPositioning(floatingEl); - }); - it("repositions immediately by default", async () => { fakeFloatingUiComponent.open = true; diff --git a/packages/calcite-components/src/utils/floating-ui.ts b/packages/calcite-components/src/utils/floating-ui.ts index c3a36b2d84c..7fc3f6a1860 100644 --- a/packages/calcite-components/src/utils/floating-ui.ts +++ b/packages/calcite-components/src/utils/floating-ui.ts @@ -402,29 +402,27 @@ export function getEffectivePlacement(floatingEl: HTMLElement, placement: Logica * * Note: this is not needed for components that use `calcite-popover`. * - * @param component - * @param options - * @param options.referenceEl - * @param options.floatingEl - * @param options.overlayPositioning - * @param options.placement - * @param options.flipDisabled - * @param options.flipPlacements - * @param options.offsetDistance - * @param options.offsetSkidding - * @param options.arrowEl - * @param options.type - * @param delayed + * @param component - A floating-ui component. + * @param options - Reposition parameters. + * @param options.referenceEl - Reference element. + * @param options.floatingEl - Floating element. + * @param options.overlayPositioning - type of positioning to use for the overlaid content. + * @param options.placement - Determines where the component will be positioned relative to the `referenceElement`. + * @param options.flipDisabled - Prevents flipping the component's placement when overlapping its `referenceElement`. + * @param options.flipPlacements - Defines the available placements that can be used when a flip occurs. + * @param options.offsetDistance - Offsets the position of the popover away from the `referenceElement`. + * @param options.offsetSkidding - Offsets the position of the component along the `referenceElement`. + * @param options.arrowEl - A customizable arrow element. + * @param options.type - The type of floating UI. + * @param delayed - Reposition the component after a delay. + * + * @returns {Promise} */ export async function reposition( component: FloatingUIComponent, options: Parameters[1], delayed = false ): Promise { - if (!component.open) { - return; - } - const positionFunction = delayed ? getDebouncedReposition(component) : positionFloatingUI; return positionFunction(component, options); @@ -466,9 +464,9 @@ const componentToDebouncedRepositionMap = new WeakMap void) => { + : (_refEl: HTMLElement, _floatingEl: HTMLElement, updateCallback: () => void): (() => void) => { updateCallback(); return () => { /* noop */ @@ -509,9 +507,9 @@ export function connectFloatingUI( /** * Helper to tear down floating element interactions on disconnectedCallback. * - * @param component - * @param referenceEl - * @param floatingEl + * @param component - A floating-ui component. + * @param referenceEl - Reference element. + * @param floatingEl - Floating element. */ export function disconnectFloatingUI( component: FloatingUIComponent, From 50ac6a97e77db1b01069f33059858f41f264b32d Mon Sep 17 00:00:00 2001 From: Matt Driscoll Date: Thu, 12 Oct 2023 15:28:52 -0700 Subject: [PATCH 2/7] cleanup --- packages/calcite-components/src/utils/floating-ui.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/calcite-components/src/utils/floating-ui.ts b/packages/calcite-components/src/utils/floating-ui.ts index 7fc3f6a1860..d3c1cc51e16 100644 --- a/packages/calcite-components/src/utils/floating-ui.ts +++ b/packages/calcite-components/src/utils/floating-ui.ts @@ -464,7 +464,7 @@ const componentToDebouncedRepositionMap = new WeakMap Date: Thu, 12 Oct 2023 15:31:58 -0700 Subject: [PATCH 3/7] cleanup --- packages/calcite-components/src/utils/floating-ui.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/calcite-components/src/utils/floating-ui.ts b/packages/calcite-components/src/utils/floating-ui.ts index d3c1cc51e16..49145e537d2 100644 --- a/packages/calcite-components/src/utils/floating-ui.ts +++ b/packages/calcite-components/src/utils/floating-ui.ts @@ -404,8 +404,8 @@ export function getEffectivePlacement(floatingEl: HTMLElement, placement: Logica * * @param component - A floating-ui component. * @param options - Reposition parameters. - * @param options.referenceEl - Reference element. - * @param options.floatingEl - Floating element. + * @param options.referenceEl - The `referenceElement` used to position the component according to its `placement` value. + * @param options.floatingEl - The `floatingElement` containing the floating ui. * @param options.overlayPositioning - type of positioning to use for the overlaid content. * @param options.placement - Determines where the component will be positioned relative to the `referenceElement`. * @param options.flipDisabled - Prevents flipping the component's placement when overlapping its `referenceElement`. @@ -465,8 +465,8 @@ const componentToDebouncedRepositionMap = new WeakMap Date: Thu, 12 Oct 2023 15:35:04 -0700 Subject: [PATCH 4/7] cleanup --- .../calcite-components/src/utils/floating-ui.spec.ts | 10 ++++++++++ packages/calcite-components/src/utils/floating-ui.ts | 1 - 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/calcite-components/src/utils/floating-ui.spec.ts b/packages/calcite-components/src/utils/floating-ui.spec.ts index 692874d54c5..5c8face8977 100644 --- a/packages/calcite-components/src/utils/floating-ui.spec.ts +++ b/packages/calcite-components/src/utils/floating-ui.spec.ts @@ -80,6 +80,16 @@ describe("repositioning", () => { expect(floatingEl.style.left).toBe("0"); } + it("repositions for unopened components", async () => { + await reposition(fakeFloatingUiComponent, positionOptions); + assertOpenPositioning(floatingEl); + + fakeFloatingUiComponent.open = true; + + await reposition(fakeFloatingUiComponent, positionOptions); + assertOpenPositioning(floatingEl); + }); + it("repositions immediately by default", async () => { fakeFloatingUiComponent.open = true; diff --git a/packages/calcite-components/src/utils/floating-ui.ts b/packages/calcite-components/src/utils/floating-ui.ts index 49145e537d2..c8e371941b6 100644 --- a/packages/calcite-components/src/utils/floating-ui.ts +++ b/packages/calcite-components/src/utils/floating-ui.ts @@ -415,7 +415,6 @@ export function getEffectivePlacement(floatingEl: HTMLElement, placement: Logica * @param options.arrowEl - A customizable arrow element. * @param options.type - The type of floating UI. * @param delayed - Reposition the component after a delay. - * * @returns {Promise} */ export async function reposition( From c4d52b704d135c0cf50959f460b61327112b231d Mon Sep 17 00:00:00 2001 From: Matt Driscoll Date: Thu, 12 Oct 2023 16:06:48 -0700 Subject: [PATCH 5/7] fix common test --- packages/calcite-components/src/tests/commonTests.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/calcite-components/src/tests/commonTests.ts b/packages/calcite-components/src/tests/commonTests.ts index 78c9e159537..9e6f2b19781 100644 --- a/packages/calcite-components/src/tests/commonTests.ts +++ b/packages/calcite-components/src/tests/commonTests.ts @@ -1321,7 +1321,7 @@ export function floatingUIOwner( await scrollTo(scrollablePageSizeInPx, scrollablePageSizeInPx); await page.waitForChanges(); - expect(await getTransform()).toBe(initialClosedTransform); + expect(await getTransform()).not.toBe(initialClosedTransform); await scrollTo(0, 0); await page.waitForChanges(); From ba5408ab4c6c8d44b1c758479915677d068e97f0 Mon Sep 17 00:00:00 2001 From: Matt Driscoll Date: Fri, 13 Oct 2023 08:45:55 -0700 Subject: [PATCH 6/7] fix test --- .../src/components/combobox/combobox.e2e.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/calcite-components/src/components/combobox/combobox.e2e.ts b/packages/calcite-components/src/components/combobox/combobox.e2e.ts index e315419538c..39ae35891ef 100644 --- a/packages/calcite-components/src/components/combobox/combobox.e2e.ts +++ b/packages/calcite-components/src/components/combobox/combobox.e2e.ts @@ -1624,13 +1624,11 @@ describe("calcite-combobox", () => { it("should gain focus when it's items are selected via click", async () => { const page = await newE2EPage(); await page.setContent(html` - + `); await skipAnimations(page); - const item = await page.find("calcite-combobox-item"); - await item.click(); - await page.waitForChanges(); const focusedId = await page.evaluate(() => { + document.getElementById("demoItemId").click(); const el = document.activeElement; return el.id; }); From ab889bad5cf83f72157349b21645f44a689d4c0c Mon Sep 17 00:00:00 2001 From: Matt Driscoll Date: Fri, 13 Oct 2023 10:21:19 -0700 Subject: [PATCH 7/7] review fixes --- .../src/components/combobox/combobox.e2e.ts | 10 ++++++---- .../calcite-components/src/utils/floating-ui.spec.ts | 8 ++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/calcite-components/src/components/combobox/combobox.e2e.ts b/packages/calcite-components/src/components/combobox/combobox.e2e.ts index 39ae35891ef..ec9594ebdc8 100644 --- a/packages/calcite-components/src/components/combobox/combobox.e2e.ts +++ b/packages/calcite-components/src/components/combobox/combobox.e2e.ts @@ -1605,7 +1605,7 @@ describe("calcite-combobox", () => { it("should not focus on the combobox when items are programmatically selected", async () => { const page = await newE2EPage(); - await page.setContent(html` + await page.setContent(html` `); const item = await page.find("calcite-combobox-item"); @@ -1623,12 +1623,14 @@ describe("calcite-combobox", () => { it("should gain focus when it's items are selected via click", async () => { const page = await newE2EPage(); - await page.setContent(html` - + await page.setContent(html` + `); await skipAnimations(page); + const item = await page.find("calcite-combobox-item"); + await item.click(); + await page.waitForChanges(); const focusedId = await page.evaluate(() => { - document.getElementById("demoItemId").click(); const el = document.activeElement; return el.id; }); diff --git a/packages/calcite-components/src/utils/floating-ui.spec.ts b/packages/calcite-components/src/utils/floating-ui.spec.ts index 5c8face8977..e3174e94c72 100644 --- a/packages/calcite-components/src/utils/floating-ui.spec.ts +++ b/packages/calcite-components/src/utils/floating-ui.spec.ts @@ -91,23 +91,23 @@ describe("repositioning", () => { }); it("repositions immediately by default", async () => { + assertPreOpenPositioning(floatingEl); + fakeFloatingUiComponent.open = true; reposition(fakeFloatingUiComponent, positionOptions); - assertPreOpenPositioning(floatingEl); - await waitForAnimationFrame(); assertOpenPositioning(floatingEl); }); it("can reposition after a delay", async () => { + assertPreOpenPositioning(floatingEl); + fakeFloatingUiComponent.open = true; reposition(fakeFloatingUiComponent, positionOptions, true); - assertPreOpenPositioning(floatingEl); - await new Promise((resolve) => setTimeout(resolve, repositionDebounceTimeout)); assertOpenPositioning(floatingEl); });