[IMP] Charts: keep and disable useless dataseries for pyramid chart

Task Description

Currently, when creating a pyramid chart with more than two data series,
we are removing the unused ones, which lead to a loss of data series
when switching back to another chart type.

This PR aims to change this behavior, by keeping all the data series but
keeping only the first two in the runtime, and showing the redondant
series as disabled in the config panel.

Related Task

Task: 4570622

Author: anhe-odoo

src/components/selection_input/selection_input.ts

@@ -47,6 +47,7 @@ interface Props {
   onSelectionRemoved?: (index: number) => void;
   onSelectionConfirmed?: () => void;
   colors?: Color[];
+  enabled?: boolean[];
 }
 
 type SelectionRangeEditMode = "select-range" | "text-edit";
@@ -60,6 +61,7 @@ interface SelectionRange extends Omit<RangeInputValue, "color"> {
   isFocused: boolean;
   isValidRange: boolean;
   color?: Color;
+  enabled?: boolean;
 }
 /**
  * This component can be used when the user needs to input some
@@ -82,6 +84,7 @@ export class SelectionInput extends Component<Props, SpreadsheetChildEnv> {
     onSelectionReordered: { type: Function, optional: true },
     onSelectionRemoved: { type: Function, optional: true },
     colors: { type: Array, optional: true, default: [] },
+    enabled: { type: Array, optional: true, default: [] },
   };
   private state: State = useState({
     isMissing: false,
@@ -121,7 +124,8 @@ export class SelectionInput extends Component<Props, SpreadsheetChildEnv> {
       SelectionInputStore,
       this.props.ranges,
       this.props.hasSingleRange || false,
-      this.props.colors
+      this.props.colors,
+      this.props.enabled
     );
     onWillUpdateProps((nextProps) => {
       if (nextProps.ranges.join() !== this.store.selectionInputValues.join()) {
@@ -139,6 +143,12 @@ export class SelectionInput extends Component<Props, SpreadsheetChildEnv> {
       ) {
         this.store.updateColors(nextProps.colors || []);
       }
+      if (
+        nextProps.enabled?.join() !== this.props.enabled?.join() &&
+        nextProps.enabled?.join() !== this.store.enabled.join()
+      ) {
+        this.store.updateEnabled(nextProps.enabled || []);
+      }
     });
   }
 
@@ -179,12 +189,19 @@ export class SelectionInput extends Component<Props, SpreadsheetChildEnv> {
   }
 
   getColor(range: SelectionRange) {
+    if (!this.isEnabled(range)) {
+      return "#fff";
+    }
     if (!range.color) {
       return "";
     }
     return cssPropertiesToCss({ color: range.color });
   }
 
+  isEnabled(range: SelectionRange): boolean {
+    return range.enabled ?? true;
+  }
+
   private triggerChange() {
     const ranges = this.store.selectionInputValues;
     this.props.onSelectionChanged?.(ranges);

src/components/selection_input/selection_input.xml

@@ -29,7 +29,8 @@
             t-att-class="{
               'o-focused' : range.isFocused,
               'o-invalid border-danger position-relative': isInvalid || !range.isValidRange,
-              'text-decoration-underline': range.xc and range.isFocused and state.mode === 'select-range'
+              'text-decoration-underline': range.xc and range.isFocused and state.mode === 'select-range',
+              'o-disabled':  !isEnabled(range)
             }"
             t-ref="{{range.isFocused ? 'focusedInput' : 'unfocusedInput' + range_index}}"
           />

src/components/selection_input/selection_input_store.ts

@@ -30,6 +30,7 @@ export class SelectionInputStore extends SpreadsheetStore {
     "reset",
     "confirm",
     "updateColors",
+    "updateEnabled",
   ] as const;
   private ranges: RangeInputValue[] = [];
   focusedRangeIndex: number | null = null;
@@ -41,7 +42,8 @@ export class SelectionInputStore extends SpreadsheetStore {
     get: Get,
     private initialRanges: string[] = [],
     private readonly inputHasSingleRange: boolean = false,
-    public colors: Color[] = []
+    public colors: Color[] = [],
+    public enabled: boolean[] = []
   ) {
     super(get);
     if (inputHasSingleRange && initialRanges.length > 1) {
@@ -173,6 +175,10 @@ export class SelectionInputStore extends SpreadsheetStore {
     }));
   }
 
+  updateEnabled(enabled: boolean[]) {
+    this.enabled = enabled;
+  }
+
   confirm() {
     for (const range of this.selectionInputs) {
       if (range.xc === "") {
@@ -221,6 +227,7 @@ export class SelectionInputStore extends SpreadsheetStore {
             : null,
         isFocused: this.hasMainFocus && this.focusedRangeIndex === index,
         isValidRange: input.xc === "" || this.getters.isRangeValid(input.xc),
+        enabled: this.enabled?.[index],
       })
     );
   }

src/components/side_panel/chart/building_blocks/data_series/data_series.ts

@@ -33,6 +33,10 @@ export class ChartDataSeries extends Component<Props, SpreadsheetChildEnv> {
     return this.props.ranges.map((r) => r.backgroundColor);
   }
 
+  get enabled(): boolean[] {
+    return this.props.ranges.map((r) => !r.ignored);
+  }
+
   get title() {
     return this.props.hasSingleRange ? _t("Data range") : _t("Data series");
   }

src/components/side_panel/chart/building_blocks/data_series/data_series.xml

@@ -10,6 +10,7 @@
         onSelectionReordered="props.onSelectionReordered"
         onSelectionRemoved="props.onSelectionRemoved"
         colors="colors"
+        enabled="enabled"
       />
     </Section>
   </t>

src/components/side_panel/chart/building_blocks/generic_side_panel/config_panel.ts

@@ -128,10 +128,12 @@ export class GenericChartConfigPanel extends Component<Props, SpreadsheetChildEn
       { dataSets: this.dataSets },
       this.dataSets.length
     );
+    const ignored = this.dataSets.map((ds) => ds.ignored);
     const colors = this.dataSets.map((ds) => colorGenerator.next());
-    this.dataSets = indexes.map((i) => ({
-      backgroundColor: colors[i],
-      ...this.dataSets[i],
+    this.dataSets = indexes.map((index, i) => ({
+      backgroundColor: colors[index],
+      ...this.dataSets[index],
+      ignored: ignored[i],
     }));
     this.state.datasetDispatchResult = this.props.updateChart(this.props.figureId, {
       dataSets: this.dataSets,
@@ -144,12 +146,19 @@ export class GenericChartConfigPanel extends Component<Props, SpreadsheetChildEn
       this.dataSets.length
     );
     const colors = this.dataSets.map((ds) => colorGenerator.next());
+    const ignored = this.dataSets.map((ds) => ds.ignored);
     this.dataSets = this.dataSets
       .map((ds, i) => ({
         backgroundColor: colors[i],
         ...ds,
       }))
       .filter((_, i) => i !== index);
+    for (let i = 0; i < this.dataSets.length; i++) {
+      this.dataSets[i] = {
+        ...this.dataSets[i],
+        ignored: ignored[i],
+      };
+    }
     this.state.datasetDispatchResult = this.props.updateChart(this.props.figureId, {
       dataSets: this.dataSets,
     });

src/components/side_panel/chart/geo_chart_panel/geo_chart_config_panel.ts

@@ -1,4 +1,3 @@
-import { spreadRange } from "../../../../helpers";
 import { GenericChartConfigPanel } from "../building_blocks/generic_side_panel/config_panel";
 import { GeoChartRegionSelectSection } from "./geo_chart_region_select_section";
 
@@ -7,14 +6,7 @@ export class GeoChartConfigPanel extends GenericChartConfigPanel {
   static components = { ...GenericChartConfigPanel.components, GeoChartRegionSelectSection };
 
   get dataRanges() {
-    return this.getDataSeriesRanges().slice(0, 1);
-  }
-
-  onDataSeriesConfirmed() {
-    this.dataSets = spreadRange(this.env.model.getters, this.dataSets).slice(0, 1);
-    this.state.datasetDispatchResult = this.props.updateChart(this.props.figureId, {
-      dataSets: this.dataSets,
-    });
+    return this.getDataSeriesRanges().map((ds, i) => ({ ...ds, ignored: i > 0 }));
   }
 
   getLabelRangeOptions() {

src/components/side_panel/chart/geo_chart_panel/geo_chart_config_panel.xml

@@ -9,9 +9,10 @@
 
       <ChartDataSeries
         ranges="dataRanges"
-        onSelectionChanged="(ranges) => this.onDataSeriesRangesChanged(ranges)"
-        onSelectionConfirmed="() => this.onDataSeriesConfirmed()"
-        hasSingleRange="true"
+        onSelectionChanged.bind="onDataSeriesRangesChanged"
+        onSelectionConfirmed.bind="onDataSeriesConfirmed"
+        onSelectionReordered.bind="onDataSeriesReordered"
+        onSelectionRemoved.bind="onDataSeriesRemoved"
       />
       <ChartLabelRange
         range="this.getLabelRange()"

src/helpers/figures/charts/geo_chart.ts

@@ -109,6 +109,7 @@ export class GeoChart extends AbstractChart {
       range.push({
         ...this.dataSetDesign?.[i],
         dataRange: this.getters.getRangeString(dataSet.dataRange, this.sheetId),
+        ignored: undefined,
       });
     }
     return {
@@ -154,6 +155,7 @@ export class GeoChart extends AbstractChart {
       ranges.push({
         ...this.dataSetDesign?.[i],
         dataRange: this.getters.getRangeString(dataSet.dataRange, targetSheetId || this.sheetId),
+        ignored: i > 0,
       });
     }
     return {
@@ -193,7 +195,12 @@ export class GeoChart extends AbstractChart {
 
 export function createGeoChartRuntime(chart: GeoChart, getters: Getters): GeoChartRuntime {
   const definition = chart.getDefinition();
-  const chartData = getGeoChartData(definition, chart.dataSets, chart.labelRange, getters);
+  const chartData = getGeoChartData(
+    definition,
+    chart.dataSets.slice(0, 1),
+    chart.labelRange,
+    getters
+  );
 
   const config: ChartConfiguration = {
     type: "choropleth",

src/helpers/figures/charts/pyramid_chart.ts

@@ -66,7 +66,7 @@ export class PyramidChart extends AbstractChart {
       definition.dataSets,
       sheetId,
       definition.dataSetsHaveTitle
-    ).slice(0, 2);
+    );
     this.labelRange = createValidRange(getters, sheetId, definition.labelRange);
     this.background = definition.background;
     this.legendPosition = definition.legendPosition;
@@ -114,6 +114,7 @@ export class PyramidChart extends AbstractChart {
       range.push({
         ...this.dataSetDesign?.[i],
         dataRange: this.getters.getRangeString(dataSet.dataRange, this.sheetId),
+        ignored: undefined,
       });
     }
     return {
@@ -159,6 +160,7 @@ export class PyramidChart extends AbstractChart {
       ranges.push({
         ...this.dataSetDesign?.[i],
         dataRange: this.getters.getRangeString(dataSet.dataRange, targetSheetId || this.sheetId),
+        ignored: i > 1,
       });
     }
     return {
@@ -203,7 +205,12 @@ export function createPyramidChartRuntime(
   getters: Getters
 ): PyramidChartRuntime {
   const definition = chart.getDefinition();
-  const chartData = getPyramidChartData(definition, chart.dataSets, chart.labelRange, getters);
+  const chartData = getPyramidChartData(
+    definition,
+    chart.dataSets.slice(0, 2),
+    chart.labelRange,
+    getters
+  );
 
   const config: ChartConfiguration = {
     type: "bar",

src/types/chart/chart.ts

@@ -79,6 +79,7 @@ export interface DatasetDesign {
   readonly backgroundColor?: string;
   readonly yAxisId?: string;
   readonly label?: string;
+  readonly ignored?: boolean;
 }
 
 export interface AxisDesign {

tests/figures/chart/geochart/geo_chart_panel_component.test.ts

@@ -53,19 +53,24 @@ describe("Geo chart side panel", () => {
       expect(".o-geo-region select").toHaveValue("usa");
     });
 
-    test("Can only select a single data range", async () => {
+    test("Only one data range is enabled", async () => {
       createGeoChart(model, {
         dataSets: [{ dataRange: "A1:A3" }, { dataRange: "B1:B3" }],
       });
       await openChartConfigSidePanel(model, env, chartId);
-      expect(".o-data-series input").toHaveCount(1);
-      expect(".o-data-series input").toHaveValue("A1:A3");
+      expect(".o-data-series input").toHaveCount(2);
 
       await setInputValueAndTrigger(".o-data-series input", "A1:D3");
       await simulateClick(".o-data-series .o-selection-ok");
 
       expect(".o-data-series input").toHaveValue("A1:A3");
-      expect(getGeoChartDefinition(chartId)?.dataSets).toMatchObject([{ dataRange: "A1:A3" }]);
+      expect(getGeoChartDefinition(chartId)?.dataSets).toMatchObject([
+        { dataRange: "A1:A3", ignored: false },
+        { dataRange: "B1:B3", ignored: true },
+        { dataRange: "C1:C3", ignored: true },
+        { dataRange: "D1:D3", ignored: true },
+        { dataRange: "B1:B3", ignored: true },
+      ]);
     });
 
     test("Can change the displayed region", async () => {

tests/figures/chart/pyramid_chart/pyramid_chart_component.test.ts

@@ -20,7 +20,7 @@ describe("Pyramid chart side panel", () => {
     ({ fixture, env } = await mountComponentWithPortalTarget(SidePanel, { model }));
   });
 
-  test("Only first 2 ranges are kept when changing the selection input", async () => {
+  test("Only first 2 ranges are enabled when changing the selection input", async () => {
     createChart(model, { type: "pyramid", dataSets: [] }, "id");
     await openChartConfigSidePanel(model, env, "id");
 
@@ -30,13 +30,17 @@ describe("Pyramid chart side panel", () => {
     await simulateClick(".o-data-series .o-selection-ok");
 
     expect(getPyramidDefinition("id").dataSets).toEqual([
-      { dataRange: "A1:A5" },
-      { dataRange: "B1:B5" },
+      { dataRange: "A1:A5", ignored: false },
+      { dataRange: "B1:B5", ignored: false },
+      { dataRange: "C1:C5", ignored: true },
+      { dataRange: "D1:D5", ignored: true },
     ]);
 
     const inputs = fixture.querySelectorAll<HTMLInputElement>(".o-chart .o-data-series input");
-    expect(inputs).toHaveLength(2);
+    expect(inputs).toHaveLength(4);
     expect(inputs[0].value).toBe("A1:A5");
     expect(inputs[1].value).toBe("B1:B5");
+    expect(inputs[2].value).toBe("C1:C5");
+    expect(inputs[3].value).toBe("D1:D5");
   });
 });

tests/figures/chart/pyramid_chart/pyramid_chart_plugin.test.ts

@@ -48,11 +48,15 @@ describe("population pyramid chart", () => {
       model = new Model();
     });
 
-    test("We only keep the first two datasets", () => {
+    test("We only keep the first two datasets and ignore the others", () => {
       const dataSets = [{ dataRange: "A1" }, { dataRange: "A2" }, { dataRange: "A3" }];
       createChart(model, { type: "pyramid", dataSets }, "id");
       const definition = model.getters.getChartDefinition("id") as PyramidChartDefinition;
-      expect(definition.dataSets).toEqual([{ dataRange: "A1" }, { dataRange: "A2" }]);
+      expect(definition.dataSets).toEqual([
+        { dataRange: "A1", ignored: false },
+        { dataRange: "A2", ignored: false },
+        { dataRange: "A3", ignored: true },
+      ]);
     });
 
     test("Runtime is a stacked bar chart, with the second dataset converted to negative values", () => {