Skip to content

Commit d836497

Browse files
fix: Don't AutoScroll to embed when multiple-duration event's booking page is embedded (#16411)
* Fix and add test and eslint rule * Fix eslint errors --------- Co-authored-by: Peer Richelsen <peeroke@gmail.com>
1 parent 38fa0fa commit d836497

File tree

17 files changed

+129
-21
lines changed

17 files changed

+129
-21
lines changed

apps/web/components/booking/CancelBooking.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export default function CancelBooking(props: Props) {
4747

4848
const cancelBookingRef = useCallback((node: HTMLTextAreaElement) => {
4949
if (node !== null) {
50+
// eslint-disable-next-line @calcom/eslint/no-scroll-into-view-embed -- CancelBooking is not usually used in embed mode
5051
node.scrollIntoView({ behavior: "smooth" });
5152
node.focus();
5253
}

apps/web/playwright/fixtures/users.ts

+6
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,12 @@ export const createUsersFixture = (
292292
{ title: "Paid", slug: "paid", length: 30, price: 1000 },
293293
{ title: "Opt in", slug: "opt-in", requiresConfirmation: true, length: 30 },
294294
{ title: "Seated", slug: "seated", seatsPerTimeSlot: 2, length: 30 },
295+
{
296+
title: "Multiple duration",
297+
slug: "multiple-duration",
298+
length: 30,
299+
metadata: { multipleDuration: [30, 60, 90] },
300+
},
295301
];
296302

297303
if (opts?.eventTypes) defaultEventTypes = defaultEventTypes.concat(opts.eventTypes);

packages/embeds/embed-core/index.html

+4
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,10 @@ <h3><a href="?only=ns:weekView">Test Week View</a></h3>
417417
<h3><a href="?only=ns:columnView">Test Column View</a></h3>
418418
<div class="place" style="width: 100%"></div>
419419
</div>
420+
<div class="inline-embed-container" id="cal-booking-place-autoScrollTest">
421+
<h3><a href="?only=ns:autoScrollTest">Test Auto Scroll</a></h3>
422+
<div class="place" style="width: 100%"></div>
423+
</div>
420424
<script type="module" src="./playground.ts"></script>
421425
</body>
422426
</html>

packages/embeds/embed-core/playground.ts

+16
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,22 @@ if (only === "all" || only == "ns:columnView") {
531531
});
532532
}
533533

534+
if (only === "all" || only == "ns:autoScrollTest") {
535+
if (!calLink) {
536+
throw new Error("cal-link parameter is required for autoScrollTest");
537+
}
538+
Cal("init", "autoScrollTest", {
539+
debug: true,
540+
origin: origin,
541+
});
542+
Cal.ns.autoScrollTest("inline", {
543+
elementOrSelector: "#cal-booking-place-autoScrollTest .place",
544+
calLink: calLink,
545+
config: {
546+
"flag.coep": "true",
547+
},
548+
});
549+
}
534550
// Verifies that the type of e.detail.data is valid. type-check will fail if we accidentally break it.
535551
const bookingSuccessfulV2Callback = (e: EmbedEvent<"bookingSuccessfulV2">) => {
536552
const data = e.detail.data;

packages/embeds/embed-core/playwright/lib/testUtils.ts

+19
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ export const getBooking = async (bookingId: string) => {
2929
return booking;
3030
};
3131

32+
/**
33+
* @deprecated use ensureEmbedIframe instead.
34+
*/
3235
export const getEmbedIframe = async ({
3336
calNamespace,
3437
page,
@@ -59,6 +62,22 @@ export const getEmbedIframe = async ({
5962
return null;
6063
};
6164

65+
export const ensureEmbedIframe = async ({
66+
calNamespace,
67+
page,
68+
pathname,
69+
}: {
70+
calNamespace: string;
71+
page: Page;
72+
pathname: string;
73+
}) => {
74+
const embedIframe = await getEmbedIframe({ calNamespace, page, pathname });
75+
if (!embedIframe) {
76+
throw new Error("Embed iframe not found");
77+
}
78+
return embedIframe;
79+
};
80+
6281
async function selectFirstAvailableTimeSlotNextMonth(frame: Frame, page: Page) {
6382
await frame.click('[data-testid="incrementMonth"]');
6483

packages/embeds/embed-core/playwright/tests/inline.e2e.ts

+17-12
Original file line numberDiff line numberDiff line change
@@ -6,29 +6,23 @@ import {
66
assertNoRequestIsBlocked,
77
bookFirstEvent,
88
deleteAllBookingsByEmail,
9-
getEmbedIframe,
9+
ensureEmbedIframe,
1010
} from "../lib/testUtils";
1111

1212
test.describe("Inline Iframe", () => {
13-
test("Inline Iframe - Configured with Dark Theme. Do booking and verify that COEP/CORP headers are correctly set", async ({
13+
test("Configured with Dark Theme. Do booking and verify that COEP/CORP headers are correctly set", async ({
1414
page,
15-
embeds: { addEmbedListeners, getActionFiredDetails },
15+
embeds,
1616
}) => {
1717
await deleteAllBookingsByEmail("embed-user@example.com");
18-
await addEmbedListeners("");
19-
await page.goto("/?only=ns:default");
18+
await embeds.gotoPlayground({ calNamespace: "", url: "/?only=ns:default" });
2019
const calNamespace = "";
21-
const embedIframe = await getEmbedIframe({ calNamespace, page, pathname: "/pro" });
22-
expect(embedIframe).toBeEmbedCalLink(calNamespace, getActionFiredDetails, {
23-
pathname: "/pro",
20+
const embedIframe = await ensureEmbedIframe({ calNamespace, page, pathname: "/pro" });
21+
expect(embedIframe).toBeEmbedCalLink(calNamespace, embeds.getActionFiredDetails, {
2422
searchParams: {
2523
theme: "dark",
2624
},
2725
});
28-
// expect(await page.screenshot()).toMatchSnapshot("event-types-list.png");
29-
if (!embedIframe) {
30-
throw new Error("Embed iframe not found");
31-
}
3226

3327
assertNoRequestIsBlocked(page);
3428

@@ -57,6 +51,17 @@ test.describe("Inline Iframe", () => {
5751
});
5852
});
5953

54+
test("Ensure iframe doesn't hijack scroll in embed mode", async ({ page, embeds, users }) => {
55+
const user = await users.create();
56+
const calNamespace = "autoScrollTest";
57+
await embeds.gotoPlayground({ calNamespace, url: `?only=ns:autoScrollTest` });
58+
const calLink = `${user.username}/multiple-duration`;
59+
await page.goto(`/?only=ns:autoScrollTest&cal-link=${calLink}`);
60+
const embedIframe = await ensureEmbedIframe({ calNamespace, page, pathname: `/${calLink}` });
61+
const finalScrollPosition = await page.evaluate(() => window.scrollY);
62+
expect(finalScrollPosition).toBe(0);
63+
});
64+
6065
todo(
6166
"Ensure that on all pages - [user], [user]/[type], team/[slug], team/[slug]/book, UI styling works if these pages are directly linked in embed"
6267
);

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

+1
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,7 @@ export class Cal {
401401
// Try to readjust and scroll into view if more than 25% is hidden.
402402
// Otherwise we assume that user might have positioned the content appropriately already
403403
if (top < 0 && Math.abs(top / height) >= 0.25) {
404+
// eslint-disable-next-line @calcom/eslint/no-scroll-into-view-embed -- Intentionally done
404405
this.inlineEl.scrollIntoView({ behavior: "smooth" });
405406
}
406407
});

packages/eslint-plugin/src/configs/recommended.ts

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const recommended = {
77
"@calcom/eslint/avoid-web-storage": "error",
88
"@calcom/eslint/avoid-prisma-client-import-for-enums": "error",
99
"@calcom/eslint/no-prisma-include-true": "warn",
10+
"@calcom/eslint/no-scroll-into-view-embed": "error",
1011
},
1112
};
1213

packages/eslint-plugin/src/rules/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,5 @@ export default {
77
"avoid-prisma-client-import-for-enums": require("./avoid-prisma-client-import-for-enums").default,
88
"no-prisma-include-true": require("./no-prisma-include-true").default,
99
"deprecated-imports-next-router": require("./deprecated-imports-next-router").default,
10+
"no-scroll-into-view-embed": require("./no-scroll-into-view-embed").default,
1011
} as ESLint.Plugin["rules"];
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import type { TSESTree } from "@typescript-eslint/utils";
2+
import { ESLintUtils } from "@typescript-eslint/utils";
3+
4+
const createRule = ESLintUtils.RuleCreator((name) => `https://developer.cal.com/eslint/rule/${name}`);
5+
6+
export default createRule({
7+
name: "no-scroll-into-view-embed",
8+
meta: {
9+
docs: {
10+
description: "Disallow usage of scrollIntoView in embed mode",
11+
recommended: "error",
12+
},
13+
messages: {
14+
noScrollIntoViewForEmbed:
15+
"Make sure to call scrollIntoView conditionally if it is called without user action. Use useIsEmbed() to detect if embed mode and then don't call it for embed case.",
16+
},
17+
type: "problem",
18+
schema: [],
19+
},
20+
defaultOptions: [],
21+
create(context) {
22+
return {
23+
CallExpression(node: TSESTree.CallExpression) {
24+
const { callee } = node;
25+
26+
if (callee.type === "MemberExpression") {
27+
if (callee.property.type === "Identifier" && callee.property.name === "scrollIntoView") {
28+
context.report({
29+
node,
30+
messageId: "noScrollIntoViewForEmbed",
31+
});
32+
}
33+
} else if (callee.type === "Identifier" && callee.name === "scrollIntoView") {
34+
context.report({
35+
node,
36+
messageId: "noScrollIntoViewForEmbed",
37+
});
38+
}
39+
},
40+
};
41+
},
42+
});

packages/features/bookings/Booker/Booker.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ const BookerComponent = ({
138138
const scrolledToTimeslotsOnce = useRef(false);
139139
const scrollToTimeSlots = () => {
140140
if (isMobile && !isEmbed && !scrolledToTimeslotsOnce.current) {
141+
// eslint-disable-next-line @calcom/eslint/no-scroll-into-view-embed -- Conditional within !isEmbed condition
141142
timeslotsRef.current?.scrollIntoView({ behavior: "smooth" });
142143
scrolledToTimeslotsOnce.current = true;
143144
}

packages/features/bookings/Booker/components/hooks/useBookings.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@ export const useBookings = ({ event, hashedLink, bookingForm, metadata, teamMemb
231231
});
232232
},
233233
onError: (err, _, ctx) => {
234+
// eslint-disable-next-line @calcom/eslint/no-scroll-into-view-embed -- It is only called when user takes an action in embed
234235
bookerFormErrorRef && bookerFormErrorRef.current?.scrollIntoView({ behavior: "smooth" });
235236
},
236237
});
@@ -243,7 +244,7 @@ export const useBookings = ({ event, hashedLink, bookingForm, metadata, teamMemb
243244
},
244245
onError: (err, _, ctx) => {
245246
console.error("Error creating instant booking", err);
246-
247+
// eslint-disable-next-line @calcom/eslint/no-scroll-into-view-embed -- It is only called when user takes an action in embed
247248
bookerFormErrorRef && bookerFormErrorRef.current?.scrollIntoView({ behavior: "smooth" });
248249
},
249250
});

packages/features/bookings/components/event-meta/Duration.tsx

+5-2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { TFunction } from "next-i18next";
22
import { useEffect, useRef } from "react";
33

44
import { useIsPlatform } from "@calcom/atoms/monorepo";
5+
import { useIsEmbed } from "@calcom/embed-core/embed-iframe";
56
import { useBookerStore } from "@calcom/features/bookings/Booker/store";
67
import type { BookerEvent } from "@calcom/features/bookings/types";
78
import { classNames } from "@calcom/lib";
@@ -64,7 +65,7 @@ export const EventDuration = ({
6465
};
6566

6667
const isDynamicEvent = "isDynamic" in event && event.isDynamic;
67-
68+
const isEmbed = useIsEmbed();
6869
// Sets initial value of selected duration to the default duration.
6970
useEffect(() => {
7071
// Only store event duration in url if event has multiple durations.
@@ -74,7 +75,9 @@ export const EventDuration = ({
7475

7576
useEffect(() => {
7677
const timeout = setTimeout(() => {
78+
if (isEmbed) return;
7779
if (selectedDuration && itemRefs.current[selectedDuration]) {
80+
// eslint-disable-next-line @calcom/eslint/no-scroll-into-view-embed -- Called on !isEmbed case
7881
itemRefs.current[selectedDuration]?.scrollIntoView({
7982
behavior: "smooth",
8083
block: "center",
@@ -83,7 +86,7 @@ export const EventDuration = ({
8386
}
8487
}, 100);
8588
return () => clearTimeout(timeout);
86-
}, [selectedDuration]);
89+
}, [selectedDuration, isEmbed]);
8790

8891
if ((!event?.metadata?.multipleDuration && !isDynamicEvent) || isPlatform)
8992
return <>{getDurationFormatted(event.length, t)}</>;

packages/features/calendars/weeklyview/components/currentTime/index.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export function CurrentTime() {
3939
if (!currentTimeRef.current || scrolledIntoView) return;
4040
// Within a small timeout so element has time to render.
4141
setTimeout(() => {
42+
// eslint-disable-next-line @calcom/eslint/no-scroll-into-view-embed -- Does't seem to cause any issue. Put it under condition if needed
4243
currentTimeRef?.current?.scrollIntoView({ block: "center" });
4344
setScrolledIntoView(true);
4445
}, 100);

packages/features/settings/appDir/SettingsLayoutAppDirClient.tsx

+2
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,7 @@ const TeamListCollapsible = () => {
259259
const tabMembers = Array.from(document.getElementsByTagName("a")).filter(
260260
(bottom) => bottom.dataset.testid === "vertical-tab-Members"
261261
)[1];
262+
// eslint-disable-next-line @calcom/eslint/no-scroll-into-view-embed -- Settings layout isn't embedded
262263
tabMembers?.scrollIntoView({ behavior: "smooth" });
263264
}, 100);
264265
}
@@ -418,6 +419,7 @@ const SettingsSidebarContainer = ({
418419
const tabMembers = Array.from(document.getElementsByTagName("a")).filter(
419420
(bottom) => bottom.dataset.testid === "vertical-tab-Members"
420421
)[1];
422+
// eslint-disable-next-line @calcom/eslint/no-scroll-into-view-embed -- Settings layout isn't embedded
421423
tabMembers?.scrollIntoView({ behavior: "smooth" });
422424
}, 100);
423425
}

packages/features/settings/layouts/SettingsLayout.tsx

+2
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,7 @@ const TeamListCollapsible = () => {
259259
const tabMembers = Array.from(document.getElementsByTagName("a")).filter(
260260
(bottom) => bottom.dataset.testid === "vertical-tab-Members"
261261
)[1];
262+
// eslint-disable-next-line @calcom/eslint/no-scroll-into-view-embed -- Settings layout isn't embedded
262263
tabMembers?.scrollIntoView({ behavior: "smooth" });
263264
}, 100);
264265
}
@@ -424,6 +425,7 @@ const SettingsSidebarContainer = ({
424425
const tabMembers = Array.from(document.getElementsByTagName("a")).filter(
425426
(bottom) => bottom.dataset.testid === "vertical-tab-Members"
426427
)[1];
428+
// eslint-disable-next-line @calcom/eslint/no-scroll-into-view-embed -- Settings layout isn't embedded
427429
tabMembers?.scrollIntoView({ behavior: "smooth" });
428430
}, 100);
429431
}

playwright.config.ts

+8-6
Original file line numberDiff line numberDiff line change
@@ -200,12 +200,14 @@ expect.extend({
200200
const u = new URL(iframe.url());
201201

202202
const pathname = u.pathname;
203-
const expectedPathname = `${expectedUrlDetails.pathname}/embed`;
204-
if (expectedPathname && expectedPathname !== pathname) {
205-
return {
206-
pass: false,
207-
message: () => `Expected pathname to be ${expectedPathname} but got ${pathname}`,
208-
};
203+
if (expectedUrlDetails.pathname) {
204+
const expectedPathname = `${expectedUrlDetails.pathname}/embed`;
205+
if (pathname !== expectedPathname) {
206+
return {
207+
pass: false,
208+
message: () => `Expected pathname to be ${expectedPathname} but got ${pathname}`,
209+
};
210+
}
209211
}
210212

211213
const origin = u.origin;

0 commit comments

Comments
 (0)