From 2c96444058d9f062829cfb75603ec7c226529e89 Mon Sep 17 00:00:00 2001 From: Hariom Balhara Date: Mon, 17 Apr 2023 17:46:54 +0530 Subject: [PATCH] Fix/Keep themes in peace across embed and booking pages and App (#8108) --- apps/web/components/booking/CancelBooking.tsx | 2 - apps/web/lib/app-providers.tsx | 57 +++++++++++++------ apps/web/pages/[user].tsx | 2 + apps/web/pages/[user]/[type].tsx | 2 + apps/web/pages/[user]/book.tsx | 2 + apps/web/pages/apps/[slug]/[...pages].tsx | 12 ++-- apps/web/pages/booking/[uid].tsx | 6 +- apps/web/pages/d/[link]/[slug].tsx | 2 + apps/web/pages/d/[link]/book.tsx | 2 + apps/web/pages/team/[slug].tsx | 2 + apps/web/pages/team/[slug]/[type].tsx | 2 + apps/web/pages/team/[slug]/book.tsx | 1 + .../pages/form-edit/[...appPages].tsx | 1 - .../pages/forms/[...appPages].tsx | 2 - .../pages/reporting/[...appPages].tsx | 2 - .../pages/route-builder/[...appPages].tsx | 1 - .../pages/router/[...appPages].tsx | 2 - .../pages/routing-link/[...appPages].tsx | 2 +- packages/features/shell/Shell.tsx | 4 +- packages/lib/hooks/useTheme.tsx | 29 ++++++---- 20 files changed, 86 insertions(+), 49 deletions(-) diff --git a/apps/web/components/booking/CancelBooking.tsx b/apps/web/components/booking/CancelBooking.tsx index eff581bde9..5bb7578d1b 100644 --- a/apps/web/components/booking/CancelBooking.tsx +++ b/apps/web/components/booking/CancelBooking.tsx @@ -2,7 +2,6 @@ import { useRouter } from "next/router"; import { useCallback, useState } from "react"; import { useLocale } from "@calcom/lib/hooks/useLocale"; -import useTheme from "@calcom/lib/hooks/useTheme"; import { collectPageParameters, telemetryEventTypes, useTelemetry } from "@calcom/lib/telemetry"; import type { RecurringEvent } from "@calcom/types/Calendar"; import { Button, TextArea } from "@calcom/ui"; @@ -34,7 +33,6 @@ export default function CancelBooking(props: Props) { const [loading, setLoading] = useState(false); const telemetry = useTelemetry(); const [error, setError] = useState(booking ? null : t("booking_already_cancelled")); - useTheme(props.theme); const cancelBookingRef = useCallback((node: HTMLTextAreaElement) => { if (node !== null) { diff --git a/apps/web/lib/app-providers.tsx b/apps/web/lib/app-providers.tsx index b793123e7e..dbefd11b2a 100644 --- a/apps/web/lib/app-providers.tsx +++ b/apps/web/lib/app-providers.tsx @@ -27,7 +27,8 @@ const I18nextAdapter = appWithTranslation & { children export type AppProps = Omit>, "Component"> & { Component: NextAppProps["Component"] & { requiresLicense?: boolean; - isThemeSupported?: boolean | ((arg: { router: NextRouter }) => boolean); + isThemeSupported?: boolean; + isBookingPage?: boolean | ((arg: { router: NextRouter }) => boolean); getLayout?: (page: React.ReactElement, router: NextRouter) => ReactNode; }; @@ -61,45 +62,66 @@ const CustomI18nextProvider = (props: AppPropsWithChildren) => { return ; }; +const enum ThemeSupport { + // e.g. Login Page + None = "none", + // Entire App except Booking Pages + App = "systemOnly", + // Booking Pages(including Routing Forms) + Booking = "userConfigured", +} + const CalcomThemeProvider = ( props: PropsWithChildren< - WithNonceProps & { isThemeSupported?: boolean | ((arg: { router: NextRouter }) => boolean) } + WithNonceProps & { + isBookingPage?: boolean | ((arg: { router: NextRouter }) => boolean); + isThemeSupported?: boolean; + } > ) => { // We now support the inverse of how we handled it in the past. Setting this to false will disable theme. // undefined or true means we use system theme const router = useRouter(); - const isThemeSupported = (() => { - if (typeof props.isThemeSupported === "function") { - return props.isThemeSupported({ router: router }); + const isBookingPage = (() => { + if (typeof props.isBookingPage === "function") { + return props.isBookingPage({ router: router }); } - if (typeof props.isThemeSupported === "undefined") { - return true; - } - return props.isThemeSupported; + + return props.isBookingPage; })(); - const forcedTheme = !isThemeSupported ? "light" : undefined; + const themeSupport = isBookingPage + ? ThemeSupport.Booking + : // if isThemeSupported is explicitly false, we don't use theme there + props.isThemeSupported === false + ? ThemeSupport.None + : ThemeSupport.App; + + const forcedTheme = themeSupport === ThemeSupport.None ? "light" : undefined; // Use namespace of embed to ensure same namespaced embed are displayed with same theme. This allows different embeds on the same website to be themed differently // One such example is our Embeds Demo and Testing page at http://localhost:3100 // Having `getEmbedNamespace` defined on window before react initializes the app, ensures that embedNamespace is available on the first mount and can be used as part of storageKey const embedNamespace = typeof window !== "undefined" ? window.getEmbedNamespace() : null; const isEmbedMode = typeof embedNamespace === "string"; - // If embedNamespace is not defined, we use the default storageKey -> The default storage key changs based on if we force light mode or not - // This is done to ensure that the default theme is light when we force light mode and as soon as you navigate to a page that is dark we dont need a hard refresh to change + const storageKey = isEmbedMode ? `embed-theme-${embedNamespace}` - : !isThemeSupported - ? "cal-light" - : "theme"; + : themeSupport === ThemeSupport.App + ? "app-theme" + : themeSupport === ThemeSupport.Booking + ? "booking-theme" + : undefined; return ( {/* Embed Mode can be detected reliably only on client side here as there can be static generated pages as well which can't determine if it's embed mode at backend */} {/* color-scheme makes background:transparent not work in iframe which is required by embed. */} @@ -134,7 +156,8 @@ const AppProviders = (props: AppPropsWithChildren) => { + isThemeSupported={props.Component.isThemeSupported} + isBookingPage={props.Component.isBookingPage}> {props.children} diff --git a/apps/web/pages/[user].tsx b/apps/web/pages/[user].tsx index a51f21cd87..30f9605535 100644 --- a/apps/web/pages/[user].tsx +++ b/apps/web/pages/[user].tsx @@ -195,6 +195,8 @@ export default function User(props: inferSSRProps & E ); } +User.isBookingPage = true; + const getEventTypesWithHiddenFromDB = async (userId: number) => { return ( await prisma.eventType.findMany({ diff --git a/apps/web/pages/[user]/[type].tsx b/apps/web/pages/[user]/[type].tsx index 664d7b0166..8e5759b945 100644 --- a/apps/web/pages/[user]/[type].tsx +++ b/apps/web/pages/[user]/[type].tsx @@ -54,6 +54,8 @@ export default function Type(props: AvailabilityPageProps) { ); } +Type.isBookingPage = true; + const paramsSchema = z.object({ type: z.string(), user: z.string() }); async function getUserPageProps(context: GetStaticPropsContext) { // load server side dependencies diff --git a/apps/web/pages/[user]/book.tsx b/apps/web/pages/[user]/book.tsx index 244f4bc42d..4e8e246ed8 100644 --- a/apps/web/pages/[user]/book.tsx +++ b/apps/web/pages/[user]/book.tsx @@ -69,6 +69,8 @@ export default function Book(props: BookPageProps) { ); } +Book.isBookingPage = true; + const querySchema = z.object({ bookingUid: z.string().optional(), count: z.coerce.number().optional(), diff --git a/apps/web/pages/apps/[slug]/[...pages].tsx b/apps/web/pages/apps/[slug]/[...pages].tsx index a4c502ff1c..35621eb55f 100644 --- a/apps/web/pages/apps/[slug]/[...pages].tsx +++ b/apps/web/pages/apps/[slug]/[...pages].tsx @@ -15,7 +15,7 @@ type AppPageType = { getServerSideProps: AppGetServerSideProps; // A component than can accept any properties // eslint-disable-next-line @typescript-eslint/no-explicit-any - default: ((props: any) => JSX.Element) & Pick; + default: ((props: any) => JSX.Element) & Pick; }; type Found = { @@ -70,17 +70,17 @@ const AppPage: AppPageType["default"] = function AppPage(props) { return ; }; -AppPage.isThemeSupported = ({ router }) => { +AppPage.isBookingPage = ({ router }) => { const route = getRoute(router.query.slug as string, router.query.pages as string[]); if (route.notFound) { return false; } - const isThemeSupported = route.Component.isThemeSupported; - if (typeof isThemeSupported === "function") { - return isThemeSupported({ router }); + const isBookingPage = route.Component.isBookingPage; + if (typeof isBookingPage === "function") { + return isBookingPage({ router }); } - return !!isThemeSupported; + return !!isBookingPage; }; AppPage.getLayout = (page, router) => { diff --git a/apps/web/pages/booking/[uid].tsx b/apps/web/pages/booking/[uid].tsx index 099fad6923..7dcfba24a7 100644 --- a/apps/web/pages/booking/[uid].tsx +++ b/apps/web/pages/booking/[uid].tsx @@ -268,7 +268,9 @@ export default function Success(props: SuccessProps) { return t("emailed_you_and_attendees" + titleSuffix); } - useTheme(isSuccessBookingPage ? props.profile.theme : "light"); + // This is a weird case where the same route can be opened in booking flow as a success page or as a booking detail page from the app + // As Booking Page it has to support configured theme, but as booking detail page it should not do any change. Let Shell.tsx handle it. + useTheme(isSuccessBookingPage ? props.profile.theme : undefined); useBrandColors({ brandColor: props.profile.brandColor, darkBrandColor: props.profile.darkBrandColor, @@ -705,6 +707,8 @@ export default function Success(props: SuccessProps) { ); } +Success.isBookingPage = true; + type RecurringBookingsProps = { eventType: SuccessProps["eventType"]; recurringBookings: SuccessProps["recurringBookings"]; diff --git a/apps/web/pages/d/[link]/[slug].tsx b/apps/web/pages/d/[link]/[slug].tsx index 2e1bdafb46..40328d04e4 100644 --- a/apps/web/pages/d/[link]/[slug].tsx +++ b/apps/web/pages/d/[link]/[slug].tsx @@ -24,6 +24,8 @@ export default function Type(props: DynamicAvailabilityPageProps) { return ; } +Type.isBookingPage = true; + const querySchema = z.object({ link: z.string().optional().default(""), slug: z.string().optional().default(""), diff --git a/apps/web/pages/d/[link]/book.tsx b/apps/web/pages/d/[link]/book.tsx index a8a3684846..e1949802af 100644 --- a/apps/web/pages/d/[link]/book.tsx +++ b/apps/web/pages/d/[link]/book.tsx @@ -19,6 +19,8 @@ export default function Book(props: HashLinkPageProps) { return ; } +Book.isBookingPage = true; + export async function getServerSideProps(context: GetServerSidePropsContext) { const ssr = await ssrInit(context); const link = asStringOrThrow(context.query.link as string); diff --git a/apps/web/pages/team/[slug].tsx b/apps/web/pages/team/[slug].tsx index dd9043634f..c90d2db3eb 100644 --- a/apps/web/pages/team/[slug].tsx +++ b/apps/web/pages/team/[slug].tsx @@ -203,4 +203,6 @@ export const getServerSideProps = async (context: GetServerSidePropsContext) => } as const; }; +TeamPage.isBookingPage = true; + export default TeamPage; diff --git a/apps/web/pages/team/[slug]/[type].tsx b/apps/web/pages/team/[slug]/[type].tsx index 69eaadfeaa..63a5c9301f 100644 --- a/apps/web/pages/team/[slug]/[type].tsx +++ b/apps/web/pages/team/[slug]/[type].tsx @@ -24,6 +24,8 @@ export default function TeamType(props: AvailabilityTeamPageProps) { return ; } +TeamType.isBookingPage = true; + export const getServerSideProps = async (context: GetServerSidePropsContext) => { const slugParam = asStringOrNull(context.query.slug); const typeParam = asStringOrNull(context.query.type); diff --git a/apps/web/pages/team/[slug]/book.tsx b/apps/web/pages/team/[slug]/book.tsx index 5fb2f7d66c..0290e67cb3 100644 --- a/apps/web/pages/team/[slug]/book.tsx +++ b/apps/web/pages/team/[slug]/book.tsx @@ -23,6 +23,7 @@ export type TeamBookingPageProps = inferSSRProps; export default function TeamBookingPage(props: TeamBookingPageProps) { return ; } +TeamBookingPage.isBookingPage = true; const querySchema = z.object({ rescheduleUid: z.string().optional(), diff --git a/packages/app-store/routing-forms/pages/form-edit/[...appPages].tsx b/packages/app-store/routing-forms/pages/form-edit/[...appPages].tsx index 99415bcfbc..6df0cdbfb9 100644 --- a/packages/app-store/routing-forms/pages/form-edit/[...appPages].tsx +++ b/packages/app-store/routing-forms/pages/form-edit/[...appPages].tsx @@ -332,7 +332,6 @@ export default function FormEditPage({ /> ); } -FormEditPage.isThemeSupported = true; FormEditPage.getLayout = (page: React.ReactElement) => { return ( diff --git a/packages/app-store/routing-forms/pages/forms/[...appPages].tsx b/packages/app-store/routing-forms/pages/forms/[...appPages].tsx index 12f048088a..cef722a0ac 100644 --- a/packages/app-store/routing-forms/pages/forms/[...appPages].tsx +++ b/packages/app-store/routing-forms/pages/forms/[...appPages].tsx @@ -252,8 +252,6 @@ export default function RoutingForms({ ); } -RoutingForms.isThemeSupported = true; - RoutingForms.getLayout = (page: React.ReactElement) => { return {page}; }; diff --git a/packages/app-store/routing-forms/pages/reporting/[...appPages].tsx b/packages/app-store/routing-forms/pages/reporting/[...appPages].tsx index 714e0a2053..96174cac1e 100644 --- a/packages/app-store/routing-forms/pages/reporting/[...appPages].tsx +++ b/packages/app-store/routing-forms/pages/reporting/[...appPages].tsx @@ -197,8 +197,6 @@ export default function ReporterWrapper({ ); } -ReporterWrapper.isThemeSupported = true; - ReporterWrapper.getLayout = (page: React.ReactElement) => { return ( diff --git a/packages/app-store/routing-forms/pages/route-builder/[...appPages].tsx b/packages/app-store/routing-forms/pages/route-builder/[...appPages].tsx index ae3b0fa40e..2539142625 100644 --- a/packages/app-store/routing-forms/pages/route-builder/[...appPages].tsx +++ b/packages/app-store/routing-forms/pages/route-builder/[...appPages].tsx @@ -523,7 +523,6 @@ export default function RouteBuilder({ /> ); } -RouteBuilder.isThemeSupported = true; RouteBuilder.getLayout = (page: React.ReactElement) => { return ( diff --git a/packages/app-store/routing-forms/pages/router/[...appPages].tsx b/packages/app-store/routing-forms/pages/router/[...appPages].tsx index 6ca33edcc1..b9cc2c5516 100644 --- a/packages/app-store/routing-forms/pages/router/[...appPages].tsx +++ b/packages/app-store/routing-forms/pages/router/[...appPages].tsx @@ -26,8 +26,6 @@ export default function Router({ form, message }: inferSSRProps; } -RoutingLink.isThemeSupported = true; +RoutingLink.isBookingPage = true; export const getServerSideProps = async function getServerSideProps( context: AppGetServerSidePropsContext, diff --git a/packages/features/shell/Shell.tsx b/packages/features/shell/Shell.tsx index 8c1ba4d387..961138f222 100644 --- a/packages/features/shell/Shell.tsx +++ b/packages/features/shell/Shell.tsx @@ -22,7 +22,6 @@ import classNames from "@calcom/lib/classNames"; import { APP_NAME, DESKTOP_APP_LINK, JOIN_SLACK, ROADMAP, WEBAPP_URL } from "@calcom/lib/constants"; import getBrandColours from "@calcom/lib/getBrandColours"; import { useLocale } from "@calcom/lib/hooks/useLocale"; -import useTheme from "@calcom/lib/hooks/useTheme"; import { isKeyInObject } from "@calcom/lib/isKeyInObject"; import { trpc } from "@calcom/trpc/react"; import useMeQuery from "@calcom/trpc/react/hooks/useMeQuery"; @@ -231,7 +230,8 @@ export default function Shell(props: LayoutProps) { // if a page is unauthed and isPublic is true, the redirect does not happen. useRedirectToLoginIfUnauthenticated(props.isPublic); useRedirectToOnboardingIfNeeded(); - useTheme(); + // System Theme is automatically supported using ThemeProvider. If we intend to use user theme throughout the app we need to uncomment this. + // useTheme(profile.theme); useBrandColors(); return !props.isPublic ? ( diff --git a/packages/lib/hooks/useTheme.tsx b/packages/lib/hooks/useTheme.tsx index 8a6c4555b2..fc7f66d808 100644 --- a/packages/lib/hooks/useTheme.tsx +++ b/packages/lib/hooks/useTheme.tsx @@ -4,23 +4,28 @@ import { useEffect } from "react"; import { useEmbedTheme } from "@calcom/embed-core/embed-iframe"; import type { Maybe } from "@calcom/trpc/server"; -// makes sure the ui doesn't flash -export default function useTheme(theme?: Maybe) { +/** + * It should be called once per route and only if you want to use app configured theme. System only theme works automatically by using ThemeProvider + * Calling it without a theme will just returns the current theme. + * It handles embed configured theme as well. + */ +export default function useTheme(themeToSet?: Maybe) { const { resolvedTheme, setTheme, forcedTheme, theme: activeTheme } = useNextTheme(); const embedTheme = useEmbedTheme(); - // Embed UI configuration takes more precedence over App Configuration - const currentTheme = embedTheme || theme || "system"; useEffect(() => { - if (currentTheme !== activeTheme && typeof currentTheme === "string") { - setTheme(currentTheme); - } + // If themeToSet is not provided the purpose is to just return the current the current values + if (themeToSet === undefined) return; + + // Embed theme takes precedence over theme configured in app. This allows embeds to be themed differently + const finalThemeToSet = embedTheme || themeToSet || "system"; + + if (!finalThemeToSet || finalThemeToSet === activeTheme) return; + + console.log("Setting theme", { resolvedTheme, finalThemeToSet, activeTheme, forcedTheme }); + setTheme(finalThemeToSet); // eslint-disable-next-line react-hooks/exhaustive-deps -- we do not want activeTheme to re-render this effect - }, [currentTheme, setTheme]); - - useEffect(() => { - if (forcedTheme) setTheme(forcedTheme); - }, [forcedTheme, setTheme]); + }, [themeToSet, setTheme]); return { resolvedTheme,