Skip to content

Commit

Permalink
Add comments
Browse files Browse the repository at this point in the history
  • Loading branch information
hariombalhara committed May 16, 2023
1 parent 414777d commit c09828b
Show file tree
Hide file tree
Showing 13 changed files with 57 additions and 32 deletions.
39 changes: 32 additions & 7 deletions apps/web/lib/app-providers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const I18nextAdapter = appWithTranslation<NextJsAppProps<SSRConfig> & { children

// Workaround for https://github.com/vercel/next.js/issues/8592
export type AppProps = Omit<
NextAppProps<WithNonceProps & { appearanceBasis?: string } & Record<string, unknown>>,
NextAppProps<WithNonceProps & { themeBasis?: string } & Record<string, unknown>>,
"Component"
> & {
Component: NextAppProps["Component"] & {
Expand Down Expand Up @@ -76,7 +76,7 @@ const enum ThemeSupport {
}

type CalcomThemeProps = PropsWithChildren<
Pick<AppProps["pageProps"], "nonce" | "appearanceBasis"> &
Pick<AppProps["pageProps"], "nonce" | "themeBasis"> &
Pick<AppProps["Component"], "isBookingPage" | "isThemeSupported">
>;
const CalcomThemeProvider = (props: CalcomThemeProps) => {
Expand Down Expand Up @@ -106,6 +106,31 @@ const CalcomThemeProvider = (props: CalcomThemeProps) => {
);
};

/**
* The most important job for this fn is to generate correct storageKey for theme persistenc.
* `storageKey` is important because that key is listened for changes(using [`storage`](https://developer.mozilla.org/en-US/docs/Web/API/Window/storage_event) event) and any pages opened will change it's theme based on that(as part of next-themes implementation).
* Choosing the right storageKey avoids theme flickering caused by another page using different theme
* So, we handle all the cases here namely,
* - Both Booking Pages, /free/30min and /pro/30min but configured with different themes but being operated together.
* - Embeds using different namespace. They can be completely themed different on the same page.
* - Embeds using the same namespace but showing different cal.com links with different themes
* - Embeds using the same namespace and showing same cal.com links with different themes(Different theme is possible for same cal.com link in case of embed because of theme config available in embed)
* - App has different theme then Booking Pages.
*
* All the above cases have one thing in common, which is the origin and thus localStorage is shared and thus `storageKey` is critical to avoid theme flickering.
*
* Some things to note:
* - There is a side effect of so many factors in `storageKey` that many localStorage keys will be created if a user goes through all these scenarios(e.g like booking a lot of different users)
* - Some might recommend disabling localStorage persistence but that doesn't give good UX as then we would default to light theme always for a few seconds before switching to dark theme(if that's the user's preference).
* - We can't disable [`storage`](https://developer.mozilla.org/en-US/docs/Web/API/Window/storage_event) event handling as well because changing theme in one tab won't change the theme without refresh in other tabs. That's again a bad UX
* - Theme flickering becomes infinitely ongoing in case of embeds because of the browser's delay in processing `storage` event within iframes. Consider two embeds simulatenously opened with pages A and B. Note the timeline and keep in mind that it happened
* because 'setItem(A)' and 'Receives storageEvent(A)' allowed executing setItem(B) in b/w because of the delay.
* - t1 -> setItem(A) & Fires storageEvent(A) - On Page A) - Current State(A)
* - t2 -> setItem(B) & Fires storageEvent(B) - On Page B) - Current State(B)
* - t3 -> Receives storageEvent(A) & thus setItem(A) & thus fires storageEvent(A) (On Page B) - Current State(A)
* - t4 -> Receives storageEvent(B) & thus setItem(B) & thus fires storageEvent(B) (On Page A) - Current State(B)
* - ... and so on ...
*/
function getThemeProviderProps({
props,
isEmbedMode,
Expand All @@ -132,15 +157,15 @@ function getThemeProviderProps({
: ThemeSupport.App;

const isBookingPageThemSupportRequired = themeSupport === ThemeSupport.Booking;
const appearanceBasis = props.appearanceBasis;
const themeBasis = props.themeBasis;

if ((isBookingPageThemSupportRequired || isEmbedMode) && !appearanceBasis) {
if ((isBookingPageThemSupportRequired || isEmbedMode) && !themeBasis) {
console.warn(
"`appearanceBasis` is required for booking page theme support. Not providing it will cause theme flicker."
"`themeBasis` is required for booking page theme support. Not providing it will cause theme flicker."
);
}

const appearanceIdSuffix = appearanceBasis ? ":" + appearanceBasis : "";
const appearanceIdSuffix = themeBasis ? ":" + themeBasis : "";
const forcedTheme = themeSupport === ThemeSupport.None ? "light" : undefined;
let embedExplicitlySetThemeSuffix = "";

Expand Down Expand Up @@ -192,7 +217,7 @@ const AppProviders = (props: AppPropsWithChildren) => {
<TooltipProvider>
{/* color-scheme makes background:transparent not work which is required by embed. We need to ensure next-theme adds color-scheme to `body` instead of `html`(https://github.com/pacocoursey/next-themes/blob/main/src/index.tsx#L74). Once that's done we can enable color-scheme support */}
<CalcomThemeProvider
appearanceBasis={props.pageProps.appearanceBasis}
themeBasis={props.pageProps.themeBasis}
nonce={props.pageProps.nonce}
isThemeSupported={props.Component.isThemeSupported}
isBookingPage={props.Component.isBookingPage}>
Expand Down
2 changes: 1 addition & 1 deletion apps/web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@
"next-collect": "^0.2.1",
"next-i18next": "^11.3.0",
"next-seo": "^4.26.0",
"next-themes": "0.2.0",
"next-themes": "^0.2.0",
"nodemailer": "^6.7.8",
"otplib": "^12.0.1",
"qrcode": "^1.5.1",
Expand Down
2 changes: 1 addition & 1 deletion apps/web/pages/[user].tsx
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ export const getServerSideProps = async (context: GetServerSidePropsContext) =>
safeBio,
profile,
// Dynamic group has no theme preference right now. It uses system theme.
appearanceBasis: isDynamicGroup ? null : user.username,
themeBasis: isDynamicGroup ? null : user.username,
user: {
emailMd5: crypto.createHash("md5").update(user.email).digest("hex"),
},
Expand Down
5 changes: 2 additions & 3 deletions apps/web/pages/[user]/[type].tsx
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ async function getUserPageProps(context: GetStaticPropsContext) {
image: `${WEBAPP_URL}/${user.username}/avatar.png`,
},
// Dynamic group has no theme preference right now. It uses system theme.
appearanceBasis: user.username,
themeBasis: user.username,
away: user?.away,
isDynamic: false,
trpcState: ssg.dehydrate(),
Expand Down Expand Up @@ -294,7 +294,6 @@ async function getDynamicGroupPageProps(context: GetStaticPropsContext) {

const profile = {
name: getGroupName(dynamicNames),
// Dynamic Group doesn't have theming preference support. It uses system theme always.
image: null,
slug: "" + length,
theme: null as string | null,
Expand All @@ -311,7 +310,7 @@ async function getDynamicGroupPageProps(context: GetStaticPropsContext) {
eventType: eventTypeObject,
profile,
// Dynamic group has no theme preference right now. It uses system theme.
appearanceBasis: null,
themeBasis: null,
isDynamic: true,
away: false,
trpcState: ssg.dehydrate(),
Expand Down
2 changes: 1 addition & 1 deletion apps/web/pages/[user]/book.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ export async function getServerSideProps(context: GetServerSidePropsContext) {
away: user.away,
profile,
// Dynamic group has no theme preference right now. It uses system theme.
appearanceBasis: isDynamicGroupBooking ? null : user.username,
themeBasis: isDynamicGroupBooking ? null : user.username,
eventType: eventTypeObject,
booking,
currentSlotBooking: currentSlotBooking,
Expand Down
2 changes: 1 addition & 1 deletion apps/web/pages/booking/[uid].tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1092,7 +1092,7 @@ export async function getServerSideProps(context: GetServerSidePropsContext) {

return {
props: {
appearanceBasis: eventType.team ? eventType.team.slug : eventType.users[0]?.username,
themeBasis: eventType.team ? eventType.team.slug : eventType.users[0]?.username,
hideBranding: eventType.team ? eventType.team.hideBranding : eventType.users[0].hideBranding,
profile,
eventType,
Expand Down
2 changes: 1 addition & 1 deletion apps/web/pages/d/[link]/[slug].tsx
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ export const getServerSideProps = async (context: GetServerSidePropsContext) =>
return {
props: {
away: user.away,
appearanceBasis: user.username,
themeBasis: user.username,
isDynamicGroup: false,
profile,
date,
Expand Down
2 changes: 1 addition & 1 deletion apps/web/pages/d/[link]/book.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export async function getServerSideProps(context: GetServerSidePropsContext) {
return {
props: {
profile,
appearanceBasis: user.username,
themeBasis: user.username,
eventType: eventTypeObject,
booking: null,
currentSlotBooking: null,
Expand Down
2 changes: 1 addition & 1 deletion apps/web/pages/team/[slug].tsx
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ export const getServerSideProps = async (context: GetServerSidePropsContext) =>
return {
props: {
team: { ...team, safeBio, members },
appearanceBasis: team.slug,
themeBasis: team.slug,
trpcState: ssr.dehydrate(),
markdownStrippedBio,
},
Expand Down
2 changes: 1 addition & 1 deletion apps/web/pages/team/[slug]/[type].tsx
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ export const getServerSideProps = async (context: GetServerSidePropsContext) =>
brandColor: team.brandColor,
darkBrandColor: team.darkBrandColor,
},
appearanceBasis: team.slug,
themeBasis: team.slug,
date: dateParam,
eventType: eventTypeObject,
workingHours,
Expand Down
2 changes: 1 addition & 1 deletion apps/web/pages/team/[slug]/book.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ export async function getServerSideProps(context: GetServerSidePropsContext) {
image: eventTypeObject.team?.logo || null,
eventName: null,
},
appearanceBasis: eventTypeObject.team?.slug,
themeBasis: eventTypeObject.team?.slug,
eventType: eventTypeObject,
recurringEventCount,
booking,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ export const getServerSideProps = async function getServerSideProps(
include: {
user: {
select: {
username: true,
theme: true,
brandColor: true,
darkBrandColor: true,
Expand All @@ -209,7 +210,7 @@ export const getServerSideProps = async function getServerSideProps(
return {
props: {
isEmbed,
appearanceBasis: form.user.username,
themeBasis: form.user.username,
profile: {
theme: form.user.theme,
brandColor: form.user.brandColor,
Expand Down
24 changes: 12 additions & 12 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5001,7 +5001,7 @@ __metadata:
next-collect: ^0.2.1
next-i18next: ^11.3.0
next-seo: ^4.26.0
next-themes: 0.2.0
next-themes: ^0.2.0
nodemailer: ^6.7.8
otplib: ^12.0.1
postcss: ^8.4.18
Expand Down Expand Up @@ -30599,17 +30599,6 @@ __metadata:
languageName: node
linkType: hard

"next-themes@npm:0.2.0":
version: 0.2.0
resolution: "next-themes@npm:0.2.0"
peerDependencies:
next: "*"
react: "*"
react-dom: "*"
checksum: cee02db16bee471856557db9572ffa041b1d77254798e345fad09e1bd8b878e937cdacc73bb73c598eb3b042f20fff0201bd73d2de42e2061fa818585415e857
languageName: node
linkType: hard

"next-themes@npm:^0.0.8":
version: 0.0.8
resolution: "next-themes@npm:0.0.8"
Expand All @@ -30621,6 +30610,17 @@ __metadata:
languageName: node
linkType: hard

"next-themes@npm:^0.2.0":
version: 0.2.0
resolution: "next-themes@npm:0.2.0"
peerDependencies:
next: "*"
react: "*"
react-dom: "*"
checksum: cee02db16bee471856557db9572ffa041b1d77254798e345fad09e1bd8b878e937cdacc73bb73c598eb3b042f20fff0201bd73d2de42e2061fa818585415e857
languageName: node
linkType: hard

"next-tick@npm:^1.1.0":
version: 1.1.0
resolution: "next-tick@npm:1.1.0"
Expand Down

0 comments on commit c09828b

Please sign in to comment.