From cc3624d1b09b3139f4df27a86f0e58fe21c9a7a5 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Sun, 22 Dec 2024 08:51:50 -0700 Subject: [PATCH 1/3] fix(popover, tooltip): skip ref set up logic on component removal --- .../calcite-components/src/components/popover/popover.tsx | 5 ++++- .../calcite-components/src/components/tooltip/tooltip.tsx | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/calcite-components/src/components/popover/popover.tsx b/packages/calcite-components/src/components/popover/popover.tsx index 7febcdb51b2..518220def34 100644 --- a/packages/calcite-components/src/components/popover/popover.tsx +++ b/packages/calcite-components/src/components/popover/popover.tsx @@ -387,7 +387,10 @@ export class Popover private setFloatingEl(el: HTMLDivElement): void { this.floatingEl = el; - requestAnimationFrame(() => this.setUpReferenceElement()); + + if (el) { + requestAnimationFrame(() => this.setUpReferenceElement()); + } } private setTransitionEl(el: HTMLDivElement): void { diff --git a/packages/calcite-components/src/components/tooltip/tooltip.tsx b/packages/calcite-components/src/components/tooltip/tooltip.tsx index befcfb164f4..20c09270660 100644 --- a/packages/calcite-components/src/components/tooltip/tooltip.tsx +++ b/packages/calcite-components/src/components/tooltip/tooltip.tsx @@ -247,7 +247,10 @@ export class Tooltip extends LitElement implements FloatingUIComponent, OpenClos private setFloatingEl(el: HTMLDivElement): void { this.floatingEl = el; - requestAnimationFrame(() => this.setUpReferenceElement()); + + if (el) { + requestAnimationFrame(() => this.setUpReferenceElement()); + } } private setTransitionEl(el: HTMLDivElement): void { From 187bbb160884a64ff0e961d0001c3e9f4de53784 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Mon, 23 Dec 2024 00:56:22 -0700 Subject: [PATCH 2/3] add tests --- .../src/components/popover/popover.e2e.ts | 50 ++++++++++++++++++- .../src/components/tooltip/tooltip.e2e.ts | 50 ++++++++++++++++++- 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/packages/calcite-components/src/components/popover/popover.e2e.ts b/packages/calcite-components/src/components/popover/popover.e2e.ts index f0ea02bf118..c3debe2eb19 100644 --- a/packages/calcite-components/src/components/popover/popover.e2e.ts +++ b/packages/calcite-components/src/components/popover/popover.e2e.ts @@ -1,5 +1,5 @@ import { newE2EPage } from "@arcgis/lumina-compiler/puppeteerTesting"; -import { describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, MockInstance, vi } from "vitest"; import { html } from "../../../support/formatting"; import { accessible, @@ -729,6 +729,54 @@ describe("calcite-popover", () => { }); }); + describe("warning messages", () => { + let consoleSpy: MockInstance; + + beforeEach( + () => + (consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => { + // hide warning messages during test + })), + ); + + afterEach(() => consoleSpy.mockClear()); + + it("does not warn if reference element is present", async () => { + const page = await newE2EPage(); + await page.setContent( + html`content +
referenceElement
`, + ); + await page.waitForChanges(); + + expect(consoleSpy).not.toHaveBeenCalled(); + }); + + it("does not warn after removal", async () => { + const page = await newE2EPage(); + await page.setContent( + html`content +
referenceElement
`, + ); + await page.waitForChanges(); + const popover = await page.find("calcite-popover"); + await popover.callMethod("remove"); + await page.waitForChanges(); + + expect(consoleSpy).not.toHaveBeenCalled(); + }); + + it("warns if reference element is not present", async () => { + const page = await newE2EPage(); + await page.setContent(`content`); + await page.waitForChanges(); + + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringMatching(new RegExp(`reference-element id "non-existent-ref" was not found`)), + ); + }); + }); + describe("theme", () => { describe("default", () => { themed( diff --git a/packages/calcite-components/src/components/tooltip/tooltip.e2e.ts b/packages/calcite-components/src/components/tooltip/tooltip.e2e.ts index bb0f8719f7e..06549ce8449 100644 --- a/packages/calcite-components/src/components/tooltip/tooltip.e2e.ts +++ b/packages/calcite-components/src/components/tooltip/tooltip.e2e.ts @@ -1,5 +1,5 @@ import { newE2EPage, E2EPage } from "@arcgis/lumina-compiler/puppeteerTesting"; -import { describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, MockInstance, vi } from "vitest"; import { accessible, defaults, floatingUIOwner, hidden, openClose, renders, themed } from "../../tests/commonTests"; import { html } from "../../../support/formatting"; import { getElementXY, GlobalTestProps, skipAnimations } from "../../tests/utils"; @@ -1204,6 +1204,54 @@ describe("calcite-tooltip", () => { }); }); + describe("warning messages", () => { + let consoleSpy: MockInstance; + + beforeEach( + () => + (consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => { + // hide warning messages during test + })), + ); + + afterEach(() => consoleSpy.mockClear()); + + it("does not warn if reference element is present", async () => { + const page = await newE2EPage(); + await page.setContent( + html` content +
referenceElement
`, + ); + await page.waitForChanges(); + + expect(consoleSpy).not.toHaveBeenCalled(); + }); + + it("does not warn after removal", async () => { + const page = await newE2EPage(); + await page.setContent( + html` content +
referenceElement
`, + ); + await page.waitForChanges(); + const popover = await page.find("calcite-popover"); + await popover.callMethod("remove"); + await page.waitForChanges(); + + expect(consoleSpy).not.toHaveBeenCalled(); + }); + + it("warns if reference element is not present", async () => { + const page = await newE2EPage(); + await page.setContent(`content`); + await page.waitForChanges(); + + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringMatching(new RegExp(`reference-element id "non-existent-ref" was not found`)), + ); + }); + }); + describe("theme", () => { describe("default", () => { themed( From 8885281306307dceb705d838ea2a584c399de6cc Mon Sep 17 00:00:00 2001 From: JC Franco Date: Mon, 23 Dec 2024 09:56:28 -0700 Subject: [PATCH 3/3] fix test --- .../calcite-components/src/components/tooltip/tooltip.e2e.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/calcite-components/src/components/tooltip/tooltip.e2e.ts b/packages/calcite-components/src/components/tooltip/tooltip.e2e.ts index 06549ce8449..d0c35301f59 100644 --- a/packages/calcite-components/src/components/tooltip/tooltip.e2e.ts +++ b/packages/calcite-components/src/components/tooltip/tooltip.e2e.ts @@ -1234,8 +1234,8 @@ describe("calcite-tooltip", () => {
referenceElement
`, ); await page.waitForChanges(); - const popover = await page.find("calcite-popover"); - await popover.callMethod("remove"); + const tooltip = await page.find("calcite-tooltip"); + await tooltip.callMethod("remove"); await page.waitForChanges(); expect(consoleSpy).not.toHaveBeenCalled();