🐛(react) fix Select mono selected item update label

When we were updating the label from the options array of the selected item, the field was still showing this old value. Fixes #316
openfun · Apr 24, 2024 · 91c8935 · 91c8935
1 parent 285cf99
commit 91c8935
Show file tree

Hide file tree

Showing 6 changed files with 195 additions and 33 deletions.
diff --git a/.changeset/breezy-phones-repeat.md b/.changeset/breezy-phones-repeat.md
@@ -0,0 +1,5 @@
+---
+"@openfun/cunningham-react": patch
+---
+
+fix Select mono selected item update label
diff --git a/packages/react/src/components/Forms/Select/index.tsx b/packages/react/src/components/Forms/Select/index.tsx
@@ -6,7 +6,7 @@ import { FieldProps } from ":/components/Forms/Field";
 export * from ":/components/Forms/Select/mono";
 export * from ":/components/Forms/Select/multi";
 
-type BaseOption = {
+export type BaseOption = {
   value: string;
   label: string;
   render: () => ReactNode;

diff --git a/packages/react/src/components/Forms/Select/mono-searchable.tsx b/packages/react/src/components/Forms/Select/mono-searchable.tsx
@@ -51,25 +51,17 @@ export const SelectMonoSearchable = forwardRef<SelectHandle, SubProps>(
       downshiftReturn.inputValue,
     ]);
 
-    // When component is controlled, this useEffect will update the local selected item.
+    // Similar to: useKeepSelectedItemInSyncWithOptions ( see docs )
+    // The only difference is that it does not apply when there is an inputFilter. ( See below why )
     useEffect(() => {
+      // If there is an inputFilter, using selectItem will trigger onInputValueChange that will sets inputFilter to
+      // empty, and then ignoring the existing filter and displaying all options.
       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, inputFilter]);
 

diff --git a/packages/react/src/components/Forms/Select/mono-simple.tsx b/packages/react/src/components/Forms/Select/mono-simple.tsx
@@ -1,4 +1,4 @@
-import { useSelect } from "downshift";
+import { useSelect, UseSelectReturnValue } from "downshift";
 import React, {
   forwardRef,
   useEffect,
@@ -11,9 +11,27 @@ import {
   SelectMonoAux,
   SubProps,
 } from ":/components/Forms/Select/mono-common";
-import { SelectHandle } from ":/components/Forms/Select";
+import { Option, SelectHandle, SelectProps } from ":/components/Forms/Select";
 import { SelectedOption } from ":/components/Forms/Select/utils";
 
+/**
+ * Here we ensure that the selected item is always in sync with the options.
+ * Ex: If the selected options changes label we want to reflect that.
+ * @param downshiftReturn
+ * @param props
+ */
+const useKeepSelectedItemInSyncWithOptions = (
+  downshiftReturn: UseSelectReturnValue<Option>,
+  props: Pick<SelectProps, "value" | "options">,
+) => {
+  useEffect(() => {
+    const optionToSelect = props.options.find(
+      (option) => optionToValue(option) === props.value,
+    );
+    downshiftReturn.selectItem(optionToSelect ?? null);
+  }, [props.value, props.options]);
+};
+
 export const SelectMonoSimple = forwardRef<SelectHandle, SubProps>(
   (props, ref) => {
     const downshiftReturn = useSelect({
@@ -22,23 +40,7 @@ export const SelectMonoSimple = forwardRef<SelectHandle, SubProps>(
       itemToString: optionToString,
     });
 
-    // When component is controlled, this useEffect will update the local selected item.
-    useEffect(() => {
-      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]);
+    useKeepSelectedItemInSyncWithOptions(downshiftReturn, props);
 
     const wrapperRef = useRef<HTMLElement>(null);
 

diff --git a/packages/react/src/components/Forms/Select/mono.spec.tsx b/packages/react/src/components/Forms/Select/mono.spec.tsx
@@ -756,7 +756,6 @@ describe("<Select/>", () => {
         expectMenuToBeOpen(menu);
 
         expectOptions(["Paris", "Panama"]);
-
         myOptions.shift();
 
         // Rerender the select with the options mutated
@@ -1017,6 +1016,75 @@ describe("<Select/>", () => {
       await user.click(option);
       expect(searchTerm).toBeUndefined();
     });
+
+    it("updates the selected value label if the option label changes", async () => {
+      const myOptions = [
+        {
+          label: "Paris",
+          value: "paris",
+        },
+        {
+          label: "Panama",
+          value: "panama",
+        },
+        {
+          label: "London",
+          value: "london",
+        },
+      ];
+
+      const Wrapper = ({ options }: { options: Option[] }) => {
+        const [value, setValue] = useState<string | number | undefined>(
+          "paris",
+        );
+        const [onChangeCounts, setOnChangeCounts] = useState(0);
+        return (
+          <CunninghamProvider>
+            <div>
+              <div>Value = {value}|</div>
+              <div>onChangeCounts = {onChangeCounts}|</div>
+              <Select
+                label="City"
+                options={options}
+                value={value}
+                onChange={(e) => {
+                  setValue(e.target.value as string);
+                  setOnChangeCounts(onChangeCounts + 1);
+                }}
+                searchable={true}
+              />
+            </div>
+          </CunninghamProvider>
+        );
+      };
+
+      const { rerender } = render(<Wrapper options={myOptions} />, {
+        wrapper: CunninghamProvider,
+      });
+
+      const input = screen.getByRole("combobox", {
+        name: "City",
+      });
+      expect(input).toHaveValue("Paris");
+      screen.getByText("Value = paris|");
+      screen.getByText("onChangeCounts = 0|");
+
+      rerender(
+        <Wrapper
+          options={[
+            {
+              label: "Paname",
+              value: "paris",
+            },
+            ...myOptions.slice(1),
+          ]}
+        />,
+      );
+
+      await waitFor(() => expect(input).toHaveValue("Paname"));
+      screen.getByText("Value = paris|");
+      screen.getByText("onChangeCounts = 0|");
+    });
   });
 
   describe("Simple", () => {
@@ -1842,6 +1910,72 @@ describe("<Select/>", () => {
       screen.getByText("onChangeCounts = 2|");
     });
 
+    it("updates the selected value label if the option label changes", async () => {
+      const myOptions = [
+        {
+          label: "Paris",
+          value: "paris",
+        },
+        {
+          label: "Panama",
+          value: "panama",
+        },
+        {
+          label: "London",
+          value: "london",
+        },
+      ];
+
+      const Wrapper = ({ options }: { options: Option[] }) => {
+        const [value, setValue] = useState<string | number | undefined>(
+          "paris",
+        );
+        const [onChangeCounts, setOnChangeCounts] = useState(0);
+        return (
+          <CunninghamProvider>
+            <div>
+              <div>Value = {value}|</div>
+              <div>onChangeCounts = {onChangeCounts}|</div>
+              <Select
+                label="City"
+                options={options}
+                value={value}
+                onChange={(e) => {
+                  setValue(e.target.value as string);
+                  setOnChangeCounts(onChangeCounts + 1);
+                }}
+              />
+            </div>
+          </CunninghamProvider>
+        );
+      };
+
+      const { rerender } = render(<Wrapper options={myOptions} />, {
+        wrapper: CunninghamProvider,
+      });
+
+      const valueRendered = document.querySelector(".c__select__inner__value");
+      expect(valueRendered).toHaveTextContent("Paris");
+      screen.getByText("Value = paris|");
+      screen.getByText("onChangeCounts = 0|");
+
+      rerender(
+        <Wrapper
+          options={[
+            {
+              label: "Paname",
+              value: "paris",
+            },
+            ...myOptions.slice(1),
+          ]}
+        />,
+      );
+
+      expect(valueRendered).toHaveTextContent("Paname");
+      screen.getByText("Value = paris|");
+      screen.getByText("onChangeCounts = 0|");
+    });
+
     it("blurs from ref", async () => {
       const ref = createRef<SelectHandle>();
       render(

diff --git a/packages/react/src/components/Forms/Select/mono.stories.tsx b/packages/react/src/components/Forms/Select/mono.stories.tsx
@@ -108,6 +108,35 @@ export const Controlled = () => {
   );
 };
 
+export const ControlledEdit = () => {
+  const [value, setValue] = useState(OPTIONS[0].value);
+  const [options, setOptions] = useState(OPTIONS);
+
+  const edit = () => {
+    setOptions([{ value: "woodbury", label: "EDITTED" }, ...OPTIONS.slice(1)]);
+  };
+
+  return (
+    <div>
+      <div>
+        Value: <span>{value}</span>
+        <Button onClick={edit}>Edit</Button>
+      </div>
+      <Select
+        label="Select a city"
+        options={options}
+        value={value}
+        multi={false}
+        searchable={true}
+        onChange={(e) => {
+          setValue(e.target.value as string);
+        }}
+      />
+      <Button onClick={() => setValue("")}>Reset</Button>
+    </div>
+  );
+};
+
 export const Overflow = {
   render: Template,