Fix MultiSelect component to not set newlySelected incorrectly; also add complementing newlyRemoved

Author: andreas-schultz

CHANGELOG.md

@@ -33,6 +33,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
 -   new icons:
     -   `state-confirmed-all`
     -   `state-declined-all`
+-   `<MultiSuggestField />`
+    -   `MultiSuggestFieldSelectionProps` provides `newlyRemoved` for callbacks set when removing a selected item
 
 ### Fixed
 
@@ -57,6 +59,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
     -   use the latest provided `onChange` function
 -   `<TextField />`, `<TextArea />`
     -   fix emoji false-positives in invisible character detection
+-   `<MultiSuggestField />`
+    -   `onSelection` now sets `newlySelected` only for add actions and no longer sets it to the last element
 
 ### Changed
 

src/components/MultiSelect/MultiSelect.tsx

@@ -22,6 +22,7 @@ import {
 
 export interface MultiSuggestFieldSelectionProps<T> {
     newlySelected?: T;
+    newlyRemoved?: T;
     selectedItems: T[];
     createdItems: Partial<T>[];
 }
@@ -178,6 +179,11 @@ export function MultiSuggestField<T>({
     intent,
     ...otherMultiSelectProps
 }: MultiSuggestFieldProps<T>) {
+    type SelectionChange =
+        | { type: "selected"; item: T }
+        | { type: "removed"; item: T }
+        | { type: "none" };
+
     // Options created by a user
     const createdItems = useRef<T[]>([]);
     // Options passed ouside (f.e. from the backend)
@@ -199,6 +205,7 @@ export function MultiSuggestField<T>({
         query?: string;
         timeoutId?: number;
     }>({});
+    const selectionChange = useRef<SelectionChange>({ type: "none" });
 
     /** Update external items when they change
      *  e.g for auto-complete when query change
@@ -209,11 +216,21 @@ export function MultiSuggestField<T>({
     }, [items.map((item) => itemId(item)).join("|")]);
 
     React.useEffect(() => {
-        onSelection?.({
-            newlySelected: selectedItems.slice(-1)[0],
+        const selectionParams: MultiSuggestFieldSelectionProps<T> = {
             createdItems: createdItems.current,
             selectedItems,
-        });
+        };
+
+        if (selectionChange.current.type === "selected") {
+            selectionParams.newlySelected = selectionChange.current.item;
+        }
+
+        if (selectionChange.current.type === "removed") {
+            selectionParams.newlyRemoved = selectionChange.current.item;
+        }
+
+        onSelection?.(selectionParams);
+        selectionChange.current = { type: "none" };
     }, [
         onSelection,
         selectedItems.map((item) => itemId(item)).join("|"),
@@ -228,6 +245,7 @@ export function MultiSuggestField<T>({
             return;
         }
 
+        selectionChange.current = { type: "none" };
         setSelectedItems(externalSelectedItems);
     }, [externalSelectedItems?.map((item) => itemId(item)).join("|")]);
 
@@ -268,8 +286,13 @@ export function MultiSuggestField<T>({
      * @param matcher
      */
     const removeItemSelection = (matcher: string) => {
-        const filteredItems = selectedItems.filter((item) => itemId(item) !== matcher);
-        setSelectedItems(filteredItems);
+        setSelectedItems((items) => {
+            const removedItem = items.find((item) => itemId(item) === matcher);
+
+            selectionChange.current = removedItem ? { type: "removed", item: removedItem } : { type: "none" };
+
+            return items.filter((item) => itemId(item) !== matcher);
+        });
     };
 
     const defaultFilterPredicate = (item: T, query: string) => {
@@ -286,6 +309,7 @@ export function MultiSuggestField<T>({
         if (itemHasBeenSelectedAlready(itemId(item))) {
             removeItemSelection(itemId(item));
         } else {
+            selectionChange.current = { type: "selected", item };
             setSelectedItems((items) => [...items, item]);
         }
 
@@ -365,6 +389,7 @@ export function MultiSuggestField<T>({
     const handleClear = () => {
         requestState.current.query = "";
 
+        selectionChange.current = { type: "none" };
         setSelectedItems([]);
         setFilteredItems([...externalItems, ...createdItems.current]);
     };
@@ -375,7 +400,13 @@ export function MultiSuggestField<T>({
      * @param index
      */
     const removeTagFromSelectionViaIndex = (_label: React.ReactNode, index: number) => {
-        setSelectedItems([...selectedItems.slice(0, index), ...selectedItems.slice(index + 1)]);
+        setSelectedItems((items) => {
+            const removedItem = items[index];
+
+            selectionChange.current = removedItem ? { type: "removed", item: removedItem } : { type: "none" };
+
+            return [...items.slice(0, index), ...items.slice(index + 1)];
+        });
     };
 
     /**

src/components/MultiSuggestField/tests/MultiSuggestField.test.tsx

@@ -261,6 +261,86 @@ describe("MultiSuggestField", () => {
             });
         });
 
+        it("should set newlySelected only when an item is added", async () => {
+            const onSelection = jest.fn();
+            const initiallySelected = predefinedNotControlledValues.args.selectedItems;
+
+            const { container } = render(
+                <MultiSuggestField
+                    {...dropdownOnFocus.args}
+                    items={items}
+                    selectedItems={initiallySelected}
+                    onSelection={onSelection}
+                />
+            );
+
+            await waitFor(() => {
+                expect(onSelection).toHaveBeenCalledWith({
+                    createdItems: [],
+                    selectedItems: initiallySelected,
+                });
+            });
+
+            onSelection.mockClear();
+
+            const [inputContainer] = container.getElementsByClassName("eccgui-multiselect");
+            const [input] = inputContainer.getElementsByTagName("input");
+
+            fireEvent.click(input);
+
+            await waitFor(() => {
+                const listbox = screen.getByRole("listbox");
+                const menuItems = listbox.getElementsByClassName("eccgui-menu__item");
+
+                expect(menuItems.length).toBe(dropdownOnFocus.args.items.length);
+
+                fireEvent.click(menuItems[2]);
+            });
+
+            await waitFor(() => {
+                expect(onSelection).toHaveBeenLastCalledWith({
+                    createdItems: [],
+                    newlySelected: items[2],
+                    selectedItems: [...initiallySelected, items[2]],
+                });
+            });
+        });
+
+        it("should set newlyRemoved only when an item is removed", async () => {
+            const onSelection = jest.fn();
+            const initiallySelected = predefinedNotControlledValues.args.selectedItems;
+
+            const { container } = render(
+                <MultiSuggestField
+                    {...predefinedNotControlledValues.args}
+                    selectedItems={initiallySelected}
+                    onSelection={onSelection}
+                />
+            );
+
+            await waitFor(() => {
+                expect(onSelection).toHaveBeenCalledWith({
+                    createdItems: [],
+                    selectedItems: initiallySelected,
+                });
+            });
+
+            onSelection.mockClear();
+
+            const [firstTag] = Array.from(container.querySelectorAll("span[data-tag-index]"));
+            const removeTagButton = firstTag.querySelector("button");
+
+            fireEvent.click(removeTagButton!);
+
+            await waitFor(() => {
+                expect(onSelection).toHaveBeenLastCalledWith({
+                    createdItems: [],
+                    newlyRemoved: initiallySelected[0],
+                    selectedItems: initiallySelected.slice(1),
+                });
+            });
+        });
+
         it("should filter items by custom search function", async () => {
             const { container } = render(<MultiSuggestField {...CustomSearch.args} items={items} />);
 
@@ -340,6 +420,13 @@ describe("MultiSuggestField", () => {
                 expect(getByText(firstSelected)).toBeInTheDocument();
                 expect(getByText(secondSelected)).toBeInTheDocument();
             });
+
+            await waitFor(() => {
+                expect(onSelection).toHaveBeenCalledWith({
+                    createdItems: [],
+                    selectedItems: predefinedNotControlledValues.args.selectedItems,
+                });
+            });
         });
 
         it("should call onSelection function with the selected items", async () => {
@@ -485,7 +572,6 @@ describe("MultiSuggestField", () => {
             await waitFor(() => {
                 const expectedObject = {
                     createdItems: [],
-                    newlySelected: items.at(-1),
                     selectedItems: items,
                 };
                 expect(onSelection).toHaveBeenCalledWith(expectedObject);
@@ -513,7 +599,6 @@ describe("MultiSuggestField", () => {
             await waitFor(() => {
                 const expectedObject = {
                     createdItems: [],
-                    newlySelected: items.at(-1),
                     selectedItems: items,
                 };
                 expect(onSelection).toHaveBeenCalledWith(expectedObject);
@@ -536,7 +621,7 @@ describe("MultiSuggestField", () => {
 
                     const expectedObject = {
                         createdItems: [],
-                        newlySelected: selected.at(-1),
+                        newlyRemoved: items[i],
                         selectedItems: selected,
                     };