fix: make theme logic safe and add better e2e tests (#16959)

* fix: improve theme logic

* remove null from ThemeLabelProps value

* add better e2e tests

apps/web/modules/settings/my-account/appearance-view.tsx

@@ -192,15 +192,21 @@ const AppearanceView = ({
       </div>
       <Form
         form={userAppThemeFormMethods}
-        handleSubmit={(values) => {
+        handleSubmit={({ appTheme }) => {
+          if (appTheme === "light" || appTheme === "dark") {
+            mutation.mutate({
+              appTheme,
+            });
+            return;
+          }
           mutation.mutate({
-            appTheme: values.appTheme === "" ? null : values.appTheme,
+            appTheme: null,
           });
         }}>
         <div className="border-subtle flex flex-col justify-between border-x px-6 py-8 sm:flex-row">
           <ThemeLabel
             variant="system"
-            value={undefined}
+            value="system"
             label={t("theme_system")}
             defaultChecked={user.appTheme === null}
             register={userAppThemeFormMethods.register}
@@ -244,17 +250,21 @@ const AppearanceView = ({
           </div>
           <Form
             form={userThemeFormMethods}
-            handleSubmit={(values) => {
+            handleSubmit={({ theme }) => {
+              if (theme === "light" || theme === "dark") {
+                mutation.mutate({
+                  theme,
+                });
+                return;
+              }
               mutation.mutate({
-                // Radio values don't support null as values, therefore we convert an empty string
-                // back to null here.
-                theme: values.theme === "" ? null : values.theme,
+                theme: null,
               });
             }}>
             <div className="border-subtle flex flex-col justify-between border-x px-6 py-8 sm:flex-row">
               <ThemeLabel
                 variant="system"
-                value={undefined}
+                value="system"
                 label={t("theme_system")}
                 defaultChecked={user.theme === null}
                 register={userThemeFormMethods.register}

apps/web/playwright/change-theme.e2e.ts

@@ -1,10 +1,70 @@
 import { expect } from "@playwright/test";
-import { testBothFutureAndLegacyRoutes } from "playwright/lib/future-legacy-routes";
 
 import { test } from "./lib/fixtures";
 
-testBothFutureAndLegacyRoutes.describe("Change Theme Test", () => {
-  test("change theme to dark", async ({ page, users }) => {
+test.describe("Change App Theme Test", () => {
+  test("change app theme to dark", async ({ page, users }) => {
+    const pro = await users.create();
+    await pro.apiLogin();
+    await page.goto("/settings/my-account/appearance");
+    await page.click('[data-testid="appTheme-dark"]');
+    await page.click('[data-testid="update-app-theme-btn"]');
+
+    const toast = await page.waitForSelector('[data-testid="toast-success"]');
+    expect(toast).toBeTruthy();
+
+    const darkModeClass = await page.getAttribute("html", "class");
+    expect(darkModeClass).toContain("dark");
+
+    const themeValue = await page.evaluate(() => localStorage.getItem("app-theme"));
+    expect(themeValue).toBe("dark");
+  });
+
+  test("change app theme to light", async ({ page, users }) => {
+    const pro = await users.create();
+    await pro.apiLogin();
+    await page.goto("/settings/my-account/appearance");
+    await page.click('[data-testid="appTheme-light"]');
+    await page.click('[data-testid="update-app-theme-btn"]');
+
+    const toast = await page.waitForSelector('[data-testid="toast-success"]');
+    expect(toast).toBeTruthy();
+
+    const darkModeClass = await page.getAttribute("html", "class");
+    expect(darkModeClass).toContain("light");
+
+    const themeValue = await page.evaluate(() => localStorage.getItem("app-theme"));
+    expect(themeValue).toBe("light");
+  });
+
+  test("change app theme to system", async ({ page, users }) => {
+    const pro = await users.create();
+    await pro.apiLogin();
+
+    await page.goto("/settings/my-account/appearance");
+    await page.click('[data-testid="appTheme-light"]');
+    await page.click('[data-testid="update-app-theme-btn"]');
+    const toast1 = await page.waitForSelector('[data-testid="toast-success"]');
+    expect(toast1).toBeTruthy();
+
+    await page.click('[data-testid="appTheme-system"]');
+    await page.click('[data-testid="update-app-theme-btn"]');
+    const toast2 = await page.waitForSelector('[data-testid="toast-success"]');
+    expect(toast2).toBeTruthy();
+
+    await page.waitForTimeout(3000);
+    const themeValue = await page.evaluate(() => localStorage.getItem("app-theme"));
+    expect(themeValue).toBe("system");
+
+    const systemTheme = await page.evaluate(() => {
+      return window.matchMedia("(prefers-color-scheme: dark)").matches ? "dark" : "light";
+    });
+    expect(await page.getAttribute("html", "class")).toContain(systemTheme);
+  });
+});
+
+test.describe("Change Booking Page Theme Test", () => {
+  test("change booking page theme to dark", async ({ page, users }) => {
     const pro = await users.create();
     await pro.apiLogin();
 
@@ -19,11 +79,11 @@ testBothFutureAndLegacyRoutes.describe("Change Theme Test", () => {
     expect(toast).toBeTruthy();
     //Go to the profile page and check if the theme is dark
     await page.goto(`/${pro.username}`);
-    const darkModeClass = await page.getAttribute("html", "class");
-    expect(darkModeClass).toContain("dark");
+    const htmlClass = await page.getAttribute("html", "class");
+    expect(htmlClass).toContain("dark");
   });
 
-  test("change theme to light", async ({ page, users }) => {
+  test("change booking page theme to light", async ({ page, users }) => {
     const pro = await users.create();
     await pro.apiLogin();
 
@@ -38,7 +98,31 @@ testBothFutureAndLegacyRoutes.describe("Change Theme Test", () => {
     expect(toast).toBeTruthy();
     //Go to the profile page and check if the theme is light
     await page.goto(`/${pro.username}`);
-    const darkModeClass = await page.getAttribute("html", "class");
-    expect(darkModeClass).toContain("light");
+    const htmlClass = await page.getAttribute("html", "class");
+    expect(htmlClass).toContain("light");
+  });
+
+  test("change booking page theme to system", async ({ page, users }) => {
+    const pro = await users.create();
+    await pro.apiLogin();
+
+    await page.goto("/settings/my-account/appearance");
+
+    await page.click('[data-testid="theme-light"]');
+    await page.click('[data-testid="update-theme-btn"]');
+    const toast1 = await page.waitForSelector('[data-testid="toast-success"]');
+    expect(toast1).toBeTruthy();
+
+    await page.click('[data-testid="theme-system"]');
+    await page.click('[data-testid="update-theme-btn"]');
+    const toast2 = await page.waitForSelector('[data-testid="toast-success"]');
+    expect(toast2).toBeTruthy();
+    await page.goto(`/${pro.username}`);
+
+    const systemTheme = await page.evaluate(() => {
+      return window.matchMedia("(prefers-color-scheme: dark)").matches ? "dark" : "light";
+    });
+
+    expect(await page.getAttribute("html", "class")).toContain(systemTheme);
   });
 });

packages/features/ee/organizations/pages/settings/appearance.tsx

@@ -78,9 +78,15 @@ const OrgAppearanceView = ({
     <div>
       <Form
         form={themeForm}
-        handleSubmit={(value) => {
+        handleSubmit={({ theme }) => {
+          if (theme === "light" || theme === "dark") {
+            mutation.mutate({
+              theme,
+            });
+            return;
+          }
           mutation.mutate({
-            theme: value.theme === "" ? null : value.theme,
+            theme: null,
           });
         }}>
         <div className="border-subtle mt-6 flex items-center rounded-t-xl border p-6 text-sm">
@@ -92,7 +98,7 @@ const OrgAppearanceView = ({
         <div className="border-subtle flex flex-col justify-between border-x px-6 py-8 sm:flex-row">
           <ThemeLabel
             variant="system"
-            value={undefined}
+            value="system"
             label={t("theme_system")}
             defaultChecked={currentOrg.theme === null}
             register={themeForm.register}

packages/features/ee/teams/pages/team-appearance-view.tsx

@@ -90,10 +90,10 @@ const ProfileView = ({ team, isAppDir }: ProfileViewProps) => {
         <>
           <Form
             form={themeForm}
-            handleSubmit={(values) => {
+            handleSubmit={({ theme }) => {
               mutation.mutate({
                 id: team.id,
-                theme: values.theme === "" ? null : values.theme,
+                theme: theme === "light" || theme === "dark" ? theme : null,
               });
             }}>
             <div className="border-subtle mt-6 flex items-center rounded-t-xl border p-6 text-sm">
@@ -105,7 +105,7 @@ const ProfileView = ({ team, isAppDir }: ProfileViewProps) => {
             <div className="border-subtle flex flex-col justify-between border-x px-6 py-8 sm:flex-row">
               <ThemeLabel
                 variant="system"
-                value={null}
+                value="system"
                 label={t("theme_system")}
                 defaultChecked={team.theme === null}
                 register={themeForm.register}

packages/features/settings/ThemeLabel.tsx

@@ -1,6 +1,6 @@
 interface ThemeLabelProps {
   variant: "light" | "dark" | "system";
-  value?: "light" | "dark" | null;
+  value: "light" | "dark" | "system";
   label: string;
   defaultChecked?: boolean;
   register: any;

packages/ui/styles/useCalcomTheme.tsx

@@ -8,6 +8,10 @@ type CssVariables = Record<string, string>;
 const useCalcomTheme = (theme: Record<string, CssVariables>) => {
   useEffect(() => {
     Object.entries(theme).forEach(([key, value]) => {
+      if (!value) {
+        // should not be reached
+        return;
+      }
       if (key === "root") {
         const root = document.documentElement;
         Object.entries(value).forEach(([key, value]) => {