[FIX] FindAndReplaceSidePanel: Fix range update

How to reproduce:
- Open the F&R SidePanel.
- Choose the 'Specific Range' Search Option from the dropdown.
- Attempt to pick a large range from the grid.

The component SelectionInput heavily relies on the fact its props are
updated by the parent component after SelectionInput invoked
`props.onSelectionChanged`.

The dataRange stored in the component `FindAndReplacePanel`
was not observed, which means the after `SelectionInput` called
`onSelectionChanged`, the update of the datarange would not update the
props passed to `SelectionInput`. This would cause a desynchronized
state which lead to an infinite loop because one of the conditions in
`SelectionInput.onWillUpdateProps` would be triggered 100% of the time
and the said condition unfortunately updates a store, then forcing a
render(true), hence the infinite loop.

In this revision, we fix the bad state management of `FindAndReplace`
but is seems again that `SelectionInput` is wobbly, so is using it. We
should try to fix its behaviour once and for all.

closes #4406

Task: 3972409
X-original-commit: a445f51
Signed-off-by: Pierre Rousseau (pro) <pro@odoo.com>

src/components/side_panel/find_and_replace/find_and_replace.ts

@@ -1,4 +1,4 @@
-import { Component, onMounted, useRef } from "@odoo/owl";
+import { Component, onMounted, useRef, useState } from "@odoo/owl";
 import { zoneToXc } from "../../../helpers";
 import { Store, useLocalStore } from "../../../store_engine";
 import { _t } from "../../../translation";
@@ -47,10 +47,9 @@ export class FindAndReplacePanel extends Component<Props, SpreadsheetChildEnv> {
     onCloseSidePanel: Function,
   };
 
-  private dataRange: string = "";
   private searchInput = useRef("searchInput");
-
   private store!: Store<FindAndReplaceStore>;
+  private state!: { dataRange: string };
 
   get hasSearchResult() {
     return this.store.selectedMatchIndex !== null;
@@ -86,6 +85,7 @@ export class FindAndReplacePanel extends Component<Props, SpreadsheetChildEnv> {
 
   setup() {
     this.store = useLocalStore(FindAndReplaceStore);
+    this.state = useState({ dataRange: "" });
     onMounted(() => this.searchInput.el?.focus());
   }
 
@@ -131,16 +131,16 @@ export class FindAndReplacePanel extends Component<Props, SpreadsheetChildEnv> {
   }
 
   onSearchRangeChanged(ranges: string[]) {
-    this.dataRange = ranges[0];
+    this.state.dataRange = ranges[0];
   }
 
   updateDataRange() {
-    if (!this.dataRange || this.searchOptions.searchScope !== "specificRange") {
+    if (!this.state.dataRange || this.searchOptions.searchScope !== "specificRange") {
       return;
     }
     const specificRange = this.env.model.getters.getRangeFromSheetXC(
       this.env.model.getters.getActiveSheetId(),
-      this.dataRange
+      this.state.dataRange
     );
     this.store.updateSearchOptions({ specificRange });
   }

src/components/side_panel/find_and_replace/find_and_replace.xml

@@ -30,7 +30,7 @@
         </select>
         <div t-if="searchOptions.searchScope === 'specificRange'">
           <SelectionInput
-            ranges="[this.dataRange]"
+            ranges="[this.state.dataRange]"
             onSelectionChanged="(ranges) => this.onSearchRangeChanged(ranges)"
             onSelectionConfirmed.bind="updateDataRange"
             hasSingleRange="true"
@@ -59,7 +59,7 @@
           />
         </div>
         <div class="o-matches-count mt-4" t-if="store.toSearch !== ''">
-          <div t-if="searchOptions.searchScope === 'specificRange' and dataRange != ''">
+          <div t-if="searchOptions.searchScope === 'specificRange' and state.dataRange != ''">
             <t t-esc="specificRangeMatchesCount"/>
           </div>
           <div>

tests/find_and_replace/find_replace_side_panel_component.test.ts

@@ -1,11 +1,13 @@
 import { Model, Spreadsheet } from "../../src";
+import { DEFAULT_CELL_HEIGHT, DEFAULT_CELL_WIDTH } from "../../src/constants";
 import { SearchOptions } from "../../src/types/find_and_replace";
 import { activateSheet, createSheet, setCellContent } from "../test_helpers/commands_helpers";
 import {
   click,
   focusAndKeyDown,
   setInputValueAndTrigger,
   simulateClick,
+  triggerMouseEvent,
 } from "../test_helpers/dom_helper";
 import { getCell, getCellContent } from "../test_helpers/getters_helpers";
 import { mountSpreadsheet, nextTick } from "../test_helpers/helpers";
@@ -116,6 +118,7 @@ describe("find and replace sidePanel component", () => {
       ).toBe(true);
     });
   });
+
   describe("basic search", () => {
     test("simple search", async () => {
       setCellContent(model, "A1", "1");
@@ -185,6 +188,35 @@ describe("find and replace sidePanel component", () => {
       expect(getMatchesCount()).toMatchObject({ specificRange: 1 });
     });
 
+    test("Ranges are properly updated by with several grid selections", async () => {
+      setCellContent(model, "A1", "1");
+      inputSearchValue("1");
+      changeSearchScope("specificRange");
+      await nextTick();
+      await nextTick();
+      (fixture.querySelector(selectors.searchRange) as HTMLInputElement).focus();
+      triggerMouseEvent(
+        ".o-grid-overlay",
+        "pointerdown",
+        DEFAULT_CELL_WIDTH / 2,
+        DEFAULT_CELL_HEIGHT / 2
+      );
+      // let at least a rendering to mimic a realistic behaviour. The compopents could render between the events
+      await nextTick();
+      triggerMouseEvent(
+        ".o-grid-overlay",
+        "pointermove",
+        DEFAULT_CELL_WIDTH / 2,
+        (3 / 2) * DEFAULT_CELL_HEIGHT
+      );
+      await nextTick();
+
+      expect(getMatchesCount()).toMatchObject({ specificRange: 0 });
+
+      await click(fixture, selectors.confirmSearchRange);
+      expect(getMatchesCount()).toMatchObject({ specificRange: 1 });
+    });
+
     test.skip("Specific range is following the active sheet", async () => {
       //TODO : uh ? Is this test wrong ? The specific range doesn't follow the active sheet in master...
       createSheet(model, { sheetId: "sh2" });