[FIX] Charts: format of negative values

- Rectified the display format of negative values within tooltips and axis labels in charts. Task ID: 3829782 closes #3935 Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
odoo · Mar 29, 2024 · 06db896 · 06db896
1 parent c2eed35
commit 06db896
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 13 deletions.
diff --git a/src/helpers/figures/charts/bar_chart.ts b/src/helpers/figures/charts/bar_chart.ts
@@ -233,7 +233,10 @@ function getBarConfiguration(
           value = Number(value);
           if (isNaN(value)) return value;
           const { locale, format } = localeFormat;
-          return formatValue(value, { locale, format: !format && value > 1000 ? "#,##" : format });
+          return formatValue(value, {
+            locale,
+            format: !format && Math.abs(value) >= 1000 ? "#,##" : format,
+          });
         },
       },
     },

diff --git a/src/helpers/figures/charts/chart_ui_common.ts b/src/helpers/figures/charts/chart_ui_common.ts
@@ -136,7 +136,7 @@ export function getDefaultChartJsRuntime(
             const xLabel = tooltipItem.dataset?.label || tooltipItem.label;
             // tooltipItem.parsed.y can be an object or a number for pie charts
             const yLabel = tooltipItem.parsed.y ?? tooltipItem.parsed;
-            const toolTipFormat = !format && yLabel > 1000 ? "#,##" : format;
+            const toolTipFormat = !format && Math.abs(yLabel) >= 1000 ? "#,##" : format;
             const yLabelStr = formatValue(yLabel, { format: toolTipFormat, locale });
             return xLabel ? `${xLabel}: ${yLabelStr}` : yLabelStr;
           },

diff --git a/src/helpers/figures/charts/line_chart.ts b/src/helpers/figures/charts/line_chart.ts
@@ -336,7 +336,10 @@ function getLineConfiguration(
           value = Number(value);
           if (isNaN(value)) return value;
           const { locale, format } = localeFormat;
-          return formatValue(value, { locale, format: !format && value > 1000 ? "#,##" : format });
+          return formatValue(value, {
+            locale,
+            format: !format && Math.abs(value) >= 1000 ? "#,##" : format,
+          });
         },
       },
     },

diff --git a/src/helpers/figures/charts/pie_chart.ts b/src/helpers/figures/charts/pie_chart.ts
@@ -227,7 +227,7 @@ function getPieConfiguration(
 
     const xLabel = tooltipItem.label || tooltipItem.dataset.label;
     const yLabel = tooltipItem.parsed.y ?? tooltipItem.parsed;
-    const toolTipFormat = !format && yLabel > 1000 ? "#,##" : format;
+    const toolTipFormat = !format && Math.abs(yLabel) >= 1000 ? "#,##" : format;
     const yLabelStr = formatValue(yLabel, { format: toolTipFormat, locale });
 
     return xLabel ? `${xLabel}: ${yLabelStr} (${percentage}%)` : `${yLabelStr} (${percentage}%)`;

diff --git a/tests/figures/chart/chart_plugin.test.ts b/tests/figures/chart/chart_plugin.test.ts
@@ -1568,6 +1568,10 @@ describe("Chart design configuration", () => {
         expect(runtime.chartJsConfig.options.scales.y?.ticks.callback!(60000000)).toEqual(
           "60,000,000"
         );
+        //@ts-ignore
+        expect(runtime.chartJsConfig.options.scales.y?.ticks.callback!(-60000000)).toEqual(
+          "-60,000,000"
+        );
       }
     );
 
@@ -1581,6 +1585,10 @@ describe("Chart design configuration", () => {
         expect(runtime.chartJsConfig.options.scales.y?.ticks.callback!(60000000)).toEqual(
           "60 000 000"
         );
+        // @ts-ignore
+        expect(runtime.chartJsConfig.options.scales.y?.ticks.callback!(-60000000)).toEqual(
+          "-60 000 000"
+        );
       }
     );
 
@@ -1603,7 +1611,7 @@ describe("Chart design configuration", () => {
     });
 
     test.each(["bar", "line"])(
-      "Basic chart tooltip label, cell without format: thousand separator",
+      "Basic chart tooltip label, cell without format: thousand separator for positive values",
       (chartType) => {
         setCellContent(model, "A2", "60000000");
         createChart(model, { ...defaultChart, type: chartType as "bar" | "line" | "pie" }, "42");
@@ -1614,14 +1622,17 @@ describe("Chart design configuration", () => {
       }
     );
 
-    test.each(["bar", "line"])("Basic chart tooltip label, cell with format", (chartType) => {
-      setCellContent(model, "A2", "6000");
-      setCellFormat(model, "A2", "[$$]#,##0.00");
-      createChart(model, { ...defaultChart, type: chartType as "bar" | "line" | "pie" }, "42");
-      const runtime = model.getters.getChartRuntime("42") as BarChartRuntime;
-      const label = getTooltipLabel(runtime, 0, 0);
-      expect(label).toEqual("$6,000.00");
-    });
+    test.each(["bar", "line"])(
+      "Basic chart tooltip label, cell without format: thousand separator for negative values",
+      (chartType) => {
+        setCellContent(model, "A2", "-60000000");
+        createChart(model, { ...defaultChart, type: chartType as "bar" | "line" | "pie" }, "42");
+        const runtime = model.getters.getChartRuntime("42") as BarChartRuntime;
+        const label = getTooltipLabel(runtime, 0, 0);
+
+        expect(label).toEqual("-60,000,000");
+      }
+    );
 
     test.each(["bar", "line"])("Basic chart tooltip label, date format is ignored", (chartType) => {
       setCellContent(model, "A2", "6000");
@@ -1672,6 +1683,24 @@ describe("Chart design configuration", () => {
       expect(label).toBe("P1: $6,000.00 (100.00%)");
     });
   });
+
+  test("pie chart tooltip label with negative value", () => {
+    setCellContent(model, "A1", "P1");
+    setCellContent(model, "A2", "-6000");
+    createChart(
+      model,
+      {
+        dataSets: ["A1:A2"],
+        labelRange: "A1",
+        type: "pie",
+      },
+      "1"
+    );
+    const chart = model.getters.getChartRuntime("1") as PieChartRuntime;
+    const label = getTooltipLabel(chart, 0, 0);
+
+    expect(label).toBe("P1: -6,000 (100.00%)");
+  });
 });
 
 describe("Chart aggregate labels", () => {