From 3fc464bb8ce6e5578330d0c07e98474e6a149bf9 Mon Sep 17 00:00:00 2001
From: Anthony Le Courric <freelance.anthony@gmail.com>
Date: Mon, 25 Sep 2023 16:03:54 +0200
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(react)=20fix=20some=20edge=20cases?=
 =?UTF-8?q?=20with=20searchable=20Select?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- rerender mutated options when the menu is opened
- keep the filter value when the menu is opened and rerender
by a mutated options
---
 .changeset/tall-bats-wink.md                  |   5 +
 .../Forms/Select/mono-searchable.tsx          |  29 ++++-
 .../components/Forms/Select/mono-simple.tsx   |  15 ++-
 .../src/components/Forms/Select/mono.spec.tsx | 106 +++++++++++++++++-
 .../src/components/Forms/Select/mono.tsx      |  31 ++++-
 5 files changed, 171 insertions(+), 15 deletions(-)
 create mode 100644 .changeset/tall-bats-wink.md

diff --git a/.changeset/tall-bats-wink.md b/.changeset/tall-bats-wink.md
new file mode 100644
index 0000000..d321175
--- /dev/null
+++ b/.changeset/tall-bats-wink.md
@@ -0,0 +1,5 @@
+---
+"@openfun/cunningham-react": minor
+---
+
+fix some edge effects on the mono select component
diff --git a/packages/react/src/components/Forms/Select/mono-searchable.tsx b/packages/react/src/components/Forms/Select/mono-searchable.tsx
index 7db2206..865b550 100644
--- a/packages/react/src/components/Forms/Select/mono-searchable.tsx
+++ b/packages/react/src/components/Forms/Select/mono-searchable.tsx
@@ -13,19 +13,19 @@ export const SelectMonoSearchable = (props: SubProps) => {
   const { t } = useCunningham();
   const [optionsToDisplay, setOptionsToDisplay] = useState(props.options);
   const [hasInputFocused, setHasInputFocused] = useState(false);
+  const [inputFilter, setInputFilter] = useState<string>();
   const inputRef = useRef<HTMLInputElement>(null);
   const downshiftReturn = useCombobox({
     ...props.downshiftProps,
     items: optionsToDisplay,
     itemToString: optionToString,
     onInputValueChange: (e) => {
-      setOptionsToDisplay(props.options.filter(getOptionsFilter(e.inputValue)));
+      setInputFilter(e.inputValue);
       if (!e.inputValue) {
         downshiftReturn.selectItem(null);
       }
     },
   });
-
   const [labelAsPlaceholder, setLabelAsPlaceholder] = useState(
     !downshiftReturn.selectedItem,
   );
@@ -43,21 +43,38 @@ export const SelectMonoSearchable = (props: SubProps) => {
 
   // When component is controlled, this useEffect will update the local selected item.
   useEffect(() => {
-    if (props.downshiftProps.initialSelectedItem !== undefined) {
+    if (inputFilter) {
       return;
     }
+
+    const selectedItem = downshiftReturn.selectedItem
+      ? optionToValue(downshiftReturn.selectedItem)
+      : undefined;
+
     const optionToSelect = props.options.find(
       (option) => optionToValue(option) === props.value,
     );
+
+    // Already selected
+    if (optionToSelect && selectedItem === props.value) {
+      return;
+    }
+
     downshiftReturn.selectItem(optionToSelect ?? null);
-  }, [props.value, props.options, props.downshiftProps]);
+  }, [props.value, props.options, inputFilter]);
 
   // Even there is already a value selected, when opening the combobox menu we want to display all available choices.
   useEffect(() => {
     if (downshiftReturn.isOpen) {
-      setOptionsToDisplay(props.options);
+      setOptionsToDisplay(
+        inputFilter
+          ? props.options.filter(getOptionsFilter(inputFilter))
+          : props.options,
+      );
+    } else {
+      setInputFilter(undefined);
     }
-  }, [downshiftReturn.isOpen]);
+  }, [downshiftReturn.isOpen, props.options, inputFilter]);
 
   const onInputBlur = () => {
     setHasInputFocused(false);
diff --git a/packages/react/src/components/Forms/Select/mono-simple.tsx b/packages/react/src/components/Forms/Select/mono-simple.tsx
index 22ed327..65cdbdc 100644
--- a/packages/react/src/components/Forms/Select/mono-simple.tsx
+++ b/packages/react/src/components/Forms/Select/mono-simple.tsx
@@ -16,14 +16,21 @@ export const SelectMonoSimple = (props: SubProps) => {
 
   // When component is controlled, this useEffect will update the local selected item.
   useEffect(() => {
-    if (props.downshiftProps.initialSelectedItem !== undefined) {
-      return;
-    }
+    const selectedItem = downshiftReturn.selectedItem
+      ? optionToValue(downshiftReturn.selectedItem)
+      : undefined;
+
     const optionToSelect = props.options.find(
       (option) => optionToValue(option) === props.value,
     );
+
+    // Already selected
+    if (optionToSelect && selectedItem === props.value) {
+      return;
+    }
+
     downshiftReturn.selectItem(optionToSelect ?? null);
-  }, [props.value, props.options, props.downshiftProps]);
+  }, [props.value, props.options]);
 
   return (
     <SelectMonoAux
diff --git a/packages/react/src/components/Forms/Select/mono.spec.tsx b/packages/react/src/components/Forms/Select/mono.spec.tsx
index 3faee95..7929181 100644
--- a/packages/react/src/components/Forms/Select/mono.spec.tsx
+++ b/packages/react/src/components/Forms/Select/mono.spec.tsx
@@ -2,7 +2,7 @@ import userEvent from "@testing-library/user-event";
 import { render, screen, waitFor } from "@testing-library/react";
 import { expect } from "vitest";
 import React, { FormEvent, useState } from "react";
-import { Select } from ":/components/Forms/Select/index";
+import { Select, Option } from ":/components/Forms/Select/index";
 import { Button } from ":/components/Button";
 import { CunninghamProvider } from ":/components/Provider";
 import {
@@ -683,6 +683,110 @@ describe("<Select/>", () => {
         city: null,
       });
     });
+
+    [
+      {
+        defaultValue: "panama",
+        type: "default value",
+        expected: "Panama",
+      },
+      {
+        value: "panama",
+        type: "value",
+        expected: "Panama",
+      },
+      {
+        type: "without values props",
+        expected: "",
+      },
+    ].forEach(({ type, expected, ...props }) => {
+      it(`render mutated option when select open and keep the filter activated with ${type}`, async () => {
+        const myOptions = [
+          {
+            label: "Paris",
+            value: "paris",
+          },
+          {
+            label: "Panama",
+            value: "panama",
+          },
+          {
+            label: "London",
+            value: "london",
+          },
+        ];
+
+        const Wrapper = ({ options }: { options: Option[] }) => {
+          return (
+            <Select
+              label="City"
+              name="city"
+              options={options}
+              searchable={true}
+              {...props}
+            />
+          );
+        };
+
+        const { rerender } = render(<Wrapper options={myOptions} />, {
+          wrapper: CunninghamProvider,
+        });
+
+        const user = userEvent.setup();
+        const input = screen.getByRole("combobox", {
+          name: "City",
+        });
+        const menu: HTMLDivElement = screen.getByRole("listbox", {
+          name: "City",
+        });
+
+        // Check init value (defaultValue / value / nothing)
+        expect(input).toHaveValue(expected);
+
+        // Add filter
+        await user.clear(input);
+        await user.type(input, "Pa");
+
+        expectMenuToBeOpen(menu);
+
+        expectOptions(["Paris", "Panama"]);
+
+        myOptions.shift();
+
+        // Rerender the select with the options mutated
+        rerender(<Wrapper options={[...myOptions]} />);
+
+        expectMenuToBeOpen(menu);
+
+        // Options is refreshed
+        expectOptions(["Panama"]);
+
+        // Filter is still active
+        expect(input).toHaveValue("Pa");
+
+        myOptions.shift();
+
+        // Rerender the select with the options mutated (only london left)
+        rerender(<Wrapper options={[...myOptions]} />);
+
+        // Filter is still active
+        expect(input).toHaveValue("Pa");
+
+        expect(screen.getByText("No options available")).toBeInTheDocument();
+
+        await user.clear(input);
+
+        expectOptions(["London"]);
+
+        await user.click(
+          screen.getByRole("option", {
+            name: "London",
+          }),
+        );
+
+        expect(input).toHaveValue("London");
+      });
+    });
   });
 
   describe("Simple", () => {
diff --git a/packages/react/src/components/Forms/Select/mono.tsx b/packages/react/src/components/Forms/Select/mono.tsx
index d66ff77..50af340 100644
--- a/packages/react/src/components/Forms/Select/mono.tsx
+++ b/packages/react/src/components/Forms/Select/mono.tsx
@@ -1,4 +1,4 @@
-import React, { PropsWithChildren } from "react";
+import React, { PropsWithChildren, useEffect, useState } from "react";
 import { UseSelectStateChange } from "downshift";
 import { FieldProps } from ":/components/Forms/Field";
 import { optionToValue, SubProps } from ":/components/Forms/Select/mono-common";
@@ -37,17 +37,32 @@ export const SelectMono = (props: SelectProps) => {
         (option) => optionToValue(option) === props.defaultValue,
       )
     : undefined;
+  const [value, setValue] = useState(
+    defaultSelectedItem ? optionToValue(defaultSelectedItem) : props.value,
+  );
+
+  /**
+   * This useEffect is used to update the local value when the component is controlled.
+   * The defaultValue is used only on first render.
+   */
+  useEffect(() => {
+    if (props.defaultValue) {
+      return;
+    }
+    setValue(props.value);
+  }, [props.value, props.defaultValue]);
 
   const commonDownshiftProps: SubProps["downshiftProps"] = {
     initialSelectedItem: defaultSelectedItem,
     onSelectedItemChange: (e: UseSelectStateChange<Option>) => {
       const eventCmp = e.selectedItem ? optionToValue(e.selectedItem) : null;
-      const valueCmp = props.value ?? null;
+      const valueCmp = value ?? null;
       // We make sure to not trigger a onChange event if the value are not different.
       // This could happen on first render when the component is controlled, the value will be
       // set inside a useEffect down in SelectMonoSearchable or SelectMonoSimple. So that means the
       // downshift component will always render empty the first time.
       if (eventCmp !== valueCmp) {
+        setValue(eventCmp || undefined);
         props.onChange?.({
           target: {
             value: e.selectedItem ? optionToValue(e.selectedItem) : undefined,
@@ -59,8 +74,16 @@ export const SelectMono = (props: SelectProps) => {
   };
 
   return props.searchable ? (
-    <SelectMonoSearchable {...props} downshiftProps={commonDownshiftProps} />
+    <SelectMonoSearchable
+      {...props}
+      downshiftProps={commonDownshiftProps}
+      value={value}
+    />
   ) : (
-    <SelectMonoSimple {...props} downshiftProps={commonDownshiftProps} />
+    <SelectMonoSimple
+      {...props}
+      downshiftProps={commonDownshiftProps}
+      value={value}
+    />
   );
 };