[FIX] selection input: highlight color not sync with range color

After re-ordering the selection inputs of a chart, the highlight colors
would not be the same as the range colors.

This was because the store mutator `updateColors` would not update
the store range colors. And `store.highlights` is based on
`ranges.colors` while `store.selectionInputs` ignores the colors
of `store.ranges`.

The test `update of colors are taken into account` was also a bit
sketchy. We would modify the `props.colors` array in place, thus
the condition `nextProps.colors !== props.colors` would never
be satisfied, and `store.updateColors` would never be called.

Task: 4577803

Author: hokolomopo

src/components/selection_input/selection_input_store.ts

@@ -31,7 +31,7 @@ export class SelectionInputStore extends SpreadsheetStore {
     "confirm",
     "updateColors",
   ] as const;
-  ranges: RangeInputValue[] = [];
+  private ranges: RangeInputValue[] = [];
   focusedRangeIndex: number | null = null;
   private inputSheetId: UID;
   private focusStore = this.get(FocusStore);
@@ -166,6 +166,11 @@ export class SelectionInputStore extends SpreadsheetStore {
 
   updateColors(colors: Color[]) {
     this.colors = colors;
+    const colorGenerator = new ColorGenerator(this.ranges.length, this.colors);
+    this.ranges = this.ranges.map((range) => ({
+      ...range,
+      color: colorGenerator.next(),
+    }));
   }
 
   confirm() {
@@ -206,14 +211,13 @@ export class SelectionInputStore extends SpreadsheetStore {
    * e.g. ["A1", "Sheet2!B3", "E12"]
    */
   get selectionInputs(): (RangeInputValue & { isFocused: boolean; isValidRange: boolean })[] {
-    const generator = new ColorGenerator(this.ranges.length, this.colors);
     return this.ranges.map((input, index) =>
       Object.assign({}, input, {
         color:
           this.hasMainFocus &&
           this.focusedRangeIndex !== null &&
           this.getters.isRangeValid(input.xc)
-            ? generator.next()
+            ? input.color
             : null,
         isFocused: this.hasMainFocus && this.focusedRangeIndex === index,
         isValidRange: input.xc === "" || this.getters.isRangeValid(input.xc),

tests/selection_input/selection_input_component.test.ts

@@ -5,6 +5,7 @@ import { SelectionInput } from "../../src/components/selection_input/selection_i
 import { ColorGenerator, toCartesian, toZone } from "../../src/helpers";
 import { useStoreProvider } from "../../src/store_engine";
 import { ModelStore } from "../../src/stores";
+import { HighlightStore } from "../../src/stores/highlight_store";
 import { Color, SpreadsheetChildEnv } from "../../src/types";
 import {
   activateSheet,
@@ -234,20 +235,26 @@ describe("Selection Input", () => {
     // data series of a chart, as the color of the removed range won't be passed anymore to
     // the colors function in the chart's side panel's props.
     let colors = ["#FF0000", "#00FF00"];
-    await createSelectionInput({
+    const { parent, env } = await createSelectionInput({
       initialRanges: ["A1", "B1"],
       colors: colors,
     });
-    simulateClick(fixture.querySelectorAll("input")[0]);
-    await nextTick();
+    const highlightStore = env.getStore(HighlightStore);
+    await simulateClick(fixture.querySelectorAll("input")[0]);
     expect(fixture.querySelectorAll("input")[0].getAttribute("style")).toBe("color:#FF0000; ");
     expect(fixture.querySelectorAll("input")[1].getAttribute("style")).toBe("color:#00FF00; ");
-    colors[0] = "#0000FF";
-    colors[1] = "#FF00FF";
-    simulateClick(fixture.querySelectorAll("input")[1]);
-    await nextTick();
+    expect(highlightStore.highlights).toMatchObject([
+      { color: "#FF0000", zone: toZone("A1") },
+      { color: "#00FF00", zone: toZone("B1") },
+    ]);
+    parent.colors = ["#0000FF", "#FF00FF"];
+    await simulateClick(fixture.querySelectorAll("input")[1]);
     expect(fixture.querySelectorAll("input")[0].getAttribute("style")).toBe("color:#0000FF; ");
     expect(fixture.querySelectorAll("input")[1].getAttribute("style")).toBe("color:#FF00FF; ");
+    expect(highlightStore.highlights).toMatchObject([
+      { color: "#0000FF", zone: toZone("A1") },
+      { color: "#FF00FF", zone: toZone("B1") },
+    ]);
   });
 
   test("can select full column as unbounded zone by clicking on header", async () => {