fix: Chart widget respects theme font (appsmithorg#38777)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: [email protected] <[email protected]>
Zeral-Zhang · Jan 21, 2025 · b734a0c · b734a0c
1 parent 255e0e0
commit b734a0c
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 58 deletions.
diff --git a/src/widgets/ChartWidget/component/EChartsConfigurationBuilder.ts b/src/widgets/ChartWidget/component/EChartsConfigurationBuilder.ts
@@ -9,9 +9,10 @@ import {
 
 import { Colors } from "constants/Colors";
 import { EChartsLayoutBuilder } from "./LayoutBuilders/EChartsLayoutBuilder";
+import { objectKeys } from "@appsmith/utils";
 
 export class EChartsConfigurationBuilder {
-  fontFamily: string = "Nunito Sans";
+  fontFamily: string = "";
   fontSize = 14;
 
   #seriesConfigurationForPieChart(
@@ -59,58 +60,60 @@ export class EChartsConfigurationBuilder {
      */
     const configs: unknown[] = [];
 
-    Object.keys(allSeriesData).forEach((seriesID, index) => {
-      const seriesData = allSeriesData[seriesID];
-      let color = seriesData.color;
-
-      if (index == 0 && (!color || color.length == 0)) {
-        color = props.primaryColor;
-      }
-
-      let seriesName = messages.Undefined;
-
-      if (seriesData.seriesName && seriesData.seriesName.length > 0) {
-        seriesName = seriesData.seriesName;
-      }
-
-      let config: Record<string, unknown> = {
-        label: { show: props.showDataPointLabel, position: "top" },
-        name: seriesName,
-        itemStyle: { color: color },
-      };
-
-      switch (props.chartType) {
-        case "BAR_CHART":
-          config = { ...config, type: "bar" };
-
-          // The series label should be on the right for bar chart
-          (config.label as Record<string, unknown>).position = "right";
-          break;
-        case "COLUMN_CHART":
-          config = { ...config, type: "bar" };
-          break;
-        case "LINE_CHART":
-          config = { ...config, type: "line" };
-          break;
-        case "AREA_CHART":
-          config = {
-            ...config,
-            type: "line",
-            areaStyle: {},
-          };
-          break;
-        case "PIE_CHART":
-          config = this.#seriesConfigurationForPieChart(
-            seriesID,
-            seriesData,
-            props.showDataPointLabel,
-            layoutConfig,
-          );
-          break;
-      }
-
-      configs.push(config);
-    });
+    objectKeys(allSeriesData).forEach(
+      (seriesID: string | number, index: number) => {
+        const seriesData = allSeriesData[seriesID];
+        let color = seriesData.color;
+
+        if (index == 0 && (!color || color.length == 0)) {
+          color = props.primaryColor;
+        }
+
+        let seriesName = messages.Undefined;
+
+        if (seriesData.seriesName && seriesData.seriesName.length > 0) {
+          seriesName = seriesData.seriesName;
+        }
+
+        let config: Record<string, unknown> = {
+          label: { show: props.showDataPointLabel, position: "top" },
+          name: seriesName,
+          itemStyle: { color: color },
+        };
+
+        switch (props.chartType) {
+          case "BAR_CHART":
+            config = { ...config, type: "bar" };
+
+            // The series label should be on the right for bar chart
+            (config.label as Record<string, unknown>).position = "right";
+            break;
+          case "COLUMN_CHART":
+            config = { ...config, type: "bar" };
+            break;
+          case "LINE_CHART":
+            config = { ...config, type: "line" };
+            break;
+          case "AREA_CHART":
+            config = {
+              ...config,
+              type: "line",
+              areaStyle: {},
+            };
+            break;
+          case "PIE_CHART":
+            config = this.#seriesConfigurationForPieChart(
+              String(seriesID),
+              seriesData,
+              props.showDataPointLabel,
+              layoutConfig,
+            );
+            break;
+        }
+
+        configs.push(config);
+      },
+    );
 
     return configs;
   }

diff --git a/src/widgets/ChartWidget/widget/index.test.ts b/src/widgets/ChartWidget/widget/index.test.ts
@@ -54,7 +54,24 @@ describe("emptyChartData", () => {
   };
 
   describe("font family", () => {
-    expect(ChartWidget.fontFamily).toEqual("Nunito Sans");
+    it("uses theme font family with Nunito Sans fallback", () => {
+      const widget = new ChartWidget(defaultProps);
+      const view = widget.renderChartWithData();
+
+      expect(view.props.children.props.fontFamily).toEqual("fontfamily");
+
+      const propsWithoutFont: ChartWidgetProps = {
+        ...defaultProps,
+        fontFamily: undefined as unknown as string,
+      };
+      const viewWithoutFont = new ChartWidget(
+        propsWithoutFont,
+      ).renderChartWithData();
+
+      expect(viewWithoutFont.props.children.props.fontFamily).toEqual(
+        "Nunito Sans",
+      );
+    });
   });
 
   describe("when chart type is basic ECharts", () => {

diff --git a/src/widgets/ChartWidget/widget/index.tsx b/src/widgets/ChartWidget/widget/index.tsx
@@ -3,6 +3,7 @@ import type { WidgetProps, WidgetState } from "widgets/BaseWidget";
 import BaseWidget from "widgets/BaseWidget";
 import Skeleton from "components/utils/Skeleton";
 import { retryPromise } from "utils/AppsmithUtils";
+import { objectKeys } from "@appsmith/utils";
 import { EventType } from "constants/AppsmithActionConstants/ActionConstants";
 import { contentConfig, styleConfig } from "./propertyConfig";
 import {
@@ -44,9 +45,9 @@ const ChartComponent = lazy(async () =>
 
 export const emptyChartData = (props: ChartWidgetProps) => {
   if (props.chartType == "CUSTOM_FUSION_CHART") {
-    return Object.keys(props.customFusionChartConfig).length == 0;
+    return objectKeys(props.customFusionChartConfig).length == 0;
   } else if (props.chartType == "CUSTOM_ECHART") {
-    return Object.keys(props.customEChartConfig).length == 0;
+    return objectKeys(props.customEChartConfig).length == 0;
   } else {
     const builder = new EChartsDatasetBuilder(props.chartType, props.chartData);
 
@@ -65,7 +66,6 @@ export const emptyChartData = (props: ChartWidgetProps) => {
 
 class ChartWidget extends BaseWidget<ChartWidgetProps, WidgetState> {
   static type = "CHART_WIDGET";
-  static fontFamily: string = "Nunito Sans";
 
   static getConfig() {
     return {
@@ -257,7 +257,7 @@ class ChartWidget extends BaseWidget<ChartWidgetProps, WidgetState> {
           customEChartConfig={this.props.customEChartConfig}
           customFusionChartConfig={this.props.customFusionChartConfig}
           dimensions={this.props}
-          fontFamily={ChartWidget.fontFamily}
+          fontFamily={this.props.fontFamily ?? "Nunito Sans"}
           hasOnDataPointClick={Boolean(this.props.onDataPointClick)}
           isLoading={this.props.isLoading}
           isVisible={this.props.isVisible}