From fd988c03e110ef7f1c59f2ec68e630a5b5dd5fca Mon Sep 17 00:00:00 2001
From: Nathan Vasse <nathan.vasse@gmail.com>
Date: Fri, 15 Sep 2023 11:00:44 +0200
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(react)=20fix=20controlled=20search?=
 =?UTF-8?q?able=20select?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Changing the controlled value was setting triggering immediately an
onChange event with undefined value. This was due to the fact that when
the controlled value was changed SelectMonoAux was searching only in
options displayed, where it should in reality be searching accross all
options.

fixes #162
---
 .changeset/tasty-hotels-hunt.md                    |  5 +++++
 .../src/components/Forms/Select/mono-common.tsx    | 13 +------------
 .../components/Forms/Select/mono-searchable.tsx    | 12 ++++++++++++
 .../src/components/Forms/Select/mono-simple.tsx    | 14 +++++++++++++-
 .../src/components/Forms/Select/mono.spec.tsx      | 10 ++++++++++
 .../src/components/Forms/Select/mono.stories.tsx   | 12 ++++++++++++
 6 files changed, 53 insertions(+), 13 deletions(-)
 create mode 100644 .changeset/tasty-hotels-hunt.md

diff --git a/.changeset/tasty-hotels-hunt.md b/.changeset/tasty-hotels-hunt.md
new file mode 100644
index 0000000..0c9bcc3
--- /dev/null
+++ b/.changeset/tasty-hotels-hunt.md
@@ -0,0 +1,5 @@
+---
+"@openfun/cunningham-react": patch
+---
+
+fix controlled searchable select triggering onChange undefined
diff --git a/packages/react/src/components/Forms/Select/mono-common.tsx b/packages/react/src/components/Forms/Select/mono-common.tsx
index 0f15a6e..f15bf9b 100644
--- a/packages/react/src/components/Forms/Select/mono-common.tsx
+++ b/packages/react/src/components/Forms/Select/mono-common.tsx
@@ -1,4 +1,4 @@
-import React, { HTMLAttributes, useEffect } from "react";
+import React, { HTMLAttributes } from "react";
 import { UseSelectReturnValue } from "downshift";
 import classNames from "classnames";
 import { useCunningham } from ":/components/Provider";
@@ -73,17 +73,6 @@ export const SelectMonoAux = ({
   const { t } = useCunningham();
   const labelProps = downshiftReturn.getLabelProps();
 
-  // When component is controlled, this useEffect will update the local selected item.
-  useEffect(() => {
-    if (downshiftProps.initialSelectedItem !== undefined) {
-      return;
-    }
-    const optionToSelect = options.find(
-      (option) => optionToValue(option) === value,
-    );
-    downshiftReturn.selectItem(optionToSelect ?? null);
-  }, [value]);
-
   return (
     <Field state={state} {...props}>
       <div
diff --git a/packages/react/src/components/Forms/Select/mono-searchable.tsx b/packages/react/src/components/Forms/Select/mono-searchable.tsx
index 5223858..e419737 100644
--- a/packages/react/src/components/Forms/Select/mono-searchable.tsx
+++ b/packages/react/src/components/Forms/Select/mono-searchable.tsx
@@ -4,6 +4,7 @@ import { useCunningham } from ":/components/Provider";
 import {
   getOptionsFilter,
   optionToString,
+  optionToValue,
   SelectMonoAux,
   SubProps,
 } from ":/components/Forms/Select/mono-common";
@@ -40,6 +41,17 @@ export const SelectMonoSearchable = (props: SubProps) => {
     downshiftReturn.inputValue,
   ]);
 
+  // When component is controlled, this useEffect will update the local selected item.
+  useEffect(() => {
+    if (props.downshiftProps.initialSelectedItem !== undefined) {
+      return;
+    }
+    const optionToSelect = props.options.find(
+      (option) => optionToValue(option) === props.value,
+    );
+    downshiftReturn.selectItem(optionToSelect ?? null);
+  }, [props.value, props.options, props.downshiftProps]);
+
   const inputProps = downshiftReturn.getInputProps({
     ref: inputRef,
     disabled: props.disabled,
diff --git a/packages/react/src/components/Forms/Select/mono-simple.tsx b/packages/react/src/components/Forms/Select/mono-simple.tsx
index 87c6457..22ed327 100644
--- a/packages/react/src/components/Forms/Select/mono-simple.tsx
+++ b/packages/react/src/components/Forms/Select/mono-simple.tsx
@@ -1,7 +1,8 @@
 import { useSelect } from "downshift";
-import React from "react";
+import React, { useEffect } from "react";
 import {
   optionToString,
+  optionToValue,
   SelectMonoAux,
   SubProps,
 } from ":/components/Forms/Select/mono-common";
@@ -13,6 +14,17 @@ export const SelectMonoSimple = (props: SubProps) => {
     itemToString: optionToString,
   });
 
+  // When component is controlled, this useEffect will update the local selected item.
+  useEffect(() => {
+    if (props.downshiftProps.initialSelectedItem !== undefined) {
+      return;
+    }
+    const optionToSelect = props.options.find(
+      (option) => optionToValue(option) === props.value,
+    );
+    downshiftReturn.selectItem(optionToSelect ?? null);
+  }, [props.value, props.options, props.downshiftProps]);
+
   return (
     <SelectMonoAux
       {...props}
diff --git a/packages/react/src/components/Forms/Select/mono.spec.tsx b/packages/react/src/components/Forms/Select/mono.spec.tsx
index 2be4e60..90902de 100644
--- a/packages/react/src/components/Forms/Select/mono.spec.tsx
+++ b/packages/react/src/components/Forms/Select/mono.spec.tsx
@@ -295,6 +295,7 @@ describe("<Select/>", () => {
             <div>
               <div>Value = {value}|</div>
               <Button onClick={() => setValue(undefined)}>Clear</Button>
+              <Button onClick={() => setValue("paris")}>Set Paris</Button>
               <Select
                 label="City"
                 options={[
@@ -367,6 +368,15 @@ describe("<Select/>", () => {
 
       // Make sure value is cleared.
       screen.getByText("Value = |");
+
+      // Make sure setting value works
+      const buttonParis = screen.getByRole("button", {
+        name: "Set Paris",
+      });
+      await user.click(buttonParis);
+
+      screen.getByText("Value = paris|");
+      expect(input).toHaveValue("Paris");
     });
     it("renders disabled", async () => {
       render(
diff --git a/packages/react/src/components/Forms/Select/mono.stories.tsx b/packages/react/src/components/Forms/Select/mono.stories.tsx
index de072dc..c627e69 100644
--- a/packages/react/src/components/Forms/Select/mono.stories.tsx
+++ b/packages/react/src/components/Forms/Select/mono.stories.tsx
@@ -87,6 +87,12 @@ export const Controlled = () => {
           onChange={(e) => setValue(e.target.value as string)}
         />
         <Button onClick={() => setValue("")}>Reset</Button>
+        <Button onClick={() => setValue(OPTIONS[0].value)}>
+          Set {OPTIONS[0].label}
+        </Button>
+        <Button onClick={() => setValue(OPTIONS[1].value)}>
+          Set {OPTIONS[1].label}
+        </Button>
       </div>
     </CunninghamProvider>
   );
@@ -156,6 +162,12 @@ export const SearchableControlled = () => {
           onChange={(e) => setValue(e.target.value as string)}
         />
         <Button onClick={() => setValue("")}>Reset</Button>
+        <Button onClick={() => setValue(OPTIONS[0].value)}>
+          Set {OPTIONS[0].label}
+        </Button>
+        <Button onClick={() => setValue(OPTIONS[1].value)}>
+          Set {OPTIONS[1].label}
+        </Button>
       </div>
     </CunninghamProvider>
   );