[IMP] charts: show single grid line for y-axis

When a chart had 2 vertical axis (left & right), we showed grid lines for both, which made the cahrt messy. Now we show only the grid lines for the left axis (or the right axis if there is no left axis). Task: 4153935 Part-of: #5013 Signed-off-by: Rémi Rahir (rar) <[email protected]>
odoo · Oct 11, 2024 · 9eae62c · 9eae62c
1 parent 8544e2f
commit 9eae62c
Show file tree

Hide file tree

Showing 8 changed files with 169 additions and 142 deletions.
diff --git a/src/helpers/figures/charts/bar_chart.ts b/src/helpers/figures/charts/bar_chart.ts
@@ -26,6 +26,7 @@ import { LegendPosition } from "../../../types/chart/common_chart";
 import { CellErrorType } from "../../../types/errors";
 import { Validator } from "../../../types/validator";
 import { toXlsxHexColor } from "../../../xlsx/helpers/colors";
+import { removeFalsyAttributes } from "../../misc";
 import { createValidRange } from "../../range";
 import { AbstractChart } from "./abstract_chart";
 import {
@@ -38,7 +39,7 @@ import {
   copyLabelRangeWithNewSheetId,
   createDataSets,
   formatTickValue,
-  getChartAxisTitleRuntime,
+  getChartAxis,
   getChartColorsGenerator,
   getDefinedAxis,
   getTrendDatasetForBarChart,
@@ -268,46 +269,20 @@ export function createBarChartRuntime(chart: BarChart, getters: Getters): BarCha
   config.options.indexAxis = chart.horizontal ? "y" : "x";
 
   config.options.scales = {};
-  const labelsAxis = { ticks: { padding: 5, color: fontColor } };
-  const valuesAxis = {
-    beginAtZero: true, // the origin of the y axis is always zero
-    ticks: {
-      color: fontColor,
-      callback: formatTickValue(localeFormat),
-    },
-  };
-
-  const xAxis = chart.horizontal ? valuesAxis : labelsAxis;
-  const yAxis = chart.horizontal ? labelsAxis : valuesAxis;
-  const { useLeftAxis, useRightAxis } = getDefinedAxis(chart.getDefinition());
+  const definition = chart.getDefinition();
+  const stacked = chart.stacked;
 
-  config.options.scales.x = { ...xAxis, title: getChartAxisTitleRuntime(chart.axesDesign?.x) };
-  if (useLeftAxis) {
-    config.options.scales.y = {
-      ...yAxis,
-      position: "left",
-      title: getChartAxisTitleRuntime(chart.axesDesign?.y),
-    };
-  }
-  if (useRightAxis) {
-    config.options.scales.y1 = {
-      ...yAxis,
-      position: "right",
-      title: getChartAxisTitleRuntime(chart.axesDesign?.y1),
-    };
-  }
-  if (chart.stacked) {
-    // @ts-ignore chart.js type is broken
-    config.options.scales!.x!.stacked = true;
-    if (useLeftAxis) {
-      // @ts-ignore chart.js type is broken
-      config.options.scales!.y!.stacked = true;
-    }
-    if (useRightAxis) {
-      // @ts-ignore chart.js type is broken
-      config.options.scales!.y1!.stacked = true;
-    }
+  const valueAxisOptions = { ...localeFormat, stacked };
+  const labelAxisOptions = { stacked, locale };
+  if (chart.horizontal) {
+    config.options.scales.x = getChartAxis(definition, "bottom", "values", valueAxisOptions);
+    config.options.scales.y = getChartAxis(definition, "left", "labels", labelAxisOptions);
+  } else {
+    config.options.scales.x = getChartAxis(definition, "bottom", "labels", labelAxisOptions);
+    config.options.scales.y = getChartAxis(definition, "left", "values", valueAxisOptions);
+    config.options.scales.y1 = getChartAxis(definition, "right", "values", valueAxisOptions);
   }
+  config.options.scales = removeFalsyAttributes(config.options.scales);
 
   config.options.plugins!.chartShowValuesPlugin = {
     showValues: chart.showValues,
@@ -316,7 +291,6 @@ export function createBarChartRuntime(chart: BarChart, getters: Getters): BarCha
     callback: formatTickValue(localeFormat),
   };
 
-  const definition = chart.getDefinition();
   const colors = getChartColorsGenerator(definition, dataSetsValues.length);
   const trendDatasets: any[] = [];
   for (const index in dataSetsValues) {
@@ -355,7 +329,7 @@ export function createBarChartRuntime(chart: BarChart, getters: Getters): BarCha
      */
     const maxLength = Math.max(...trendDatasets.map((trendDataset) => trendDataset.data.length));
     config.options.scales[TREND_LINE_XAXIS_ID] = {
-      ...xAxis,
+      ...(config.options.scales!.x as any),
       labels: Array(maxLength).fill(""),
       offset: false,
       display: false,

diff --git a/src/helpers/figures/charts/chart_common.ts b/src/helpers/figures/charts/chart_common.ts
@@ -1,4 +1,5 @@
-import { ChartDataset } from "chart.js";
+import { ChartDataset, LinearScaleOptions } from "chart.js";
+import { DeepPartial } from "chart.js/dist/types/utils";
 import { transformZone } from "../../../collaborative/ot/ot_helpers";
 import { LINE_FILL_TRANSPARENCY } from "../../../constants";
 import {
@@ -450,6 +451,54 @@ export function getDefinedAxis(definition: ChartWithAxisDefinition): {
   return { useLeftAxis, useRightAxis };
 }
 
+export function getChartAxis(
+  definition: ChartWithAxisDefinition,
+  position: "left" | "right" | "bottom",
+  type: "values" | "labels",
+  options: LocaleFormat & { stacked?: boolean }
+): DeepPartial<LinearScaleOptions> | undefined {
+  const { useLeftAxis, useRightAxis } = getDefinedAxis(definition);
+  if ((position === "left" && !useLeftAxis) || (position === "right" && !useRightAxis)) {
+    return undefined;
+  }
+
+  const fontColor = chartFontColor(definition.background);
+  let design: AxisDesign | undefined;
+  if (position === "bottom") {
+    design = definition.axesDesign?.x;
+  } else if (position === "left") {
+    design = definition.axesDesign?.y;
+  } else {
+    design = definition.axesDesign?.y1;
+  }
+
+  if (type === "values") {
+    const displayGridLines = position === "left" || (position === "right" && !useLeftAxis);
+    return {
+      position: position,
+      title: getChartAxisTitleRuntime(design),
+      grid: {
+        display: displayGridLines,
+      },
+      beginAtZero: true,
+      stacked: options?.stacked,
+      ticks: {
+        color: fontColor,
+        callback: formatTickValue(options),
+      },
+    };
+  } else {
+    return {
+      ticks: {
+        padding: 5,
+        color: fontColor,
+      },
+      stacked: options?.stacked,
+      title: getChartAxisTitleRuntime(design),
+    };
+  }
+}
+
 export function computeChartPadding({
   displayTitle,
   displayLegend,

diff --git a/src/helpers/figures/charts/chart_common_line_scatter.ts b/src/helpers/figures/charts/chart_common_line_scatter.ts
@@ -12,16 +12,15 @@ import {
 import { getChartTimeOptions, timeFormatLuxonCompatible } from "../../chart_date";
 import { colorToRGBA, rgbaToHex } from "../../color";
 import { formatValue } from "../../format/format";
-import { deepCopy, findNextDefinedValue, range } from "../../misc";
+import { deepCopy, findNextDefinedValue, range, removeFalsyAttributes } from "../../misc";
 import { isNumber } from "../../numbers";
 import {
   TREND_LINE_XAXIS_ID,
   chartFontColor,
   computeChartPadding,
   formatTickValue,
-  getChartAxisTitleRuntime,
+  getChartAxis,
   getChartColorsGenerator,
-  getDefinedAxis,
   getFullTrendingLineDataSet,
   interpolateData,
 } from "./chart_common";
@@ -256,50 +255,15 @@ export function createLineOrScatterChartRuntime(
     }),
   };
 
-  const xAxis = {
-    ticks: {
-      padding: 5,
-      color: fontColor,
-    },
-    title: getChartAxisTitleRuntime(chart.axesDesign?.x),
-  };
-
+  const definition = chart.getDefinition();
+  const stacked = "stacked" in chart && chart.stacked;
   config.options.scales = {
-    x: xAxis,
+    x: getChartAxis(definition, "bottom", "labels", { locale }),
+    y: getChartAxis(definition, "left", "values", { ...options, stacked }),
+    y1: getChartAxis(definition, "right", "values", { ...options, stacked }),
   };
+  config.options.scales = removeFalsyAttributes(config.options.scales);
 
-  const yAxis = {
-    beginAtZero: true, // the origin of the y axis is always zero
-    ticks: {
-      color: fontColor,
-      callback: formatTickValue(options),
-    },
-  };
-  const { useLeftAxis, useRightAxis } = getDefinedAxis(chart.getDefinition());
-  if (useLeftAxis) {
-    config.options.scales.y = {
-      ...yAxis,
-      position: "left",
-      title: getChartAxisTitleRuntime(chart.axesDesign?.y),
-    };
-  }
-  if (useRightAxis) {
-    config.options.scales.y1 = {
-      ...yAxis,
-      position: "right",
-      title: getChartAxisTitleRuntime(chart.axesDesign?.y1),
-    };
-  }
-  if ("stacked" in chart && chart.stacked) {
-    if (useLeftAxis) {
-      // @ts-ignore chart.js type is broken
-      config.options.scales!.y!.stacked = true;
-    }
-    if (useRightAxis) {
-      // @ts-ignore chart.js type is broken
-      config.options.scales!.y1!.stacked = true;
-    }
-  }
   config.options.plugins!.chartShowValuesPlugin = {
     showValues: chart.showValues,
     background: chart.background,
@@ -346,7 +310,6 @@ export function createLineOrScatterChartRuntime(
   const stackedChart = "stacked" in chart ? chart.stacked : false;
   const cumulative = "cumulative" in chart ? chart.cumulative : false;
 
-  const definition = chart.getDefinition();
   const colors = getChartColorsGenerator(definition, dataSetsValues.length);
   for (let [index, { label, data }] of dataSetsValues.entries()) {
     const color = colors.next();
@@ -412,7 +375,7 @@ export function createLineOrScatterChartRuntime(
      * set so that the second axis points match the classical x axis
      */
     config.options.scales[TREND_LINE_XAXIS_ID] = {
-      ...xAxis,
+      ...(config.options.scales.x as any),
       type: "category",
       labels: range(0, maxLength).map((x) => x.toString()),
       offset: false,

diff --git a/src/helpers/figures/charts/combo_chart.ts b/src/helpers/figures/charts/combo_chart.ts
@@ -29,6 +29,7 @@ import {
 import { CellErrorType } from "../../../types/errors";
 import { Validator } from "../../../types/validator";
 import { toXlsxHexColor } from "../../../xlsx/helpers/colors";
+import { removeFalsyAttributes } from "../../misc";
 import { createValidRange } from "../../range";
 import { AbstractChart } from "./abstract_chart";
 import {
@@ -41,7 +42,7 @@ import {
   copyLabelRangeWithNewSheetId,
   createDataSets,
   formatTickValue,
-  getChartAxisTitleRuntime,
+  getChartAxis,
   getChartColorsGenerator,
   getDefinedAxis,
   getTrendDatasetForBarChart,
@@ -271,49 +272,14 @@ export function createComboChartRuntime(chart: ComboChart, getters: Getters): Co
     }),
   };
 
+  const definition = chart.getDefinition();
   config.options.scales = {
-    x: {
-      ticks: {
-        padding: 5,
-        color: fontColor,
-      },
-      title: getChartAxisTitleRuntime(chart.axesDesign?.x),
-    },
+    x: getChartAxis(definition, "bottom", "labels", { locale }),
+    y: getChartAxis(definition, "left", "values", { locale, format: mainDataSetFormat }),
+    y1: getChartAxis(definition, "right", "values", { locale, format: lineDataSetsFormat }),
   };
+  config.options.scales = removeFalsyAttributes(config.options.scales);
 
-  const leftVerticalAxis = {
-    beginAtZero: true, // the origin of the y axis is always zero
-    ticks: {
-      color: fontColor,
-      callback: formatTickValue({ format: mainDataSetFormat, locale }),
-    },
-  };
-  const rightVerticalAxis = {
-    beginAtZero: true, // the origin of the y axis is always zero
-    ticks: {
-      color: fontColor,
-      callback: formatTickValue({ format: lineDataSetsFormat, locale }),
-    },
-  };
-  const definition = chart.getDefinition();
-  const { useLeftAxis, useRightAxis } = getDefinedAxis(definition);
-  if (useLeftAxis) {
-    config.options.scales.y = {
-      ...leftVerticalAxis,
-      position: "left",
-      title: getChartAxisTitleRuntime(chart.axesDesign?.y),
-    };
-  }
-  if (useRightAxis) {
-    config.options.scales.y1 = {
-      ...rightVerticalAxis,
-      position: "right",
-      grid: {
-        display: false,
-      },
-      title: getChartAxisTitleRuntime(chart.axesDesign?.y1),
-    };
-  }
   config.options.plugins!.chartShowValuesPlugin = {
     showValues: chart.showValues,
     background: chart.background,