[IMP] NumberInput: Add a small debounce on repetitive changes

Currently, changing the value of a numberInput will apply the change
instantly, including when we increment/ decrement the value of the input
using the arrow keys of the keyboard.

This revision adds a small debounce on repetitive calls to limit the
calls to "onChange".

closes #7023

Task: 4756754
Signed-off-by: Lucas Lefèvre (lul) <[email protected]>

Author: rrahir

packages/o-spreadsheet-engine/src/helpers/misc.ts

@@ -261,8 +261,17 @@ export function isObjectEmptyRecursive<T extends object>(argument: T | undefined
 /**
  * Returns a function, that, as long as it continues to be invoked, will not
  * be triggered. The function will be called after it stops being called for
- * N milliseconds. If `immediate` is passed, trigger the function on the
- * leading edge, instead of the trailing.
+ * N milliseconds. If `immediate` is passed,  the function is called is called
+ * immediately on the first call and the debouncing is triggered starting the second
+ * call in the defined time window.
+ *
+ * Example:
+ * debouncedFunction = debounce(() => console.log('Hello!'), 250);
+ * debouncedFunction(); debouncedFunction(); // Will log 'Hello!' after 250ms
+ *
+ * debouncedFunction = debounce(() => console.log('Hello!'), 250, true);
+ * debouncedFunction(); debouncedFunction(); // Will log 'Hello!' and relog it after 250ms
+ *
  *
  * Also decorate the argument function with two methods: stopDebounce and isDebouncePending.
  *
@@ -274,21 +283,23 @@ export function debounce<T extends (...args: any) => void>(
   immediate?: boolean
 ): DebouncedFunction<T> {
   let timeout: any | undefined = undefined;
+  let firstCalled = false;
   const debounced = function (this: any): void {
     const context = this;
     const args = Array.from(arguments);
+    if (!firstCalled && immediate) {
+      firstCalled = true;
+      return func.apply(context, args);
+    }
+
     function later() {
       timeout = undefined;
-      if (!immediate) {
-        func.apply(context, args);
-      }
+      firstCalled = false;
+      func.apply(context, args);
     }
-    const callNow = immediate && !timeout;
+
     clearTimeout(timeout);
     timeout = setTimeout(later, wait);
-    if (callNow) {
-      func.apply(context, args);
-    }
   };
   debounced.isDebouncePending = () => timeout !== undefined;
   debounced.stopDebounce = () => {

src/components/generic_input/generic_input.ts

@@ -26,7 +26,7 @@ export class GenericInput<T extends GenericInputProps> extends Component<T, Spre
   };
 
   protected refName = "input";
-  private inputRef!: Ref<HTMLInputElement>;
+  protected inputRef!: Ref<HTMLInputElement>;
 
   setup() {
     this.inputRef = useRef(this.refName);

src/components/number_input/number_input.ts

@@ -1,3 +1,4 @@
+import { debounce } from "../../helpers";
 import { GenericInput, GenericInputProps } from "../generic_input/generic_input";
 
 interface Props extends GenericInputProps {
@@ -16,6 +17,16 @@ export class NumberInput extends GenericInput<Props> {
     max: { type: Number, optional: true },
   };
 
+  // Very short debounce to prevent up/down arrow on number input to spam the onChange
+  debouncedOnChange = debounce(this.props.onChange.bind(this), 100, true);
+
+  save() {
+    const currentValue = (this.inputRef.el?.value || "").trim();
+    if (currentValue !== this.props.value.toString()) {
+      this.debouncedOnChange(currentValue);
+    }
+  }
+
   get inputClass(): string {
     return [this.props.class, "o-input"].join(" ");
   }

src/components/number_input/number_input.xml

@@ -6,7 +6,7 @@
       t-att-class="inputClass"
       t-att-id="props.id"
       t-att-placeholder="props.placeholder"
-      t-on-change="() => this.save(true)"
+      t-on-change="save"
       t-on-blur="save"
       t-on-pointerdown="onMouseDown"
       t-on-pointerup="onMouseUp"

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

@@ -1,6 +1,6 @@
 import { SpreadsheetChildEnv } from "@odoo/o-spreadsheet-engine/types/spreadsheet_env";
 import { Component } from "@odoo/owl";
-import { clip, debounce } from "../../../../../helpers";
+import { clip } from "../../../../../helpers";
 import { NumberInput } from "../../../../number_input/number_input";
 import { Section } from "../../../components/section/section";
 
@@ -14,9 +14,6 @@ export class PieHoleSize extends Component<Props, SpreadsheetChildEnv> {
   static components = { Section, NumberInput };
   static props = { onValueChange: Function, value: Number };
 
-  // Very short debounce to prevent up/down arrow on number input to spam the onChange
-  debouncedOnChange = debounce(this.onChange.bind(this), 100);
-
   onChange(value: string) {
     if (!isNaN(Number(value))) {
       this.props.onValueChange(clip(Number(value), 0, 95));

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

@@ -7,7 +7,7 @@
           class="'o-pie-hole-size-input'"
           min="0"
           max="95"
-          onChange.bind="debouncedOnChange"
+          onChange.bind="onChange"
         />
         %
       </div>

tests/components/number_input.test.ts

@@ -28,6 +28,13 @@ async function mountNumberInput(props: Props) {
 }
 
 describe("NumberInput", () => {
+  beforeAll(() => {
+    jest.useFakeTimers();
+  });
+
+  afterAll(() => {
+    jest.useRealTimers();
+  });
   test("Can render a number input", async () => {
     await mountNumberInput({ value: 5, onChange: () => {} });
     expect(fixture).toMatchSnapshot();
@@ -50,6 +57,7 @@ describe("NumberInput", () => {
     await mountNumberInput({ value: 5, onChange });
     setInputValueAndTrigger(fixture.querySelector("input")!, "2");
     await click(document.body);
+    jest.advanceTimersByTime(100);
     expect(onChange).toHaveBeenCalledWith("2");
   });
 
@@ -66,6 +74,7 @@ describe("NumberInput", () => {
     fixture.querySelector("input")!.focus();
     setInputValueAndTrigger(fixture.querySelector("input")!, "4");
     await keyDown({ key: "Enter" });
+    jest.advanceTimersByTime(100);
     expect(onChange).toHaveBeenCalledWith("4");
   });
 
@@ -82,6 +91,7 @@ describe("NumberInput", () => {
     await mountNumberInput({ value: 5, onChange });
     setInputValueAndTrigger(fixture.querySelector("input")!, "2");
     fixture.querySelector("input")!.blur();
+    jest.advanceTimersByTime(100);
     expect(onChange).toHaveBeenCalledWith("2");
   });
 

tests/figures/chart/sunburst/sunburst_panel_component.test.ts

@@ -174,16 +174,18 @@ describe("Sunburst chart side panel", () => {
     });
   });
 
-  test("Can change sunburst chart hole size, and input is debounced", async () => {
+  test("Can change sunburst chart hole size, and input is debounced on ,ultiple calls", async () => {
     const chartId = createSunburstChart(model, {});
     await openChartDesignSidePanel(model, env, fixture, chartId);
 
     expect(".o-pie-hole-size-input").toHaveValue("25");
     jest.useFakeTimers();
-    await setInputValueAndTrigger(".o-pie-hole-size-input", "50");
-    expect(getSunburstDefinition(chartId).pieHolePercentage).toEqual(undefined); // debounced
+    setInputValueAndTrigger(".o-pie-hole-size-input", "50");
+    setInputValueAndTrigger(".o-pie-hole-size-input", "51");
+    setInputValueAndTrigger(".o-pie-hole-size-input", "52");
+    expect(getSunburstDefinition(chartId).pieHolePercentage).toEqual(50); // debounced
     jest.advanceTimersByTime(1000);
-    expect(getSunburstDefinition(chartId).pieHolePercentage).toEqual(50);
+    expect(getSunburstDefinition(chartId).pieHolePercentage).toEqual(52);
     jest.useRealTimers();
   });
 });

tests/helpers/debounce_helper.test.ts

@@ -0,0 +1,43 @@
+import { debounce } from "../../src/helpers";
+
+describe("Debounce Helper", () => {
+  beforeAll(() => {
+    jest.useFakeTimers();
+  });
+
+  afterAll(() => {
+    jest.useRealTimers();
+  });
+
+  test("debounce calls function after wait time", () => {
+    const func = jest.fn();
+    const debouncedFunc = debounce(func, 200);
+
+    debouncedFunc();
+    debouncedFunc();
+    debouncedFunc();
+    debouncedFunc();
+    expect(func).not.toBeCalled();
+    jest.advanceTimersByTime(100);
+    expect(func).not.toBeCalled();
+    jest.advanceTimersByTime(100);
+    expect(func).toBeCalledTimes(1);
+  });
+
+  test("debounce with immediate=true calls function immediately and then after wait time", () => {
+    const func = jest.fn();
+    const debouncedFunc = debounce(func, 200, true);
+
+    debouncedFunc();
+    expect(func).toBeCalledTimes(1);
+    debouncedFunc();
+    debouncedFunc();
+    expect(func).toBeCalledTimes(1);
+    jest.advanceTimersByTime(200);
+    expect(func).toBeCalledTimes(2);
+    debouncedFunc();
+    expect(func).toBeCalledTimes(3);
+    jest.advanceTimersByTime(200);
+    expect(func).toBeCalledTimes(3);
+  });
+});