fix(sdk): Fix unmapped theme colors (#46650)

* Refactor

* Fix wrong colors when passing empty SDK `colors`

* Clean up CSS custom properties

* Add missing color mapping to the SDK

* Add missing colors to the main app

* Only override additional dynamic colors when values are passed

* Fix unit tests

* Address review

Co-authored-by: Phoomparin Mano <poom@metabase.com>

---------

Co-authored-by: Phoomparin Mano <poom@metabase.com>

enterprise/frontend/src/embedding-sdk/components/private/SdkThemeProvider.tsx

@@ -8,7 +8,7 @@ import {
 } from "embedding-sdk/lib/theme";
 import { useSelector } from "metabase/lib/redux";
 import { getSettings } from "metabase/selectors/settings";
-import { getMetabaseCssVariables } from "metabase/styled-components/theme/css-variables";
+import { getMetabaseSdkCssVariables } from "metabase/styled-components/theme/css-variables";
 import { ThemeProvider, useMantineTheme } from "metabase/ui";
 import { getApplicationColors } from "metabase-enterprise/settings/selectors";
 
@@ -40,5 +40,5 @@ export const SdkThemeProvider = ({ theme, children }: Props) => {
 
 function GlobalStyles() {
   const theme = useMantineTheme();
-  return <Global styles={getMetabaseCssVariables(theme)} />;
+  return <Global styles={getMetabaseSdkCssVariables(theme)} />;
 }

enterprise/frontend/src/embedding-sdk/lib/theme/embedding-color-palette.ts

@@ -13,22 +13,39 @@ import { getEmbeddingChartColors } from "./get-embedding-chart-colors";
  */
 export type MappableSdkColor = Exclude<MetabaseColor, "charts">;
 
+type NEW_SEMANTIC_COLOR =
+  | "text-primary"
+  | "text-secondary"
+  | "text-tertiary"
+  | "text-selected"
+  | "text-brand"
+  | "text-white"
+  | "background"
+  | "background-selected"
+  | "background-disabled"
+  | "background-inverse"
+  | "background-brand";
+
+/**
+ * Mapping of SDK colors to main app colors. There could be additional values
+ * for new semantic colors we add to colors.module.css
+ */
 export const SDK_TO_MAIN_APP_COLORS_MAPPING: Record<
   MappableSdkColor,
-  ColorName
+  (ColorName | NEW_SEMANTIC_COLOR)[]
 > = {
-  brand: "brand",
-  border: "border",
-  filter: "filter",
-  summarize: "summarize",
-  "text-primary": "text-dark",
-  "text-secondary": "text-medium",
-  "text-tertiary": "text-light",
-  background: "bg-white",
-  "background-hover": "bg-light",
-  shadow: "shadow",
-  positive: "success",
-  negative: "danger",
+  brand: ["brand"],
+  border: ["border"],
+  filter: ["filter"],
+  summarize: ["summarize"],
+  "text-primary": ["text-dark", "text-primary"],
+  "text-secondary": ["text-medium", "text-secondary"],
+  "text-tertiary": ["text-light", "text-tertiary"],
+  background: ["bg-white", "background"],
+  "background-hover": ["bg-light"],
+  shadow: ["shadow"],
+  positive: ["success"],
+  negative: ["danger"],
 
   // positive: "success",
   // negative: "danger",
@@ -45,19 +62,20 @@ const originalColors = { ...colors };
  * @param appPalette color palette from the admin appearance settings
  */
 export function getEmbeddingColorPalette(
-  sdkColors?: MetabaseColors,
+  sdkColors: MetabaseColors = {},
   appPalette?: ColorPalette,
 ): ColorPalette {
-  if (!sdkColors) {
-    return originalColors;
-  }
-
   const mappedSdkColors = Object.fromEntries(
     Object.entries(sdkColors)
-      .map(([key, value]) => [
-        SDK_TO_MAIN_APP_COLORS_MAPPING[key as MappableSdkColor],
-        value,
-      ])
+      .flatMap(([key, value]) => {
+        const themeColorNames =
+          SDK_TO_MAIN_APP_COLORS_MAPPING[key as MappableSdkColor];
+        if (themeColorNames) {
+          return themeColorNames.map(mappedColor => [mappedColor, value]);
+        } else {
+          return [];
+        }
+      })
       .filter(([mappedKey]) => mappedKey),
   );
 

enterprise/frontend/src/embedding-sdk/lib/theme/get-embedding-theme.ts

@@ -56,10 +56,12 @@ export function getEmbeddingThemeOverride(
       const color = theme.colors[name as MetabaseColor];
 
       if (color && typeof color === "string") {
-        const themeColorName =
+        const themeColorNames =
           SDK_TO_MAIN_APP_COLORS_MAPPING[name as MappableSdkColor];
 
-        override.colors[themeColorName] = colorTuple(color);
+        for (const themeColorName of themeColorNames) {
+          override.colors[themeColorName] = colorTuple(color);
+        }
       }
     }
   }

enterprise/frontend/src/embedding-sdk/lib/theme/get-embedding-theme.unit.spec.ts

@@ -24,7 +24,9 @@ describe("Transform Embedding Theme Override", () => {
       colors: {
         brand: expect.arrayContaining(["hotpink"]),
         "text-dark": expect.arrayContaining(["yellow"]),
+        "text-primary": expect.arrayContaining(["yellow"]),
         "text-light": expect.arrayContaining(["green"]),
+        "text-tertiary": expect.arrayContaining(["green"]),
       },
       other: {
         fontSize: "2rem",

frontend/src/metabase/css/core/colors.module.css

@@ -7,8 +7,38 @@
 :root {
   /* Semantic colors */
   --mb-color-brand: var(--mb-base-color-blue-40);
-  --mb-color-brand-light: #ddecfa;
-  --mb-color-brand-lighter: #eef6fc;
+  --mb-color-brand-light: color-mix(in srgb, var(--mb-color-brand), #fff 80%);
+  --mb-color-brand-lighter: color-mix(in srgb, var(--mb-color-brand), #fff 90%);
+  --mb-color-brand-alpha-04: color-mix(
+    in srgb,
+    var(--mb-color-brand) 4%,
+    transparent
+  );
+  --mb-color-brand-alpha-88: color-mix(
+    in srgb,
+    var(--mb-color-brand) 88%,
+    transparent
+  );
+  --mb-color-border-alpha-30: color-mix(
+    in srgb,
+    var(--mb-color-border) 30%,
+    transparent
+  );
+  --mb-color-text-white-alpha-85: color-mix(
+    in srgb,
+    var(--mb-color-text-white) 85%,
+    transparent
+  );
+  --mb-color-bg-black-alpha-60: color-mix(
+    in srgb,
+    var(--mb-color-bg-black) 60%,
+    transparent
+  );
+  --mb-color-bg-white-alpha-15: color-mix(
+    in srgb,
+    var(--mb-color-bg-white) 15%,
+    transparent
+  );
   --mb-color-success: #84bb4c;
   --mb-color-summarize: #88bf4d;
   --mb-color-error: #ed6e6e;

frontend/src/metabase/styled-components/theme/css-variables.ts

@@ -2,7 +2,7 @@ import { css } from "@emotion/react";
 import { get } from "lodash";
 
 import type { MetabaseComponentTheme } from "embedding-sdk";
-import { color } from "metabase/lib/colors";
+import { SDK_TO_MAIN_APP_COLORS_MAPPING } from "embedding-sdk/lib/theme/embedding-color-palette";
 import type { MantineTheme } from "metabase/ui";
 
 // https://www.raygesualdo.com/posts/flattening-object-keys-with-typescript-types/
@@ -23,7 +23,21 @@ type MetabaseComponentThemeKey = FlattenObjectKeys<MetabaseComponentTheme>;
 export function getMetabaseCssVariables(theme: MantineTheme) {
   return css`
     :root {
-      ${getDesignSystemCssVariables(theme)}
+      --mb-default-font-family: "${theme.fontFamily}";
+
+      /* Semantic colors */
+      --mb-color-brand: ${theme.fn.themeColor("brand")};
+      --mb-color-summarize: ${theme.fn.themeColor("summarize")};
+      --mb-color-filter: ${theme.fn.themeColor("filter")};
+      ${getThemeSpecificCssVariables(theme)}
+    }
+  `;
+}
+
+export function getMetabaseSdkCssVariables(theme: MantineTheme) {
+  return css`
+    :root {
+      ${getSdkDesignSystemCssVariables(theme)}
       ${getThemeSpecificCssVariables(theme)}
     }
   `;
@@ -37,70 +51,37 @@ export function getMetabaseCssVariables(theme: MantineTheme) {
  * You don't need to add new colors from `colors.module.css` here since they'll already
  * be available globally at :root
  **/
-function getDesignSystemCssVariables(theme: MantineTheme) {
+function getSdkDesignSystemCssVariables(theme: MantineTheme) {
   return css`
     --mb-default-font-family: "${theme.fontFamily}";
 
-    --mb-color-bg-light: ${theme.fn.themeColor("bg-light")};
-    --mb-color-bg-dark: ${theme.fn.themeColor("bg-dark")};
-    --mb-color-brand: ${theme.fn.themeColor("brand")};
-
-    --mb-color-brand-light: color-mix(in srgb, var(--mb-color-brand), #fff 80%);
-    --mb-color-brand-lighter: color-mix(
-      in srgb,
-      var(--mb-color-brand),
-      #fff 90%
-    );
-    --mb-color-brand-alpha-04: color-mix(
-      in srgb,
-      var(--mb-color-brand) 4%,
-      transparent
-    );
-    --mb-color-brand-alpha-88: color-mix(
-      in srgb,
-      var(--mb-color-brand) 88%,
-      transparent
-    );
-    --mb-color-border-alpha-30: color-mix(
-      in srgb,
-      var(--mb-color-border) 30%,
-      transparent
-    );
-    --mb-color-text-white-alpha-85: color-mix(
-      in srgb,
-      var(--mb-color-text-white) 85%,
-      transparent
-    );
-    --mb-color-bg-black-alpha-60: color-mix(
-      in srgb,
-      var(--mb-color-bg-black) 60%,
-      transparent
-    );
-    --mb-color-bg-white-alpha-15: color-mix(
-      in srgb,
-      var(--mb-color-bg-white) 15%,
-      transparent
-    );
-
     /* Semantic colors */
-    --mb-color-focus: ${theme.fn.themeColor("focus")};
-    --mb-color-bg-white: ${theme.fn.themeColor("bg-white")};
-    --mb-color-bg-black: ${theme.fn.themeColor("bg-black")};
-    --mb-color-shadow: ${theme.fn.themeColor("shadow")};
-    --mb-color-border: ${theme.fn.themeColor("border")};
-    --mb-color-text-dark: ${theme.fn.themeColor("text-dark")};
-    --mb-color-text-medium: ${theme.fn.themeColor("text-medium")};
-    --mb-color-text-light: ${theme.fn.themeColor("text-light")};
-    --mb-color-danger: ${theme.fn.themeColor("danger")};
-    --mb-color-error: ${theme.fn.themeColor("error")};
-    --mb-color-filter: ${theme.fn.themeColor("filter")};
-    --mb-color-bg-error: ${color("bg-error")};
-    --mb-color-bg-medium: ${theme.fn.themeColor("bg-medium")};
-    --mb-color-bg-night: ${color("bg-night")};
-    --mb-color-text-white: ${theme.fn.themeColor("text-white")};
-    --mb-color-success: ${theme.fn.themeColor("success")};
-    --mb-color-summarize: ${theme.fn.themeColor("summarize")};
-    --mb-color-warning: ${theme.fn.themeColor("warning")};
+    /* Dynamic colors from SDK */
+    ${Object.entries(SDK_TO_MAIN_APP_COLORS_MAPPING).flatMap(
+      ([_, metabaseColorNames]) => {
+        return metabaseColorNames.map(metabaseColorName => {
+          /**
+           * Prevent returning the primary color when color is not found,
+           * so we could add a logic to fallback to the default color ourselves.
+           *
+           * We will only create CSS custom properties for colors that are defined
+           * in the palette, and additional colors overridden by the SDK.
+           *
+           * @see SDK_TO_MAIN_APP_COLORS_MAPPING
+           */
+          const color = theme.fn.themeColor(
+            metabaseColorName,
+            undefined,
+            false,
+          );
+          const colorExist = color !== metabaseColorName;
+
+          if (colorExist) {
+            return `--mb-color-${metabaseColorName}: ${color};`;
+          }
+        });
+      },
+    )}
   `;
 }