Merge pull request #7136 from Sage/FE-6901

fix(filterable-select): cannot backspace with object as values

src/components/select/__internal__/utils/are-objects-equal.test.tsx

@@ -0,0 +1,29 @@
+import areObjectsEqual from "./are-objects-equal";
+
+const value1 = { id: "Amber", value: 1, text: "Amber" };
+const value2 = { id: "Amber", value: 1, text: "Amber" };
+const value3 = { id: "Black", value: 2, text: "Black" };
+const value4 = { id: "Black", value: 2 };
+const value5 = { ids: "Amber", values: 1, text: "Amber" };
+const invalidParam = "notAnObject" as unknown as Record<string, unknown>;
+
+test("returns true when the objects have the same keys and values", () => {
+  expect(areObjectsEqual(value1, value2)).toBe(true);
+});
+
+test("returns false when the objects have the same keys but not the same values", () => {
+  expect(areObjectsEqual(value1, value3)).toBe(false);
+});
+
+test("returns false when the objects do not have the same keys length", () => {
+  expect(areObjectsEqual(value1, value4)).toBe(false);
+});
+
+test("returns false when the objects have the same keys length but different key names", () => {
+  expect(areObjectsEqual(value1, value5)).toBe(false);
+});
+
+test("throws error if both parameters are not objects", () => {
+  expect(areObjectsEqual(value1, invalidParam)).toBe(false);
+  expect(areObjectsEqual(invalidParam, value1)).toBe(false);
+});

src/components/select/__internal__/utils/are-objects-equal.tsx

@@ -0,0 +1,32 @@
+export default function areObjectsEqual(
+  obj1: Record<string, unknown>,
+  obj2: Record<string, unknown>,
+) {
+  // Check if both arguments are objects
+  if (
+    typeof obj1 !== "object" ||
+    typeof obj2 !== "object" ||
+    obj1 === null ||
+    obj2 === null
+  ) {
+    return false;
+  }
+  // Get the keys of both objects
+  const keys1 = Object.keys(obj1);
+  const keys2 = Object.keys(obj2);
+  // If the number of keys is different, objects aren't equal
+  if (keys1.length !== keys2.length) {
+    return false;
+  } // Compare keys and values
+  for (const key of keys1) {
+    // Check if obj2 has the key and values are strictly equal
+    if (
+      !Object.prototype.hasOwnProperty.call(obj2, key) ||
+      obj1[key] !== obj2[key]
+    ) {
+      return false;
+    }
+  }
+
+  return true;
+}

src/components/select/filterable-select/filterable-select.component.tsx

@@ -14,6 +14,7 @@ import SelectList, {
   ListPlacement,
 } from "../__internal__/select-list/select-list.component";
 import isExpectedOption from "../__internal__/utils/is-expected-option";
+import areObjectsEqual from "../__internal__/utils/are-objects-equal";
 import isNavigationKey from "../__internal__/utils/is-navigation-key";
 import Logger from "../../../__internal__/utils/logger";
 import useStableCallback from "../../../hooks/__internal__/useStableCallback";
@@ -160,6 +161,7 @@ export const FilterableSelect = React.forwardRef<
     const [selectedValue, setSelectedValue] = useState<
       string | Record<string, unknown> | undefined
     >(value || defaultValue || "");
+    const receivedValue = useRef(value);
     const [highlightedValue, setHighlightedValue] = useState<
       string | Record<string, unknown> | undefined
     >("");
@@ -427,9 +429,27 @@ export const FilterableSelect = React.forwardRef<
       );
     }, [listActionButton, onListAction]);
 
+    const isFirstRender = useRef(true);
+
     useEffect(() => {
+      // when we render for the first time, we run setMatchingText to populate the input with the correct text
+      if (isFirstRender.current) {
+        setMatchingText(selectedValue);
+      }
+
       if (isControlled.current) {
-        setMatchingText(value);
+        // when value is an object we should only run setMatchingText if the object changes between renders
+        if (
+          typeof value === "object" &&
+          typeof receivedValue.current === "object"
+        ) {
+          if (!areObjectsEqual(value, receivedValue.current)) {
+            setMatchingText(value);
+            receivedValue.current = value;
+          }
+        } else {
+          setMatchingText(value);
+        }
       }
       // update text value only when children are changing
       // eslint-disable-next-line react-hooks/exhaustive-deps
@@ -439,8 +459,6 @@ export const FilterableSelect = React.forwardRef<
       onFilterChangeProp as (filterTextArg: unknown) => void,
     );
 
-    const isFirstRender = useRef(true);
-
     useEffect(() => {
       if (onFilterChange && !isFirstRender.current) {
         onFilterChange(filterText);

src/components/select/filterable-select/filterable-select.test.tsx

@@ -54,6 +54,60 @@ const FilterableSelectWithState = ({
   );
 };
 
+const FilterableSelectWithStateAndObjects = ({
+  label,
+  ...props
+}: Partial<FilterableSelectProps>) => {
+  const optionListValues = [
+    { id: "Black", value: 1, text: "Black" },
+    { id: "Blue", value: 2, text: "Blue" },
+  ];
+
+  const [value, setValue] = useState<Record<string, unknown>>(
+    optionListValues[1],
+  );
+
+  function onChangeHandler(event: React.ChangeEvent<HTMLInputElement>) {
+    setValue(event.target.value as unknown as Record<string, unknown>);
+  }
+  return (
+    <FilterableSelect
+      label={label}
+      value={value}
+      onChange={onChangeHandler}
+      {...props}
+    >
+      {optionListValues.map((option) => (
+        <Option key={option.id} text={option.text} value={option} />
+      ))}
+    </FilterableSelect>
+  );
+};
+
+const FilterableSelectWithDefaultValueStateAndObjects = ({
+  label,
+  defaultValue,
+  ...props
+}: Partial<FilterableSelectProps>) => {
+  const optionListValues = [
+    { id: "Black", value: 1, text: "Black" },
+    { id: "Blue", value: 2, text: "Blue" },
+  ];
+
+  return (
+    <FilterableSelect
+      label={label}
+      defaultValue={defaultValue}
+      onChange={() => {}}
+      {...props}
+    >
+      {optionListValues.map((option) => (
+        <Option key={option.id} text={option.text} value={option} />
+      ))}
+    </FilterableSelect>
+  );
+};
+
 test("should display a deprecation warning only once for all instances of component when they are uncontrolled", () => {
   const loggerSpy = jest.spyOn(Logger, "deprecate");
   render(
@@ -183,6 +237,16 @@ test("should initially render combobox with custom `placeholder` when prop is pa
   );
 });
 
+test("should initially render default value text when prop is passed", () => {
+  render(
+    <FilterableSelectWithDefaultValueStateAndObjects
+      defaultValue={{ id: "Blue", value: 2, text: "Blue" }}
+    />,
+  );
+
+  expect(screen.getByRole("combobox")).toHaveValue("Blue");
+});
+
 test("should render combobox with correct accessible name when `label` prop is provided", () => {
   render(
     <FilterableSelect label="Colour" onChange={() => {}} value="">
@@ -699,6 +763,25 @@ describe("when the input is focused", () => {
     expect(screen.getByRole("combobox")).toHaveValue("red");
   });
 
+  it("should delete the last character with backspace and the correct match text is provided, if the values are objects", async () => {
+    const user = userEvent.setup();
+
+    render(<FilterableSelectWithStateAndObjects />);
+
+    const input = screen.getByRole("combobox");
+
+    expect(input).toHaveValue("Blue");
+
+    await user.click(input);
+    await user.click(input);
+
+    await user.type(input, "{Backspace}");
+    expect(input).toHaveValue("Blu");
+
+    await user.type(input, "{Backspace}");
+    expect(input).toHaveValue("Black");
+  });
+
   it("should display the list but not update the input value when the user types a character that does not match an option", async () => {
     const user = userEvent.setup();
     render(