Skip to content

Commit bed76be

Browse files
fix: Layout shift in inline embed caused due to showing embed before it was really ready (calcom#24427)
1 parent ff38d6c commit bed76be

5 files changed

Lines changed: 354 additions & 101 deletions

File tree

packages/embeds/embed-core/src/embed-iframe.ts

Lines changed: 9 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,13 @@ import { useEffect, useRef, useState, useCallback } from "react";
44

55
import type { Message } from "./embed";
66
import { embedStore, EMBED_IFRAME_STATE } from "./embed-iframe/lib/embedStore";
7-
import { runAsap, isBookerReady, isLinkReady, recordResponseIfQueued } from "./embed-iframe/lib/utils";
7+
import {
8+
runAsap,
9+
isBookerReady,
10+
isLinkReady,
11+
recordResponseIfQueued,
12+
keepParentInformedAboutDimensionChanges,
13+
} from "./embed-iframe/lib/utils";
814
import { sdkActionManager } from "./sdk-event";
915
import type {
1016
UiConfig,
@@ -195,7 +201,6 @@ export const useEmbedStyles = (elementName: keyof EmbedStyles) => {
195201

196202
useEffect(() => {
197203
return registerNewSetter({ elementName, setState: setStyles, styles: true });
198-
// eslint-disable-next-line react-hooks/exhaustive-deps
199204
}, []);
200205
const styles = embedStore.styles || {};
201206
// 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)
207212

208213
useEffect(() => {
209214
return registerNewSetter({ elementName, setState: setNonStyles, styles: false });
210-
// eslint-disable-next-line react-hooks/exhaustive-deps
211215
}, []);
212216

213217
// Always read the data from global embedStore so that even across components, the same data is used.
@@ -387,7 +391,6 @@ export const methods = {
387391
setEmbedStyles(stylesConfig || {});
388392
setEmbedNonStyles(stylesConfig || {});
389393
},
390-
// eslint-disable-next-line @typescript-eslint/no-unused-vars
391394
parentKnowsIframeReady: (_unused: unknown) => {
392395
log("Method: `parentKnowsIframeReady` called");
393396
// 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 = {
425428
...queryParamsFromConfig
426429
} = config;
427430
// We reset it to allow informing parent again through `__dimensionChanged` event about possibly updated dimensions with changes in config
428-
embedStore.parentInformedAboutContentHeight = false;
431+
embedStore.providedCorrectHeightToParent = false;
429432

430433
if (noSlotsFetchOnConnect !== "true") {
431434
log("Method: connect, noSlotsFetchOnConnect is false. Requesting slots re-fetch");
@@ -484,93 +487,6 @@ const messageParent = (data: CustomEvent["detail"]) => {
484487
);
485488
};
486489

487-
/**
488-
* This function is called once the iframe loads.
489-
* It isn't called on "connect"
490-
*/
491-
function keepParentInformedAboutDimensionChanges() {
492-
let knownIframeHeight: number | null = null;
493-
let knownIframeWidth: number | null = null;
494-
let isFirstTime = true;
495-
let isWindowLoadComplete = false;
496-
runAsap(function informAboutScroll() {
497-
if (document.readyState !== "complete") {
498-
// Wait for window to load to correctly calculate the initial scroll height.
499-
runAsap(informAboutScroll);
500-
return;
501-
}
502-
if (!isWindowLoadComplete) {
503-
// On Safari, even though document.readyState is complete, still the page is not rendered and we can't compute documentElement.scrollHeight correctly
504-
// Postponing to just next cycle allow us to fix this.
505-
setTimeout(() => {
506-
isWindowLoadComplete = true;
507-
informAboutScroll();
508-
}, 100);
509-
return;
510-
}
511-
512-
if (!embedStore.windowLoadEventFired) {
513-
sdkActionManager?.fire("__windowLoadComplete", {});
514-
}
515-
embedStore.windowLoadEventFired = true;
516-
517-
// 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.
518-
// It avoids the unwanted padding outside main tag.
519-
const mainElement =
520-
document.getElementsByClassName("main")[0] ||
521-
document.getElementsByTagName("main")[0] ||
522-
document.documentElement;
523-
const documentScrollHeight = document.documentElement.scrollHeight;
524-
const documentScrollWidth = document.documentElement.scrollWidth;
525-
526-
if (!(mainElement instanceof HTMLElement)) {
527-
throw new Error("Main element should be an HTMLElement");
528-
}
529-
530-
const mainElementStyles = getComputedStyle(mainElement);
531-
// 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.
532-
const contentHeight = Math.ceil(
533-
parseFloat(mainElementStyles.height) +
534-
parseFloat(mainElementStyles.marginTop) +
535-
parseFloat(mainElementStyles.marginBottom)
536-
);
537-
const contentWidth = Math.ceil(
538-
parseFloat(mainElementStyles.width) +
539-
parseFloat(mainElementStyles.marginLeft) +
540-
parseFloat(mainElementStyles.marginRight)
541-
);
542-
543-
// During first render let iframe tell parent that how much is the expected height to avoid scroll.
544-
// Parent would set the same value as the height of iframe which would prevent scroll.
545-
// 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
546-
const iframeHeight = isFirstTime ? documentScrollHeight : contentHeight;
547-
const iframeWidth = isFirstTime ? documentScrollWidth : contentWidth;
548-
549-
if (!iframeHeight || !iframeWidth) {
550-
runAsap(informAboutScroll);
551-
return;
552-
}
553-
const isThereAChangeInDimensions = knownIframeHeight !== iframeHeight || knownIframeWidth !== iframeWidth;
554-
if (isThereAChangeInDimensions || !embedStore.parentInformedAboutContentHeight) {
555-
embedStore.parentInformedAboutContentHeight = true;
556-
557-
knownIframeHeight = iframeHeight;
558-
knownIframeWidth = iframeWidth;
559-
// FIXME: This event shouldn't be subscribable by the user. Only by the SDK.
560-
sdkActionManager?.fire("__dimensionChanged", {
561-
iframeHeight,
562-
iframeWidth,
563-
isFirstTime,
564-
});
565-
}
566-
isFirstTime = false;
567-
// Parent Counterpart would change the dimension of iframe and thus page's dimension would be impacted which is recursive.
568-
// It should stop ideally by reaching a hiddenHeight value of 0.
569-
// 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.
570-
runAsap(informAboutScroll);
571-
});
572-
}
573-
574490
function main() {
575491
if (!isBrowser) {
576492
return;
@@ -659,7 +575,7 @@ function initializeAndSetupEmbed() {
659575
const pageStatus = window.CalComPageStatus;
660576

661577
if (!pageStatus || pageStatus == "200") {
662-
keepParentInformedAboutDimensionChanges();
578+
keepParentInformedAboutDimensionChanges({ embedStore });
663579
} else
664580
sdkActionManager?.fire("linkFailed", {
665581
code: pageStatus,

packages/embeds/embed-core/src/embed-iframe/__tests__/isLinkReady.test.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
22

3+
import { type EmbedStore } from "../lib/embedStore";
34
import { fakeCurrentDocumentUrl, takeBookerToSlotsLoadingState, takeBookerToReadyState } from "./test-utils";
45

56
beforeEach(() => {
@@ -13,15 +14,15 @@ afterEach(() => {
1314
});
1415

1516
describe("isLinkReady", async () => {
16-
let isLinkReady: typeof import("../lib/utils").isLinkReady;
17-
let embedStore: typeof import("../lib/embedStore").embedStore;
17+
let isLinkReady: (props: { embedStore: EmbedStore }) => boolean;
18+
let embedStore: EmbedStore;
1819

1920
beforeEach(async () => {
2021
({ isLinkReady } = await import("../lib/utils"));
2122
({ embedStore } = await import("../lib/embedStore"));
2223

2324
// Reset embedStore state to ensure test isolation
24-
embedStore.parentInformedAboutContentHeight = true;
25+
embedStore.providedCorrectHeightToParent = true;
2526
embedStore.renderState = null;
2627
embedStore.connectVersion = 1;
2728
});
@@ -72,11 +73,11 @@ describe("isLinkReady", async () => {
7273
});
7374
});
7475

75-
describe("when parent not informed about content height", () => {
76+
describe("when parent not provided correct height", () => {
7677
it("should return false regardless of other conditions", () => {
7778
fakeCurrentDocumentUrl({ params: { "cal.embed.pageType": "user.event.booking.slots" } });
7879
takeBookerToReadyState();
79-
embedStore.parentInformedAboutContentHeight = false; // Not informed yet
80+
embedStore.providedCorrectHeightToParent = false; // Not informed yet
8081

8182
const result = isLinkReady({ embedStore });
8283
expect(result).toBe(false);

0 commit comments

Comments
 (0)