refactor(theme): remove duplicate color definitions (@fehmer)

.github/pull_request_template.md

@@ -17,14 +17,14 @@
   - Make sure to follow the [themes documentation](https://github.com/monkeytypegame/monkeytype/blob/master/docs/THEMES.md)
   - [ ] Add theme to `packages/schemas/src/themes.ts`
   - [ ] Add theme to `frontend/src/ts/constants/themes.ts`
-  - [ ] Add theme css file to `frontend/static/themes`
+  - [ ] (optional) Add theme css file to `frontend/static/themes`
   - [ ] Add some screenshots of the theme, especially with different test settings (colorful, flip colors) to your pull request
 - [ ] Adding a layout?
   - [ ] Make sure to follow the [layouts documentation](https://github.com/monkeytypegame/monkeytype/blob/master/docs/LAYOUTS.md)
   - [ ] Add layout to `packages/schemas/src/layouts.ts`
   - [ ] Add layout json file to `frontend/static/layouts`
 - [ ] Adding a font?
-  - Make sure to follow the [themes documentation](https://github.com/monkeytypegame/monkeytype/blob/master/docs/FONTS.md)
+  - Make sure to follow the [fonts documentation](https://github.com/monkeytypegame/monkeytype/blob/master/docs/FONTS.md)
   - [ ] Add font file to `frontend/static/webfonts`
   - [ ] Add font to `packages/schemas/src/fonts.ts`
   - [ ] Add font to `frontend/src/ts/constants/fonts.ts`

docs/THEMES.md

@@ -13,55 +13,68 @@ First you will have to make a personal copy of the Monkeytype repository, also k
 
 ## Creating Themes
 
-After you have forked the repository you can now add your theme. Create a CSS file in `./frontend/static/themes/`. Call it whatever you want but make sure that it is all lowercase and replace spaces with underscores. It should look something like this:
-`theme_name.css`.
-
-Then add this code to your file:
-
-```css
-:root {
-  --bg-color: #ffffff;
-  --main-color: #ffffff;
-  --caret-color: #ffffff;
-  --sub-color: #ffffff;
-  --sub-alt-color: #ffffff;
-  --text-color: #ffffff;
-  --error-color: #ffffff;
-  --error-extra-color: #ffffff;
-  --colorful-error-color: #ffffff;
-  --colorful-error-extra-color: #ffffff;
-}
-```
-
-Here is an image showing what all the properties correspond to:
-<img width="1552" alt="Screenshot showing the page elements controlled by each color property" src="https://user-images.githubusercontent.com/83455454/149196967-abb69795-0d38-466b-a867-5aaa46452976.png">
+Pick a name for your theme. It must be all lowercase, with spaces replaced by underscores.
 
-Change the corresponding hex codes to create your theme.
-Then, go to `./packages/schemas/src/themes.ts` and add your new theme name at the _end_ of the `ThemeNameSchema` enum. Make sure to end the line with a comma.
+Go to `./packages/schemas/src/themes.ts` and add your new theme name to the __end__ of the `ThemeNameSchema` enum. Make sure to end the line with a comma.
 
 ```typescript
 export const ThemeNameSchema = z.enum([
   "8008",
   "80s_after_dark",
-  ...
+  ... all existing theme names
   "your_theme_name",
+]);
+```
+
+Then, go to `./frontend/src/ts/constants/themes.ts` and add the following code to the __end__ of the `themes` object near to the very end of the file:
+
+```typescript
+export const themes: Record<ThemeName, Theme>  = {
+  ... all existing themes
+  your_theme_name: {
+    bg: "#ffffff",
+    caret: "#ffffff",
+    main: "#ffffff",
+    sub: "#ffffff",
+    subAlt: "#ffffff",
+    text: "#ffffff",
+    error: "#ffffff",
+    errorExtra: "#ffffff",
+    colorfulError: "#ffffff",
+    colorfulErrorExtra: "#ffffff",
+  },
+}
 ```
 
-Then, go to `./frontend/src/ts/constants/themes.ts` and add the following code to the _end_ of the `themes` object near to the very end of the file:
+Here is an image showing what all the properties correspond to:
+<img width="1552" alt="Screenshot showing the page elements controlled by each color property" src="https://user-images.githubusercontent.com/83455454/149196967-abb69795-0d38-466b-a867-5aaa46452976.png">
+
+If you don't want to add any custom styling you can skip the next section.
+
+
+#### Adding custom CSS (optional)
+
+Create a CSS file in `./frontend/static/themes/` matching the name you picked earlier. Update the theme configuration in `./frontend/src/ts/constants/themes.ts`  and add `hasCss: true` like this:
 
 ```typescript
-export const themes: Record<ThemeName, Omit<Theme, "name">> = {
-  ...
+export const themes: Record<ThemeName, Theme>  = {
+  ... all existing themes
   your_theme_name: {
-    bgColor: "#ffffff",
-    mainColor: "#ffffff",
-    subColor: "#ffffff",
-    textColor: "#ffffff",
+    bg: "#ffffff",
+    caret: "#ffffff",
+    main: "#ffffff",
+    sub: "#ffffff",
+    subAlt: "#ffffff",
+    text: "#ffffff",
+    error: "#ffffff",
+    errorExtra: "#ffffff",
+    colorfulError: "#ffffff",
+    colorfulErrorExtra: "#ffffff",
+    hasCss: true,
   },
 }
 ```
 
-Make sure the name you put matches the name of the file you created (without the `.css` file extension). Add the text color and background color of your theme to their respective fields.
 
 ### Committing Themes
 

frontend/__tests__/components/Layout/Theme.spec.tsx

@@ -0,0 +1,132 @@
+import { render, fireEvent } from "@solidjs/testing-library";
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { Theme } from "../../../src/ts/components/layout/Theme";
+import * as ThemeSignal from "../../../src/ts/signals/theme";
+import { createSignal } from "solid-js";
+import { ThemeWithName } from "../../../src/ts/constants/themes";
+import * as Loader from "../../../src/ts/elements/loader";
+import * as Notifications from "../../../src/ts/elements/notifications";
+
+vi.mock("../../../src/ts/constants/themes", () => ({
+  themes: {
+    dark: { hasCss: true },
+    light: {},
+  },
+}));
+
+vi.mock("./FavIcon", () => ({
+  FavIcon: () => <div id="favicon" />,
+}));
+
+describe("Theme component", () => {
+  const [themeSignal, setThemeSignal] = createSignal<ThemeWithName>({} as any);
+  const themeSignalMock = vi.spyOn(ThemeSignal, "getTheme");
+  const loaderShowMock = vi.spyOn(Loader, "show");
+  const loaderHideMock = vi.spyOn(Loader, "hide");
+  const notificationAddMock = vi.spyOn(Notifications, "add");
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+    loaderShowMock.mockClear();
+    loaderHideMock.mockClear();
+    notificationAddMock.mockClear();
+    themeSignalMock.mockImplementation(() => themeSignal());
+    setThemeSignal({
+      name: "dark",
+      bg: "#000",
+      main: "#fff",
+      caret: "#fff",
+      sub: "#aaa",
+      subAlt: "#888",
+      text: "#fff",
+      error: "#f00",
+      errorExtra: "#c00",
+      colorfulError: "#f55",
+      colorfulErrorExtra: "#c55",
+    });
+  });
+
+  it("injects CSS variables based on theme", () => {
+    const { style } = renderComponent();
+
+    expect(style.innerHTML).toEqual(`
+:root {
+    --bg-color: #000;
+    --main-color: #fff;
+    --caret-color: #fff;
+    --sub-color: #aaa;
+    --sub-alt-color: #888;
+    --text-color: #fff;
+    --error-color: #f00;
+    --error-extra-color: #c00;
+    --colorful-error-color: #f55;
+    --colorful-error-extra-color: #c55;
+}`);
+  });
+
+  it("updates CSS variables based on signal", () => {
+    setThemeSignal({ name: "light", bg: "#f00" } as any);
+    const { style } = renderComponent();
+
+    expect(style.innerHTML).toContain("--bg-color: #f00;");
+  });
+
+  it("loads CSS file and shows loader when theme has CSS", () => {
+    const { css } = renderComponent();
+
+    expect(css.getAttribute("href")).toBe("/themes/dark.css");
+    expect(loaderShowMock).toHaveBeenCalledOnce();
+    fireEvent.load(css);
+    expect(loaderHideMock).toHaveBeenCalledOnce();
+  });
+
+  it("removes CSS when theme has no CSS", async () => {
+    themeSignalMock.mockImplementation(() => ({ name: "light" }) as any);
+    const { css } = renderComponent();
+    expect(css.getAttribute("href")).toBe("");
+  });
+
+  it("removes CSS when theme is custom", async () => {
+    themeSignalMock.mockImplementation(() => ({ name: "custom" }) as any);
+    const { css } = renderComponent();
+    expect(css.getAttribute("href")).toBe("");
+  });
+
+  it("handles CSS load error", () => {
+    const { css } = renderComponent();
+    expect(loaderShowMock).toHaveBeenCalledOnce();
+    fireEvent.error(css);
+    expect(loaderHideMock).toHaveBeenCalledOnce();
+    expect(notificationAddMock).toHaveBeenCalledWith("Failed to load theme", 0);
+  });
+
+  it("renders favicon", () => {
+    const { favIcon } = renderComponent();
+
+    expect(favIcon).toBeInTheDocument();
+    expect(favIcon).toBeEmptyDOMElement(); //mocked
+  });
+
+  function renderComponent(): {
+    style: HTMLStyleElement;
+    css: HTMLLinkElement;
+    metaThemeColor: HTMLMetaElement;
+    favIcon: HTMLElement;
+  } {
+    render(() => <Theme />);
+
+    //make sure content is rendered to the head, not the body
+    const head = document.head;
+
+    return {
+      // oxlint-disable-next-line typescript/no-non-null-assertion
+      style: head.querySelector("style#theme")!,
+      // oxlint-disable-next-line typescript/no-non-null-assertion
+      css: head.querySelector("link#currentTheme")!,
+      // oxlint-disable-next-line typescript/no-non-null-assertion
+      metaThemeColor: head.querySelector("meta#metaThemeColor")!,
+      // oxlint-disable-next-line typescript/no-non-null-assertion
+      favIcon: head.querySelector("#favicon")!,
+    };
+  }
+});

frontend/package.json

@@ -29,6 +29,7 @@
     "@monkeytype/util": "workspace:*",
     "@sentry/browser": "9.14.0",
     "@sentry/vite-plugin": "3.3.1",
+    "@solidjs/meta": "0.29.4",
     "@ts-rest/core": "3.52.1",
     "animejs": "4.2.2",
     "balloon-css": "1.2.0",

frontend/scripts/check-assets.ts

@@ -17,7 +17,7 @@ import { Layout, ThemeName } from "@monkeytype/schemas/configs";
 import { LayoutsList } from "../src/ts/constants/layouts";
 import { KnownFontName } from "@monkeytype/schemas/fonts";
 import { Fonts } from "../src/ts/constants/fonts";
-import { ThemesList } from "../src/ts/constants/themes";
+import { themes, ThemeSchema, ThemesList } from "../src/ts/constants/themes";
 import { z } from "zod";
 import { ChallengeSchema, Challenge } from "@monkeytype/schemas/challenges";
 import { LayoutObject, LayoutObjectSchema } from "@monkeytype/schemas/layouts";
@@ -381,20 +381,32 @@ async function validateThemes(): Promise<void> {
   //no missing files
   const themeFiles = fs.readdirSync("./static/themes");
 
-  //missing theme files
-  ThemesList.filter((it) => !themeFiles.includes(it.name + ".css")).forEach(
-    (it) =>
-      problems.add(
-        it.name,
-        `missing file frontend/static/themes/${it.name}.css`,
-      ),
+  //missing or additional theme files (mismatch in hasCss)
+  ThemesList.filter(
+    (it) => themeFiles.includes(it.name + ".css") !== (it.hasCss ?? false),
+  ).forEach((it) =>
+    problems.add(
+      it.name,
+      `${it.hasCss ? "missing" : "additional"} file frontend/static/themes/${it.name}.css`,
+    ),
   );
 
   //additional theme files
   themeFiles
     .filter((it) => !ThemesList.some((theme) => theme.name + ".css" === it))
     .forEach((it) => problems.add("_additional", it));
 
+  //validate theme colors are valid hex colors, not covered by typescipt
+  const themeNameSchema = z.string().regex(/^[a-z0-9_]+$/, {
+    message:
+      "theme name can only contain lowercase letters, digits and underscore",
+  });
+  for (const name of Object.keys(themes)) {
+    const theme = themes[name as ThemeName];
+    problems.addValidation(name as ThemeName, ThemeSchema.safeParse(theme));
+    problems.addValidation(name as ThemeName, themeNameSchema.safeParse(name));
+  }
+
   console.log(problems.toString());
 
   if (problems.hasError()) {

frontend/src/html/head.html

@@ -26,8 +26,7 @@
   <meta charset="UTF-8" />
   <meta name="viewport" content="width=device-width, initial-scale=1.0" />
   <title>Monkeytype | A minimalistic, customizable typing test</title>
-  <!-- default colors in case theme file fails to load -->
-  <style>
+  <style id="themeFallback">
     :root {
       --bg-color: #323437;
       --main-color: #e2b714;
@@ -47,12 +46,6 @@
     sizes="32x32"
     href="/images/favicon/favicon.ico"
   />
-  <link
-    id="favicon"
-    rel="shortcut icon"
-    type="image/svg+xml"
-    href="/images/favicon/favicon.svg"
-  />
   <link
     rel="apple-touch-icon"
     sizes="180x180"
@@ -68,7 +61,6 @@
     name="msapplication-config"
     content="/images/favicon/browserconfig.xml"
   />
-  <meta id="metaThemeColor" name="theme-color" content="#e2b714" />
   <meta
     name="name"
     content="Monkeytype | A minimalistic, customizable typing test"