[IMP] Chart: use first row as header

This commit changes the current option "Datasets include title" to
"Use row x as headers", which is more similar to the functionality in GSheets.
It will exclude the first row in the datasets/labels when creating the chart.
It doesn't take the col into consideration for simplicity.

task 3099995

closes #1903

Signed-off-by: Rémi Rahir (rar) <[email protected]>

src/components/composer/grid_composer/grid_composer.ts

@@ -3,7 +3,7 @@ import { ComponentsImportance, SELECTION_BORDER_COLOR } from "../../../constants
 import {
   deepEquals,
   fontSizeInPixels,
-  getComposerSheetName,
+  getCanonicalSheetName,
   positionToZone,
   toXC,
 } from "../../../helpers";
@@ -116,7 +116,7 @@ export class GridComposer extends Component<Props, SpreadsheetChildEnv> {
     const { col, row, sheetId } = this.env.model.getters.getCurrentEditedCell();
     const prefixSheet = sheetId !== this.env.model.getters.getActiveSheetId();
     return `${
-      prefixSheet ? getComposerSheetName(this.env.model.getters.getSheetName(sheetId)) + "!" : ""
+      prefixSheet ? getCanonicalSheetName(this.env.model.getters.getSheetName(sheetId)) + "!" : ""
     }${toXC(col, row)}`;
   }
 

src/components/side_panel/chart/bar_chart/bar_chart_config_panel.xml

@@ -24,15 +24,6 @@
           onSelectionChanged="(ranges) => this.onDataSeriesRangesChanged(ranges)"
           onSelectionConfirmed="() => this.onDataSeriesConfirmed()"
         />
-        <label>
-          <input
-            type="checkbox"
-            t-att-checked="props.definition.dataSetsHaveTitle"
-            t-on-change="onUpdateDataSetsHaveTitle"
-            class="align-middle"
-          />
-          Data series include title
-        </label>
       </div>
       <div class="o-section o-data-labels">
         <div class="o-section-title">Categories / Labels</div>
@@ -54,6 +45,19 @@
           Aggregate
         </label>
       </div>
+      <div class="o-section o-use-row-as-headers" t-if="calculateHeaderPosition()">
+        <label>
+          <input
+            type="checkbox"
+            t-att-checked="props.definition.dataSetsHaveTitle"
+            t-on-change="onUpdateDataSetsHaveTitle"
+            class="align-middle"
+          />
+          Use row
+          <span t-esc="calculateHeaderPosition()"/>
+          as headers
+        </label>
+      </div>
       <div class="o-section o-sidepanel-error" t-if="errorMessages">
         <div t-foreach="errorMessages" t-as="error" t-key="error">
           <t t-esc="error"/>

src/components/side_panel/chart/line_bar_pie_panel/config_panel.ts

@@ -1,4 +1,6 @@
 import { Component, useState } from "@odoo/owl";
+import { createRange } from "../../../../helpers";
+import { createDataSets } from "../../../../helpers/figures/charts";
 import { BarChartDefinition } from "../../../../types/chart/bar_chart";
 import { LineChartDefinition } from "../../../../types/chart/line_chart";
 import { PieChartDefinition } from "../../../../types/chart/pie_chart";
@@ -93,6 +95,27 @@ export class LineBarPieConfigPanel extends Component<Props, SpreadsheetChildEnv>
       aggregated: ev.target.checked,
     });
   }
+
+  calculateHeaderPosition(): number | undefined {
+    if (this.isDatasetInvalid || this.isLabelInvalid) {
+      return undefined;
+    }
+    const getters = this.env.model.getters;
+    const sheetId = getters.getActiveSheetId();
+    const labelRange = createRange(getters, sheetId, this.labelRange);
+    const dataSets = createDataSets(
+      getters,
+      this.dataSeriesRanges,
+      sheetId,
+      this.props.definition.dataSetsHaveTitle
+    );
+    if (dataSets.length) {
+      return dataSets[0].dataRange.zone.top + 1;
+    } else if (labelRange) {
+      return labelRange.zone.top + 1;
+    }
+    return undefined;
+  }
 }
 
 LineBarPieConfigPanel.props = {

src/components/side_panel/chart/line_bar_pie_panel/config_panel.xml

@@ -10,15 +10,6 @@
           onSelectionChanged="(ranges) => this.onDataSeriesRangesChanged(ranges)"
           onSelectionConfirmed="() => this.onDataSeriesConfirmed()"
         />
-        <label>
-          <input
-            type="checkbox"
-            t-att-checked="props.definition.dataSetsHaveTitle"
-            t-on-change="onUpdateDataSetsHaveTitle"
-            class="align-middle"
-          />
-          Data series include title
-        </label>
       </div>
       <div class="o-section o-data-labels">
         <div class="o-section-title">Categories / Labels</div>
@@ -40,6 +31,19 @@
           Aggregate
         </label>
       </div>
+      <div class="o-section o-use-row-as-headers" t-if="calculateHeaderPosition()">
+        <label>
+          <input
+            type="checkbox"
+            t-att-checked="props.definition.dataSetsHaveTitle"
+            t-on-change="onUpdateDataSetsHaveTitle"
+            class="align-middle"
+          />
+          Use row
+          <span t-esc="calculateHeaderPosition()"/>
+          as headers
+        </label>
+      </div>
       <div class="o-section o-sidepanel-error" t-if="errorMessages">
         <div t-foreach="errorMessages" t-as="error" t-key="error">
           <t t-esc="error"/>

src/components/side_panel/chart/line_chart/line_chart_config_panel.xml

@@ -24,15 +24,6 @@
           onSelectionChanged="(ranges) => this.onDataSeriesRangesChanged(ranges)"
           onSelectionConfirmed="() => this.onDataSeriesConfirmed()"
         />
-        <label>
-          <input
-            type="checkbox"
-            t-att-checked="props.definition.dataSetsHaveTitle"
-            t-on-change="onUpdateDataSetsHaveTitle"
-            class="align-middle"
-          />
-          Data series include title
-        </label>
       </div>
       <div class="o-section o-data-labels">
         <div class="o-section-title">Categories / Labels</div>
@@ -65,6 +56,19 @@
           </label>
         </div>
       </div>
+      <div class="o-section o-use-row-as-headers" t-if="calculateHeaderPosition()">
+        <label>
+          <input
+            type="checkbox"
+            t-att-checked="props.definition.dataSetsHaveTitle"
+            t-on-change="onUpdateDataSetsHaveTitle"
+            class="align-middle"
+          />
+          Use row
+          <span t-esc="calculateHeaderPosition()"/>
+          as headers
+        </label>
+      </div>
       <div class="o-section o-sidepanel-error" t-if="errorMessages">
         <div t-foreach="errorMessages" t-as="error" t-key="error">
           <t t-esc="error"/>

src/functions/module_lookup.ts

@@ -1,4 +1,4 @@
-import { getComposerSheetName, toXC, toZone } from "../helpers/index";
+import { getCanonicalSheetName, toXC, toZone } from "../helpers/index";
 import { _lt } from "../translation";
 import {
   AddFunctionDescription,
@@ -96,7 +96,7 @@ export const ADDRESS: AddFunctionDescription = {
       cellReference = rowPart + colPart;
     }
     if (sheet !== undefined) {
-      return `${getComposerSheetName(toString(sheet))}!${cellReference}`;
+      return `${getCanonicalSheetName(toString(sheet))}!${cellReference}`;
     }
     return cellReference;
   },

src/helpers/figures/charts/bar_chart.ts

@@ -33,7 +33,9 @@ import {
   copyDataSetsWithNewSheetId,
   copyLabelRangeWithNewSheetId,
   createDataSets,
+  shouldRemoveFirstLabel,
   toExcelDataset,
+  toExcelLabelRange,
   transformChartDefinitionWithDataSetsWithZone,
   updateChartRangesWithDataSets,
 } from "./chart_common";
@@ -55,6 +57,7 @@ export class BarChart extends AbstractChart {
   readonly stacked: boolean;
   readonly aggregated?: boolean;
   readonly type = "bar";
+  readonly dataSetsHaveTitle: boolean;
 
   constructor(definition: BarChartDefinition, sheetId: UID, getters: CoreGetters) {
     super(definition, sheetId, getters);
@@ -70,6 +73,7 @@ export class BarChart extends AbstractChart {
     this.legendPosition = definition.legendPosition;
     this.stacked = definition.stacked;
     this.aggregated = definition.aggregated;
+    this.dataSetsHaveTitle = definition.dataSetsHaveTitle;
   }
 
   static transformDefinition(
@@ -163,11 +167,17 @@ export class BarChart extends AbstractChart {
     const dataSets: ExcelChartDataset[] = this.dataSets
       .map((ds: DataSet) => toExcelDataset(this.getters, ds))
       .filter((ds) => ds.range !== ""); // && range !== INCORRECT_RANGE_STRING ? show incorrect #ref ?
+    const labelRange = toExcelLabelRange(
+      this.getters,
+      this.labelRange,
+      shouldRemoveFirstLabel(this.labelRange, this.dataSets[0], this.dataSetsHaveTitle)
+    );
     return {
       ...this.getDefinition(),
       backgroundColor: toXlsxHexColor(this.background || BACKGROUND_CHART_COLOR),
       fontColor: toXlsxHexColor(chartFontColor(this.background)),
       dataSets,
+      labelRange,
     };
   }
 
@@ -251,6 +261,13 @@ export function createBarChartRuntime(chart: BarChart, getters: Getters): BarCha
   const labelValues = getChartLabelValues(getters, chart.dataSets, chart.labelRange);
   let labels = labelValues.formattedValues;
   let dataSetsValues = getChartDatasetValues(getters, chart.dataSets);
+  if (
+    chart.dataSetsHaveTitle &&
+    dataSetsValues[0] &&
+    labels.length > dataSetsValues[0].data.length
+  ) {
+    labels.shift();
+  }
 
   ({ labels, dataSetsValues } = filterEmptyDataPoints(labels, dataSetsValues));
   if (chart.aggregated) {

src/helpers/figures/charts/chart_common.ts

@@ -27,7 +27,7 @@ import { formatValue } from "../../format";
 import { isDefined } from "../../misc";
 import { copyRangeWithNewSheetId } from "../../range";
 import { rangeReference } from "../../references";
-import { isFullRow, toUnboundedZone, zoneToDimension, zoneToXc } from "../../zones";
+import { getZoneArea, isFullRow, toUnboundedZone, zoneToDimension, zoneToXc } from "../../zones";
 
 /**
  * This file contains helpers that are common to different charts (mainly
@@ -151,8 +151,8 @@ export function createDataSets(
   const dataSets: DataSet[] = [];
   for (const sheetXC of dataSetsString) {
     const dataRange = getters.getRangeFromSheetXC(sheetId, sheetXC);
-    const { unboundedZone: zone, sheetId: dataSetSheetId, invalidSheetName } = dataRange;
-    if (invalidSheetName) {
+    const { unboundedZone: zone, sheetId: dataSetSheetId, invalidSheetName, invalidXc } = dataRange;
+    if (invalidSheetName || invalidXc) {
       continue;
     }
     // It's a rectangle. We treat all columns (arbitrary) as different data series.
@@ -184,13 +184,8 @@ export function createDataSets(
           )
         );
       }
-    } else if (zone.left === zone.right && zone.top === zone.bottom) {
-      // A single cell. If it's only the title, the dataset is not added.
-      if (!dataSetsHaveTitle) {
-        dataSets.push(createDataSet(getters, dataSetSheetId, zone, undefined));
-      }
     } else {
-      /* 1 row or 1 column */
+      /* 1 cell, 1 row or 1 column */
       dataSets.push(
         createDataSet(
           getters,
@@ -258,6 +253,22 @@ export function toExcelDataset(getters: CoreGetters, ds: DataSet): ExcelChartDat
   };
 }
 
+export function toExcelLabelRange(
+  getters: CoreGetters,
+  labelRange: Range | undefined,
+  shouldRemoveFirstLabel?: boolean
+) {
+  if (!labelRange) return undefined;
+  let zone = {
+    ...labelRange.zone,
+  };
+  if (shouldRemoveFirstLabel && labelRange.zone.bottom > labelRange.zone.top) {
+    zone.top = zone.top + 1;
+  }
+  const range = labelRange.clone({ zone });
+  return getters.getRangeString(range, "forceSheetReference");
+}
+
 /**
  * Transform a chart definition which supports dataSets (dataSets and LabelRange)
  * with an executed command
@@ -354,6 +365,22 @@ export function checkLabelRange(
   return CommandResult.Success;
 }
 
+export function shouldRemoveFirstLabel(
+  labelRange: Range | undefined,
+  dataset: DataSet | undefined,
+  dataSetsHaveTitle: boolean
+) {
+  if (!dataSetsHaveTitle) return false;
+  if (!labelRange) return false;
+  if (!dataset) return true;
+  const datasetLength = getZoneArea(dataset.dataRange.zone);
+  const labelLength = getZoneArea(labelRange.zone);
+  if (labelLength < datasetLength) {
+    return false;
+  }
+  return true;
+}
+
 // ---------------------------------------------------------------------------
 // Scorecard
 // ---------------------------------------------------------------------------

src/helpers/figures/charts/chart_factory.ts

@@ -163,7 +163,6 @@ export function getSmartChartDefinition(zone: Zone, getters: Getters): ChartDefi
     labelRangeXc = zoneToXc({
       ...zone,
       right: zone.left,
-      top: dataSetsHaveTitle ? zone.top + 1 : zone.top,
     });
   }
   // Only display legend for several datasets.

src/helpers/figures/charts/line_chart.ts

@@ -38,7 +38,9 @@ import {
   copyDataSetsWithNewSheetId,
   copyLabelRangeWithNewSheetId,
   createDataSets,
+  shouldRemoveFirstLabel,
   toExcelDataset,
+  toExcelLabelRange,
   transformChartDefinitionWithDataSetsWithZone,
   updateChartRangesWithDataSets,
 } from "./chart_common";
@@ -63,6 +65,7 @@ export class LineChart extends AbstractChart {
   readonly stacked: boolean;
   readonly aggregated?: boolean;
   readonly type = "line";
+  readonly dataSetsHaveTitle: boolean;
 
   constructor(definition: LineChartDefinition, sheetId: UID, getters: CoreGetters) {
     super(definition, sheetId, getters);
@@ -79,6 +82,7 @@ export class LineChart extends AbstractChart {
     this.labelsAsText = definition.labelsAsText;
     this.stacked = definition.stacked;
     this.aggregated = definition.aggregated;
+    this.dataSetsHaveTitle = definition.dataSetsHaveTitle;
   }
 
   static validateChartDefinition(
@@ -172,11 +176,17 @@ export class LineChart extends AbstractChart {
     const dataSets: ExcelChartDataset[] = this.dataSets
       .map((ds: DataSet) => toExcelDataset(this.getters, ds))
       .filter((ds) => ds.range !== ""); // && range !== INCORRECT_RANGE_STRING ? show incorrect #ref ?
+    const labelRange = toExcelLabelRange(
+      this.getters,
+      this.labelRange,
+      shouldRemoveFirstLabel(this.labelRange, this.dataSets[0], this.dataSetsHaveTitle)
+    );
     return {
       ...this.getDefinition(),
       backgroundColor: toXlsxHexColor(this.background || BACKGROUND_CHART_COLOR),
       fontColor: toXlsxHexColor(chartFontColor(this.background)),
       dataSets,
+      labelRange,
     };
   }
 
@@ -342,6 +352,13 @@ export function createLineChartRuntime(chart: LineChart, getters: Getters): Line
   const labelValues = getChartLabelValues(getters, chart.dataSets, chart.labelRange);
   let labels = axisType === "linear" ? labelValues.values : labelValues.formattedValues;
   let dataSetsValues = getChartDatasetValues(getters, chart.dataSets);
+  if (
+    chart.dataSetsHaveTitle &&
+    dataSetsValues[0] &&
+    labels.length > dataSetsValues[0].data.length
+  ) {
+    labels.shift();
+  }
 
   ({ labels, dataSetsValues } = filterEmptyDataPoints(labels, dataSetsValues));
   if (axisType === "time") {