[IMP] charts: disable legend for single dataset

Task Description When drawing a chart with only one dataset, we are allowed to choose the position of the legend but no legend will appears even after changing the position. This commit aims to fix thie issue, disabling the select box allowing to change the legend position. The chart padding has also been adapted to have more padding when there is no title/legend. Related Task Task: 4028903
odoo · Jul 8, 2024 · 1fab50c · 1fab50c
1 parent 7cfe14a
commit 1fab50c
Show file tree

Hide file tree

Showing 7 changed files with 50 additions and 25 deletions.
diff --git a/src/components/side_panel/chart/chart_with_axis/design_panel.xml b/src/components/side_panel/chart/chart_with_axis/design_panel.xml
@@ -9,7 +9,7 @@
           <t t-set-slot="title">Legend position</t>
           <select
             t-att-value="props.definition.legendPosition ?? 'top'"
-            class="o-input"
+            class="o-input legend-position-selector"
             t-on-change="this.updateLegendPosition">
             <option value="none">None</option>
             <option value="top">Top</option>

diff --git a/src/helpers/figures/charts/bar_chart.ts b/src/helpers/figures/charts/bar_chart.ts
@@ -35,6 +35,7 @@ import {
   chartFontColor,
   checkDataset,
   checkLabelRange,
+  computeChartPadding,
   copyDataSetsWithNewSheetId,
   copyLabelRangeWithNewSheetId,
   createDataSets,
@@ -232,14 +233,14 @@ function getBarConfiguration(
   const legend: DeepPartial<LegendOptions<"bar">> = {
     labels: { color: fontColor },
   };
-  if ((!chart.labelRange && chart.dataSets.length === 1) || chart.legendPosition === "none") {
+  if (chart.legendPosition === "none") {
     legend.display = false;
   } else {
     legend.position = chart.legendPosition;
   }
   config.options.plugins!.legend = { ...config.options.plugins?.legend, ...legend };
   config.options.layout = {
-    padding: { left: 20, right: 20, top: chart.title ? 10 : 25, bottom: 10 },
+    padding: computeChartPadding(!!chart.title.text, legend.display !== false),
   };
   config.options.indexAxis = chart.horizontal ? "y" : "x";
 

diff --git a/src/helpers/figures/charts/chart_common.ts b/src/helpers/figures/charts/chart_common.ts
@@ -428,3 +428,21 @@ export function getDefinedAxis(definition: ChartWithAxisDefinition): {
   useLeftAxis ||= !useRightAxis;
   return { useLeftAxis, useRightAxis };
 }
+
+export function computeChartPadding(
+  displayTitle: boolean,
+  displayLegend: boolean
+): {
+  top: number;
+  bottom: number;
+  left: number;
+  right: number;
+} {
+  let top = 25;
+  if (displayTitle) {
+    top = 0;
+  } else if (displayLegend) {
+    top = 10;
+  }
+  return { left: 20, right: 20, top, bottom: 10 };
+}
diff --git a/src/helpers/figures/charts/chart_common_line_scatter.ts b/src/helpers/figures/charts/chart_common_line_scatter.ts
@@ -7,7 +7,12 @@ import { getChartTimeOptions, timeFormatLuxonCompatible } from "../../chart_date
 import { ColorGenerator, colorToRGBA, rgbaToHex } from "../../color";
 import { formatValue } from "../../format";
 import { deepCopy, findNextDefinedValue } from "../../misc";
-import { chartFontColor, getChartAxisTitleRuntime, getDefinedAxis } from "./chart_common";
+import {
+  chartFontColor,
+  computeChartPadding,
+  getChartAxisTitleRuntime,
+  getDefinedAxis,
+} from "./chart_common";
 import {
   aggregateDataForLabels,
   filterEmptyDataPoints,
@@ -135,14 +140,14 @@ function getLineOrScatterConfiguration(
       },
     },
   };
-  if ((!chart.labelRange && chart.dataSets.length === 1) || chart.legendPosition === "none") {
+  if (chart.legendPosition === "none") {
     legend.display = false;
   } else {
     legend.position = chart.legendPosition;
   }
   Object.assign(config.options.plugins!.legend || {}, legend);
   config.options.layout = {
-    padding: { left: 20, right: 20, top: chart.title ? 10 : 25, bottom: 10 },
+    padding: computeChartPadding(!!chart.title.text, legend.display !== false),
   };
 
   config.options.scales = {

diff --git a/src/helpers/figures/charts/combo_chart.ts b/src/helpers/figures/charts/combo_chart.ts
@@ -35,6 +35,7 @@ import {
   chartFontColor,
   checkDataset,
   checkLabelRange,
+  computeChartPadding,
   copyDataSetsWithNewSheetId,
   copyLabelRangeWithNewSheetId,
   createDataSets,
@@ -245,14 +246,14 @@ export function createComboChartRuntime(chart: ComboChart, getters: Getters): Co
     labels: { color: fontColor },
     reverse: true,
   };
-  if ((!chart.labelRange && chart.dataSets.length === 1) || chart.legendPosition === "none") {
+  if (chart.legendPosition === "none") {
     legend.display = false;
   } else {
     legend.position = chart.legendPosition;
   }
   config.options.plugins!.legend = { ...config.options.plugins?.legend, ...legend };
   config.options.layout = {
-    padding: { left: 20, right: 20, top: chart.title ? 10 : 25, bottom: 10 },
+    padding: computeChartPadding(!!chart.title.text, legend.display !== false),
   };
 
   config.options.scales = {

diff --git a/tests/figures/chart/__snapshots__/chart_plugin.test.ts.snap b/tests/figures/chart/__snapshots__/chart_plugin.test.ts.snap
@@ -50,7 +50,7 @@ exports[`Linear/Time charts font color is white with a dark background color 1`]
           "bottom": 10,
           "left": 20,
           "right": 20,
-          "top": 10,
+          "top": 0,
         },
       },
       "maintainAspectRatio": false,
@@ -165,7 +165,7 @@ exports[`Linear/Time charts snapshot test of chartJS configuration for date char
           "bottom": 10,
           "left": 20,
           "right": 20,
-          "top": 10,
+          "top": 0,
         },
       },
       "maintainAspectRatio": false,
@@ -286,7 +286,7 @@ exports[`Linear/Time charts snapshot test of chartJS configuration for linear ch
           "bottom": 10,
           "left": 20,
           "right": 20,
-          "top": 10,
+          "top": 0,
         },
       },
       "maintainAspectRatio": false,
@@ -381,7 +381,7 @@ exports[`datasource tests create a chart with stacked bar 1`] = `
           "bottom": 10,
           "left": 20,
           "right": 20,
-          "top": 10,
+          "top": 0,
         },
       },
       "maintainAspectRatio": false,
@@ -476,7 +476,7 @@ exports[`datasource tests create chart with a dataset of one cell (no title) 1`]
           "bottom": 10,
           "left": 20,
           "right": 20,
-          "top": 10,
+          "top": 0,
         },
       },
       "maintainAspectRatio": false,
@@ -587,7 +587,7 @@ exports[`datasource tests create chart with column datasets 1`] = `
           "bottom": 10,
           "left": 20,
           "right": 20,
-          "top": 10,
+          "top": 0,
         },
       },
       "maintainAspectRatio": false,
@@ -698,7 +698,7 @@ exports[`datasource tests create chart with column datasets with category title
           "bottom": 10,
           "left": 20,
           "right": 20,
-          "top": 10,
+          "top": 0,
         },
       },
       "maintainAspectRatio": false,
@@ -809,7 +809,7 @@ exports[`datasource tests create chart with column datasets without series title
           "bottom": 10,
           "left": 20,
           "right": 20,
-          "top": 10,
+          "top": 0,
         },
       },
       "maintainAspectRatio": false,
@@ -905,7 +905,7 @@ exports[`datasource tests create chart with only the dataset title (no data) 1`]
           "bottom": 10,
           "left": 20,
           "right": 20,
-          "top": 10,
+          "top": 0,
         },
       },
       "maintainAspectRatio": false,
@@ -1016,7 +1016,7 @@ exports[`datasource tests create chart with rectangle dataset 1`] = `
           "bottom": 10,
           "left": 20,
           "right": 20,
-          "top": 10,
+          "top": 0,
         },
       },
       "maintainAspectRatio": false,
@@ -1127,7 +1127,7 @@ exports[`datasource tests create chart with row datasets 1`] = `
           "bottom": 10,
           "left": 20,
           "right": 20,
-          "top": 10,
+          "top": 0,
         },
       },
       "maintainAspectRatio": false,
@@ -1238,7 +1238,7 @@ exports[`datasource tests create chart with row datasets with category title 1`]
           "bottom": 10,
           "left": 20,
           "right": 20,
-          "top": 10,
+          "top": 0,
         },
       },
       "maintainAspectRatio": false,
@@ -1349,7 +1349,7 @@ exports[`datasource tests create chart with row datasets without series title 1`
           "bottom": 10,
           "left": 20,
           "right": 20,
-          "top": 10,
+          "top": 0,
         },
       },
       "maintainAspectRatio": false,

diff --git a/tests/figures/chart/chart_plugin.test.ts b/tests/figures/chart/chart_plugin.test.ts
@@ -1555,19 +1555,19 @@ describe("Chart without labels", () => {
     aggregated: false,
   };
 
-  test("The legend is not displayed when there is only one dataSet and no label", () => {
+  test("The legend is displayed even when there is only one dataSet or no label", () => {
     createChart(model, defaultChart, "42");
-    expect(getChartConfiguration(model, "42").options?.plugins?.legend?.display).toBe(false);
+    expect(getChartConfiguration(model, "42").options?.plugins?.legend?.position).toBe("top");
 
     createChart(
       model,
       { ...defaultChart, dataSets: [{ dataRange: "A1:A2" }, { dataRange: "A3:A4" }] },
       "43"
     );
-    expect(getChartConfiguration(model, "43").options?.plugins?.legend?.display).toBeUndefined();
+    expect(getChartConfiguration(model, "42").options?.plugins?.legend?.position).toBe("top");
 
     createChart(model, { ...defaultChart, labelRange: "B1:B2" }, "44");
-    expect(getChartConfiguration(model, "44").options?.plugins?.legend?.display).toBeUndefined();
+    expect(getChartConfiguration(model, "42").options?.plugins?.legend?.position).toBe("top");
   });
 
   test("Labels are empty if there is only one dataSet and no label", () => {