From 51a076212bb7ce2375c0b5cf9d5e04fcc6e1dcf6 Mon Sep 17 00:00:00 2001 From: Hariom Balhara Date: Wed, 9 Jul 2025 01:22:37 +0530 Subject: [PATCH] fix: Do not wait for slots request before removing the (#22323) default(non-skeleton) loader Co-authored-by: Udit Takkar <53316345+Udit-takkar@users.noreply.github.com> --- .../embed-core/src/EmbedElement.test.ts | 6 +- .../embeds/embed-core/src/EmbedElement.ts | 12 +-- .../src/__tests__/embed-iframe.test.ts | 27 +----- .../__tests__/isLinkReady.test.ts | 85 +++++++++++++++++++ .../src/embed-iframe/__tests__/test-utils.ts | 35 ++++++++ .../embed-core/src/embed-iframe/lib/utils.ts | 20 ++++- 6 files changed, 144 insertions(+), 41 deletions(-) create mode 100644 packages/embeds/embed-core/src/embed-iframe/__tests__/isLinkReady.test.ts create mode 100644 packages/embeds/embed-core/src/embed-iframe/__tests__/test-utils.ts diff --git a/packages/embeds/embed-core/src/EmbedElement.test.ts b/packages/embeds/embed-core/src/EmbedElement.test.ts index 57c7f83034..1c2760b878 100644 --- a/packages/embeds/embed-core/src/EmbedElement.test.ts +++ b/packages/embeds/embed-core/src/EmbedElement.test.ts @@ -167,14 +167,14 @@ describe("EmbedElement", () => { expectDefaultLoader(element); }); - it("should show default loader for when page type is not supported", () => { + it("should show skeleton loader for any non-empty page type (including unsupported ones)", () => { element = createTestEmbedElement({ dataset: { pageType: "unknown", }, }); - expectDefaultLoader(element); + expectSkeletonLoader(element); }); it("should hide skeleton loader when toggled off", () => { @@ -222,7 +222,7 @@ describe("EmbedElement", () => { isModal = true; }); - it("should show default loader for unsupported page types", () => { + it("should show default loader only when page type is not provided", () => { element = createTestEmbedElement({ isModal, }); diff --git a/packages/embeds/embed-core/src/EmbedElement.ts b/packages/embeds/embed-core/src/EmbedElement.ts index 6c018313d5..f5463037c8 100644 --- a/packages/embeds/embed-core/src/EmbedElement.ts +++ b/packages/embeds/embed-core/src/EmbedElement.ts @@ -31,12 +31,8 @@ export class EmbedElement extends HTMLElement { private boundPrefersDarkThemeChangedHandler: (e: MediaQueryListEvent) => void; private isSkeletonSupportedPageType() { const pageType = this.getPageType(); - return ( - pageType === "user.event.booking.slots" || - pageType === "team.event.booking.slots" || - pageType === "user.event.booking.form" || - pageType === "team.event.booking.form" - ); + // Any pageType being set is considered as skeleton supported. There is always a fallback skeleton loader if no direct match for a skeleton loader is found based on pageType + return !!pageType; } public assertHasShadowRoot(): asserts this is HTMLElement & { shadowRoot: ShadowRootWithStyle } { if (!this.shadowRoot) { @@ -44,8 +40,8 @@ export class EmbedElement extends HTMLElement { } } - public getPageType(): EmbedPageType { - return this.dataset.pageType as EmbedPageType; + public getPageType(): EmbedPageType | undefined { + return this.dataset.pageType as EmbedPageType | undefined; } public getLayout(): AllPossibleLayouts { return getTrueLayout({ layout: (this.dataset.layout as BookerLayouts | undefined) ?? null }); diff --git a/packages/embeds/embed-core/src/__tests__/embed-iframe.test.ts b/packages/embeds/embed-core/src/__tests__/embed-iframe.test.ts index 41b6a1d442..05c833afdd 100644 --- a/packages/embeds/embed-core/src/__tests__/embed-iframe.test.ts +++ b/packages/embeds/embed-core/src/__tests__/embed-iframe.test.ts @@ -1,43 +1,18 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -// Test helper functions -type fakeCurrentDocumentUrlParams = { - origin?: string; - path?: string; - params?: Record; -}; +import { fakeCurrentDocumentUrl, nextTick } from "../embed-iframe/__tests__/test-utils"; beforeEach(() => { // Ensure that we have it globally so that unexpected errors like 'document is not defined' don't happen due to timer being fired when test is shutting down vi.useFakeTimers(); }); -function fakeCurrentDocumentUrl({ - origin = "https://example.com", - path = "", - params = {}, -}: fakeCurrentDocumentUrlParams = {}) { - const url = new URL(path, origin); - Object.entries(params).forEach(([key, value]) => { - url.searchParams.set(key, value); - }); - return mockDocumentUrl(url); -} - -function mockDocumentUrl(url: URL | string) { - return vi.spyOn(document, "URL", "get").mockReturnValue(url.toString()); -} - afterEach(() => { vi.clearAllMocks(); vi.resetModules(); vi.useRealTimers(); }); -function nextTick() { - vi.advanceTimersByTime(100); -} - describe("embedStore.router.ensureQueryParamsInUrl", async () => { let embedStore: typeof import("../embed-iframe/lib/embedStore").embedStore; const originalHistory = window.history; diff --git a/packages/embeds/embed-core/src/embed-iframe/__tests__/isLinkReady.test.ts b/packages/embeds/embed-core/src/embed-iframe/__tests__/isLinkReady.test.ts new file mode 100644 index 0000000000..597447ec80 --- /dev/null +++ b/packages/embeds/embed-core/src/embed-iframe/__tests__/isLinkReady.test.ts @@ -0,0 +1,85 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +import { fakeCurrentDocumentUrl, takeBookerToSlotsLoadingState, takeBookerToReadyState } from "./test-utils"; + +beforeEach(() => { + vi.useFakeTimers(); +}); + +afterEach(() => { + vi.clearAllMocks(); + vi.resetModules(); + vi.useRealTimers(); +}); + +describe("isLinkReady", async () => { + let isLinkReady: typeof import("../lib/utils").isLinkReady; + let embedStore: typeof import("../lib/embedStore").embedStore; + + beforeEach(async () => { + ({ isLinkReady } = await import("../lib/utils")); + ({ embedStore } = await import("../lib/embedStore")); + + // Reset embedStore state to ensure test isolation + embedStore.parentInformedAboutContentHeight = true; + embedStore.renderState = null; + embedStore.connectVersion = 1; + }); + + describe("when skeleton loader is NOT supported (regular loader)", () => { + it("should return true immediately when no page type is provided", () => { + fakeCurrentDocumentUrl({ params: {} }); + takeBookerToSlotsLoadingState(); + + const result = isLinkReady({ embedStore }); + expect(result).toBe(true); + }); + + it("should return true immediately when page type is empty", () => { + fakeCurrentDocumentUrl({ params: { "cal.embed.pageType": "" } }); + takeBookerToSlotsLoadingState(); + + const result = isLinkReady({ embedStore }); + expect(result).toBe(true); + }); + }); + + describe("when skeleton loader is supported", () => { + it("should wait for booker to be ready when page type is user.event.booking.slots", () => { + fakeCurrentDocumentUrl({ params: { "cal.embed.pageType": "user.event.booking.slots" } }); + takeBookerToSlotsLoadingState(); + + const result = isLinkReady({ embedStore }); + expect(result).toBe(false); // Should still wait for skeleton loader pages + }); + + it("should return true when booker is ready and page type is user.event.booking.slots", () => { + fakeCurrentDocumentUrl({ params: { "cal.embed.pageType": "user.event.booking.slots" } }); + takeBookerToReadyState(); + + const result = isLinkReady({ embedStore }); + expect(result).toBe(true); + }); + }); + + describe("when not a booker page", () => { + it("should return true regardless of page type", () => { + fakeCurrentDocumentUrl({ params: { "cal.embed.pageType": "user.event.booking.slots" } }); + // No _embedBookerState set, so not a booker page + + const result = isLinkReady({ embedStore }); + expect(result).toBe(true); + }); + }); + + describe("when parent not informed about content height", () => { + it("should return false regardless of other conditions", () => { + fakeCurrentDocumentUrl({ params: { "cal.embed.pageType": "user.event.booking.slots" } }); + takeBookerToReadyState(); + embedStore.parentInformedAboutContentHeight = false; // Not informed yet + + const result = isLinkReady({ embedStore }); + expect(result).toBe(false); + }); + }); +}); diff --git a/packages/embeds/embed-core/src/embed-iframe/__tests__/test-utils.ts b/packages/embeds/embed-core/src/embed-iframe/__tests__/test-utils.ts new file mode 100644 index 0000000000..f283c178a0 --- /dev/null +++ b/packages/embeds/embed-core/src/embed-iframe/__tests__/test-utils.ts @@ -0,0 +1,35 @@ +import { vi } from "vitest"; + +export type fakeCurrentDocumentUrlParams = { + origin?: string; + path?: string; + params?: Record; +}; + +export function fakeCurrentDocumentUrl({ + origin = "https://example.com", + path = "", + params = {}, +}: fakeCurrentDocumentUrlParams = {}) { + const url = new URL(path, origin); + Object.entries(params).forEach(([key, value]) => { + url.searchParams.set(key, value); + }); + return mockDocumentUrl(url); +} + +export function mockDocumentUrl(url: URL | string) { + return vi.spyOn(document, "URL", "get").mockReturnValue(url.toString()); +} + +export function nextTick() { + vi.advanceTimersByTime(100); +} + +export function takeBookerToReadyState() { + window._embedBookerState = "slotsDone"; +} + +export function takeBookerToSlotsLoadingState() { + window._embedBookerState = "slotsLoading"; +} diff --git a/packages/embeds/embed-core/src/embed-iframe/lib/utils.ts b/packages/embeds/embed-core/src/embed-iframe/lib/utils.ts index 964d28b908..c1dfa15590 100644 --- a/packages/embeds/embed-core/src/embed-iframe/lib/utils.ts +++ b/packages/embeds/embed-core/src/embed-iframe/lib/utils.ts @@ -11,6 +11,14 @@ export function isBookerReady() { return window._embedBookerState === "slotsDone"; } +function isSkeletonSupportedPageType() { + const url = new URL(document.URL); + const pageType = url.searchParams.get("cal.embed.pageType"); + // Any non-empty pageType is skeleton supported because generateSkeleton() + // will generate a skeleton for it (either specific or fallback to default) + return !!pageType; +} + /** * It is important to be able to check realtime(instead of storing isLinkReady as a variable) if the link is ready, because there is a possibility that booker might have moved to non-ready state from ready state */ @@ -20,10 +28,14 @@ export function isLinkReady({ embedStore }: { embedStore: typeof import("./embed } if (isBookerPage()) { - // Let's wait for Booker to be ready before showing the embed - // It means that booker has loaded all its data and is ready to show - // TODO: We could try to mark the embed as ready earlier in this case not relying on document.readyState - return isBookerReady(); + if (isSkeletonSupportedPageType()) { + // Let's wait for Booker to be ready before showing the embed as there is already a skeleton loader being shown and we don't really need to show the booker's actual skeleton + // Booker's actual skeleton shows event-type description and other details too but it could cause the UX to be bad if we show two different skeletons one by one and they might not overlap well + return isBookerReady(); + } else { + // For regular loader (non-skeleton), don't wait for slots to be complete before toggling off the loader + return true; + } } return true; }