From bed76beb2f705ef6b184341ac7809028a555ea16 Mon Sep 17 00:00:00 2001 From: Hariom Balhara Date: Mon, 13 Oct 2025 20:59:31 +0530 Subject: [PATCH] fix: Layout shift in inline embed caused due to showing embed before it was really ready (#24427) --- .../embeds/embed-core/src/embed-iframe.ts | 102 +------- .../__tests__/isLinkReady.test.ts | 11 +- .../src/embed-iframe/__tests__/utils.test.ts | 230 ++++++++++++++++++ .../src/embed-iframe/lib/embedStore.ts | 4 +- .../embed-core/src/embed-iframe/lib/utils.ts | 108 +++++++- 5 files changed, 354 insertions(+), 101 deletions(-) create mode 100644 packages/embeds/embed-core/src/embed-iframe/__tests__/utils.test.ts diff --git a/packages/embeds/embed-core/src/embed-iframe.ts b/packages/embeds/embed-core/src/embed-iframe.ts index 9f0a2ff729..66d8c2403f 100644 --- a/packages/embeds/embed-core/src/embed-iframe.ts +++ b/packages/embeds/embed-core/src/embed-iframe.ts @@ -4,7 +4,13 @@ import { useEffect, useRef, useState, useCallback } from "react"; import type { Message } from "./embed"; import { embedStore, EMBED_IFRAME_STATE } from "./embed-iframe/lib/embedStore"; -import { runAsap, isBookerReady, isLinkReady, recordResponseIfQueued } from "./embed-iframe/lib/utils"; +import { + runAsap, + isBookerReady, + isLinkReady, + recordResponseIfQueued, + keepParentInformedAboutDimensionChanges, +} from "./embed-iframe/lib/utils"; import { sdkActionManager } from "./sdk-event"; import type { UiConfig, @@ -195,7 +201,6 @@ export const useEmbedStyles = (elementName: keyof EmbedStyles) => { useEffect(() => { return registerNewSetter({ elementName, setState: setStyles, styles: true }); - // eslint-disable-next-line react-hooks/exhaustive-deps }, []); const styles = embedStore.styles || {}; // Always read the data from global embedStore so that even across components, the same data is used. @@ -207,7 +212,6 @@ export const useEmbedNonStylesConfig = (elementName: keyof EmbedNonStylesConfig) useEffect(() => { return registerNewSetter({ elementName, setState: setNonStyles, styles: false }); - // eslint-disable-next-line react-hooks/exhaustive-deps }, []); // Always read the data from global embedStore so that even across components, the same data is used. @@ -387,7 +391,6 @@ export const methods = { setEmbedStyles(stylesConfig || {}); setEmbedNonStyles(stylesConfig || {}); }, - // eslint-disable-next-line @typescript-eslint/no-unused-vars parentKnowsIframeReady: (_unused: unknown) => { log("Method: `parentKnowsIframeReady` called"); // No UI change should happen in sight. Let the parent height adjust and in next cycle show it. @@ -425,7 +428,7 @@ export const methods = { ...queryParamsFromConfig } = config; // We reset it to allow informing parent again through `__dimensionChanged` event about possibly updated dimensions with changes in config - embedStore.parentInformedAboutContentHeight = false; + embedStore.providedCorrectHeightToParent = false; if (noSlotsFetchOnConnect !== "true") { log("Method: connect, noSlotsFetchOnConnect is false. Requesting slots re-fetch"); @@ -484,93 +487,6 @@ const messageParent = (data: CustomEvent["detail"]) => { ); }; -/** - * This function is called once the iframe loads. - * It isn't called on "connect" - */ -function keepParentInformedAboutDimensionChanges() { - let knownIframeHeight: number | null = null; - let knownIframeWidth: number | null = null; - let isFirstTime = true; - let isWindowLoadComplete = false; - runAsap(function informAboutScroll() { - if (document.readyState !== "complete") { - // Wait for window to load to correctly calculate the initial scroll height. - runAsap(informAboutScroll); - return; - } - if (!isWindowLoadComplete) { - // On Safari, even though document.readyState is complete, still the page is not rendered and we can't compute documentElement.scrollHeight correctly - // Postponing to just next cycle allow us to fix this. - setTimeout(() => { - isWindowLoadComplete = true; - informAboutScroll(); - }, 100); - return; - } - - if (!embedStore.windowLoadEventFired) { - sdkActionManager?.fire("__windowLoadComplete", {}); - } - embedStore.windowLoadEventFired = true; - - // Use the dimensions of main element as in most places there is max-width restriction on it and we just want to show the main content. - // It avoids the unwanted padding outside main tag. - const mainElement = - document.getElementsByClassName("main")[0] || - document.getElementsByTagName("main")[0] || - document.documentElement; - const documentScrollHeight = document.documentElement.scrollHeight; - const documentScrollWidth = document.documentElement.scrollWidth; - - if (!(mainElement instanceof HTMLElement)) { - throw new Error("Main element should be an HTMLElement"); - } - - const mainElementStyles = getComputedStyle(mainElement); - // Use, .height as that gives more accurate value in floating point. Also, do a ceil on the total sum so that whatever happens there is enough iframe size to avoid scroll. - const contentHeight = Math.ceil( - parseFloat(mainElementStyles.height) + - parseFloat(mainElementStyles.marginTop) + - parseFloat(mainElementStyles.marginBottom) - ); - const contentWidth = Math.ceil( - parseFloat(mainElementStyles.width) + - parseFloat(mainElementStyles.marginLeft) + - parseFloat(mainElementStyles.marginRight) - ); - - // During first render let iframe tell parent that how much is the expected height to avoid scroll. - // Parent would set the same value as the height of iframe which would prevent scroll. - // On subsequent renders, consider html height as the height of the iframe. If we don't do this, then if iframe gets bigger in height, it would never shrink - const iframeHeight = isFirstTime ? documentScrollHeight : contentHeight; - const iframeWidth = isFirstTime ? documentScrollWidth : contentWidth; - - if (!iframeHeight || !iframeWidth) { - runAsap(informAboutScroll); - return; - } - const isThereAChangeInDimensions = knownIframeHeight !== iframeHeight || knownIframeWidth !== iframeWidth; - if (isThereAChangeInDimensions || !embedStore.parentInformedAboutContentHeight) { - embedStore.parentInformedAboutContentHeight = true; - - knownIframeHeight = iframeHeight; - knownIframeWidth = iframeWidth; - // FIXME: This event shouldn't be subscribable by the user. Only by the SDK. - sdkActionManager?.fire("__dimensionChanged", { - iframeHeight, - iframeWidth, - isFirstTime, - }); - } - isFirstTime = false; - // Parent Counterpart would change the dimension of iframe and thus page's dimension would be impacted which is recursive. - // It should stop ideally by reaching a hiddenHeight value of 0. - // FIXME: If 0 can't be reached we need to just abandon our quest for perfect iframe and let scroll be there. Such case can be logged in the wild and fixed later on. - runAsap(informAboutScroll); - }); -} - function main() { if (!isBrowser) { return; @@ -659,7 +575,7 @@ function initializeAndSetupEmbed() { const pageStatus = window.CalComPageStatus; if (!pageStatus || pageStatus == "200") { - keepParentInformedAboutDimensionChanges(); + keepParentInformedAboutDimensionChanges({ embedStore }); } else sdkActionManager?.fire("linkFailed", { code: pageStatus, 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 index 597447ec80..8918afcaa6 100644 --- a/packages/embeds/embed-core/src/embed-iframe/__tests__/isLinkReady.test.ts +++ b/packages/embeds/embed-core/src/embed-iframe/__tests__/isLinkReady.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { type EmbedStore } from "../lib/embedStore"; import { fakeCurrentDocumentUrl, takeBookerToSlotsLoadingState, takeBookerToReadyState } from "./test-utils"; beforeEach(() => { @@ -13,15 +14,15 @@ afterEach(() => { }); describe("isLinkReady", async () => { - let isLinkReady: typeof import("../lib/utils").isLinkReady; - let embedStore: typeof import("../lib/embedStore").embedStore; + let isLinkReady: (props: { embedStore: EmbedStore }) => boolean; + let 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.providedCorrectHeightToParent = true; embedStore.renderState = null; embedStore.connectVersion = 1; }); @@ -72,11 +73,11 @@ describe("isLinkReady", async () => { }); }); - describe("when parent not informed about content height", () => { + describe("when parent not provided correct 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 + embedStore.providedCorrectHeightToParent = false; // Not informed yet const result = isLinkReady({ embedStore }); expect(result).toBe(false); diff --git a/packages/embeds/embed-core/src/embed-iframe/__tests__/utils.test.ts b/packages/embeds/embed-core/src/embed-iframe/__tests__/utils.test.ts new file mode 100644 index 0000000000..4fc29212d5 --- /dev/null +++ b/packages/embeds/embed-core/src/embed-iframe/__tests__/utils.test.ts @@ -0,0 +1,230 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +import { sdkActionManager } from "../../sdk-event"; +import { embedStore } from "../lib/embedStore"; +import { keepParentInformedAboutDimensionChanges } from "../lib/utils"; +import { fakeCurrentDocumentUrl } from "./test-utils"; + +type DimensionEvent = { + isFirstTime: boolean; + height: number; + width: number; +}; + +type DOMTestData = { + documentElementScrollDimensions: { height: number; width: number }; + mainElementDimensions: { height: number; width: number }; + readyState: "complete" | "loading"; +}; + +type DomEnvironment = DOMTestData & { + setReadyState: (readyState: "complete" | "loading") => void; +}; + +const RUN_ASAP_TICK_MS = 50; +const WINDOW_LOAD_COMPLETE_CONFIRMED_DELAY_MS = 100; + +const createDOMEnvironment = (testData: DOMTestData) => { + Object.defineProperty(document, "readyState", { + writable: true, + configurable: true, + value: testData.readyState, + }); + + const mainElement = document.createElement("main"); + mainElement.className = "main"; + document.body.appendChild(mainElement); + + Object.defineProperty(document.documentElement, "scrollHeight", { + writable: true, + configurable: true, + value: testData.documentElementScrollDimensions.height, + }); + Object.defineProperty(document.documentElement, "scrollWidth", { + writable: true, + configurable: true, + value: testData.documentElementScrollDimensions.width, + }); + + const originalGetComputedStyle = window.getComputedStyle; + vi.spyOn(window, "getComputedStyle").mockImplementation((element) => { + if (element.classList?.contains("main") || element.tagName === "MAIN") { + return { + height: `${testData.mainElementDimensions.height}px`, + width: `${testData.mainElementDimensions.width}px`, + marginTop: "0px", + marginBottom: "0px", + marginLeft: "0px", + marginRight: "0px", + } as CSSStyleDeclaration; + } + return originalGetComputedStyle(element); + }); + + return { + ...testData, + setReadyState: (readyState: "complete" | "loading") => { + Object.defineProperty(document, "readyState", { + writable: true, + configurable: true, + value: readyState, + }); + }, + }; +}; + +const resetEmbedState = () => { + embedStore.providedCorrectHeightToParent = false; + embedStore.windowLoadEventFired = false; + fakeCurrentDocumentUrl({ params: {} }); +}; + +const createDimensionTracker = () => { + const events: DimensionEvent[] = []; + sdkActionManager?.on("__dimensionChanged", (e) => { + events.push({ + isFirstTime: e.detail.data.isFirstTime, + height: e.detail.data.iframeHeight, + width: e.detail.data.iframeWidth, + }); + }); + return events; +}; + +const expectCorrectHeightNotProvided = () => { + expect(embedStore.providedCorrectHeightToParent).toBe(false); +}; + +const expectCorrectHeightProvided = () => { + expect(embedStore.providedCorrectHeightToParent).toBe(true); +}; + +const expectFirstPassEvent = ( + event: DimensionEvent | undefined, + { + documentElementScrollHeight, + documentElementScrollWidth, + }: { + documentElementScrollHeight: number; + documentElementScrollWidth: number; + } +) => { + expect(event).toBeDefined(); + expect(event?.isFirstTime).toBe(true); + expect(event?.height).toBe(documentElementScrollHeight); + expect(event?.width).toBe(documentElementScrollWidth); +}; + +const expectSecondPassEvent = ( + event: DimensionEvent | undefined, + { + mainElementHeight, + mainElementWidth, + }: { + mainElementHeight: number; + mainElementWidth: number; + } +) => { + expect(event).toBeDefined(); + expect(event?.isFirstTime).toBe(false); + expect(event?.height).toBe(mainElementHeight); + expect(event?.width).toBe(mainElementWidth); +}; + +beforeEach(() => { + vi.useFakeTimers(); +}); + +afterEach(() => { + vi.clearAllMocks(); + vi.useRealTimers(); + document.body.innerHTML = ""; +}); + +describe("keepParentInformedAboutDimensionChanges", () => { + let domEnvironment: DomEnvironment; + beforeEach(() => { + domEnvironment = createDOMEnvironment({ + documentElementScrollDimensions: { height: 1000, width: 800 }, + mainElementDimensions: { height: 500, width: 400 }, + readyState: "complete", + }); + resetEmbedState(); + }); + + describe("when iframe dimensions are calculated", () => { + it("should not mark height as provided during initial dimension pass", async () => { + const dimensionEvents = createDimensionTracker(); + + keepParentInformedAboutDimensionChanges({ embedStore }); + await vi.advanceTimersByTimeAsync(RUN_ASAP_TICK_MS + WINDOW_LOAD_COMPLETE_CONFIRMED_DELAY_MS); + expectCorrectHeightNotProvided(); + const firstEvent = dimensionEvents[0]; + expectFirstPassEvent(firstEvent, { + documentElementScrollHeight: domEnvironment.documentElementScrollDimensions.height, + documentElementScrollWidth: domEnvironment.documentElementScrollDimensions.width, + }); + }); + + it("should mark height as provided after second dimension pass completes", async () => { + const dimensionEvents = createDimensionTracker(); + + keepParentInformedAboutDimensionChanges({ embedStore }); + + await vi.advanceTimersByTimeAsync(RUN_ASAP_TICK_MS + WINDOW_LOAD_COMPLETE_CONFIRMED_DELAY_MS); + expectCorrectHeightNotProvided(); + + await vi.advanceTimersByTimeAsync(RUN_ASAP_TICK_MS); + expectCorrectHeightProvided(); + const secondEvent = dimensionEvents[1]; + expectSecondPassEvent(secondEvent, { + mainElementHeight: domEnvironment.mainElementDimensions.height, + mainElementWidth: domEnvironment.mainElementDimensions.width, + }); + }); + }); + + describe("when window load state changes", () => { + it("should wait for window load completion before calculating dimensions", async () => { + domEnvironment = createDOMEnvironment({ + documentElementScrollDimensions: { height: 1000, width: 800 }, + mainElementDimensions: { height: 500, width: 400 }, + readyState: "loading", + }); + + const dimensionEvents = createDimensionTracker(); + + keepParentInformedAboutDimensionChanges({ embedStore }); + + await vi.advanceTimersByTimeAsync(RUN_ASAP_TICK_MS + WINDOW_LOAD_COMPLETE_CONFIRMED_DELAY_MS); + + expect(dimensionEvents.length).toBe(0); + expectCorrectHeightNotProvided(); + + domEnvironment.setReadyState("complete"); + + await vi.advanceTimersByTimeAsync(RUN_ASAP_TICK_MS + WINDOW_LOAD_COMPLETE_CONFIRMED_DELAY_MS); + expect(dimensionEvents.length).toBe(1); + }); + + it("should fire window load complete event on first pass", async () => { + domEnvironment = createDOMEnvironment({ + documentElementScrollDimensions: { height: 1000, width: 800 }, + mainElementDimensions: { height: 500, width: 400 }, + readyState: "complete", + }); + + const windowLoadEvents: unknown[] = []; + sdkActionManager?.on("__windowLoadComplete", () => { + windowLoadEvents.push({}); + }); + + keepParentInformedAboutDimensionChanges({ embedStore }); + + await vi.advanceTimersByTimeAsync(RUN_ASAP_TICK_MS + WINDOW_LOAD_COMPLETE_CONFIRMED_DELAY_MS); + + expect(windowLoadEvents.length).toBe(1); + expect(embedStore.windowLoadEventFired).toBe(true); + }); + }); +}); diff --git a/packages/embeds/embed-core/src/embed-iframe/lib/embedStore.ts b/packages/embeds/embed-core/src/embed-iframe/lib/embedStore.ts index ab16c24dfd..44a73044f0 100644 --- a/packages/embeds/embed-core/src/embed-iframe/lib/embedStore.ts +++ b/packages/embeds/embed-core/src/embed-iframe/lib/embedStore.ts @@ -106,7 +106,7 @@ export const embedStore = { reactStylesStateSetters: {} as Record, reactNonStylesStateSetters: {} as Record, // Embed can show itself only after this is set to true - parentInformedAboutContentHeight: false, + providedCorrectHeightToParent: false, windowLoadEventFired: false, setTheme: undefined as ((arg0: EmbedThemeConfig) => void) | undefined, theme: undefined as UiConfig["theme"], @@ -116,3 +116,5 @@ export const embedStore = { */ setUiConfig: [] as ((arg0: UiConfig) => void)[], }; + +export type EmbedStore = typeof embedStore; 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 c1dfa15590..c0606c7d39 100644 --- a/packages/embeds/embed-core/src/embed-iframe/lib/utils.ts +++ b/packages/embeds/embed-core/src/embed-iframe/lib/utils.ts @@ -1,3 +1,6 @@ +import { sdkActionManager } from "../../sdk-event"; +import { type EmbedStore } from "../lib/embedStore"; + export function runAsap(fn: (...arg: unknown[]) => void) { // We don't use rAF because it runs slower in Safari plus doesn't run if the iframe is hidden sometimes return setTimeout(fn, 50); @@ -22,8 +25,8 @@ function isSkeletonSupportedPageType() { /** * 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 */ -export function isLinkReady({ embedStore }: { embedStore: typeof import("./embedStore").embedStore }) { - if (!embedStore.parentInformedAboutContentHeight) { +export function isLinkReady({ embedStore }: { embedStore: EmbedStore }) { + if (!embedStore.providedCorrectHeightToParent) { return false; } @@ -40,6 +43,107 @@ export function isLinkReady({ embedStore }: { embedStore: typeof import("./embed return true; } +/** + * This function is called once the iframe loads. + * It isn't called on "connect" + * + * Two-pass dimension strategy: + * 1. Initial pass: Uses document scroll dimensions to provide maximum space and avoid affecting layout + * 2. Second pass: Updates with correct dimensions of main container after parent adjusts iframe + * + * The correct iframe height is only available after the second pass, so we mark providedCorrectHeightToParent=true + * when the second pass runs (regardless of whether dimensions changed or not). + */ +export function keepParentInformedAboutDimensionChanges({ embedStore }: { embedStore: EmbedStore }) { + let knownIframeHeight: number | null = null; + let knownIframeWidth: number | null = null; + let isInitialDimensionPass = true; + let isWindowLoadComplete = false; + runAsap(function informAboutScroll() { + if (document.readyState !== "complete") { + // Wait for window to load to correctly calculate the initial scroll height. + runAsap(informAboutScroll); + return; + } + + if (!isWindowLoadComplete) { + // On Safari, even though document.readyState is complete, still the page is not rendered and we can't compute documentElement.scrollHeight correctly + // Postponing to just next cycle allow us to fix this. + setTimeout(() => { + isWindowLoadComplete = true; + informAboutScroll(); + }, 100); + return; + } + + if (!embedStore.windowLoadEventFired) { + sdkActionManager?.fire("__windowLoadComplete", {}); + } + embedStore.windowLoadEventFired = true; + + // Use the dimensions of main element as in most places there is max-width restriction on it and we just want to show the main content. + // It avoids the unwanted padding outside main tag. + const mainElement = + document.getElementsByClassName("main")[0] || + document.getElementsByTagName("main")[0] || + document.documentElement; + const documentScrollHeight = document.documentElement.scrollHeight; + const documentScrollWidth = document.documentElement.scrollWidth; + + if (!(mainElement instanceof HTMLElement)) { + throw new Error("Main element should be an HTMLElement"); + } + + const mainElementStyles = getComputedStyle(mainElement); + // Use, .height as that gives more accurate value in floating point. Also, do a ceil on the total sum so that whatever happens there is enough iframe size to avoid scroll. + const contentHeight = Math.ceil( + parseFloat(mainElementStyles.height) + + parseFloat(mainElementStyles.marginTop) + + parseFloat(mainElementStyles.marginBottom) + ); + const contentWidth = Math.ceil( + parseFloat(mainElementStyles.width) + + parseFloat(mainElementStyles.marginLeft) + + parseFloat(mainElementStyles.marginRight) + ); + + // During first render let iframe tell parent that how much is the expected height to avoid scroll. + // Parent would set the same value as the height of iframe which would prevent scroll. + // On subsequent renders, consider html height as the height of the iframe. If we don't do this, then if iframe gets bigger in height, it would never shrink + const iframeHeight = isInitialDimensionPass ? documentScrollHeight : contentHeight; + const iframeWidth = isInitialDimensionPass ? documentScrollWidth : contentWidth; + + if (!iframeHeight || !iframeWidth) { + runAsap(informAboutScroll); + return; + } + const isThereAChangeInDimensions = knownIframeHeight !== iframeHeight || knownIframeWidth !== iframeWidth; + if (isThereAChangeInDimensions || !embedStore.providedCorrectHeightToParent) { + knownIframeHeight = iframeHeight; + knownIframeWidth = iframeWidth; + + // FIXME: This event shouldn't be subscribable by the user. Only by the SDK. + sdkActionManager?.fire("__dimensionChanged", { + iframeHeight, + iframeWidth, + isFirstTime: isInitialDimensionPass, + }); + } + + // After the initial pass, we've provided the correct height to parent + // This happens on second pass regardless of whether dimensions changed + if (!isInitialDimensionPass) { + embedStore.providedCorrectHeightToParent = true; + } + + isInitialDimensionPass = false; + // Parent Counterpart would change the dimension of iframe and thus page's dimension would be impacted which is recursive. + // It should stop ideally by reaching a hiddenHeight value of 0. + // FIXME: If 0 can't be reached we need to just abandon our quest for perfect iframe and let scroll be there. Such case can be logged in the wild and fixed later on. + runAsap(informAboutScroll); + }); +} + /** * Moves the queuedFormResponse to the routingFormResponse record to mark it as an actual response now. */