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>

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,
         });

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 });

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<string, string>;
-};
+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;

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);
+    });
+  });
+});

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<string, string>;
+};
+
+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";
+}

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;
 }