Skip to content

Commit

Permalink
fix: 15758 managed state inside the MultiSelect and rendered when cha…
Browse files Browse the repository at this point in the history
…nge occurs (#15818)

* fix: managed state in the component and rendered when change occurs

* fix: writing the testcase for controlled items

---------

Co-authored-by: TJ Egan <[email protected]>
  • Loading branch information
mHuzefa and tw15egan authored Mar 11, 2024
1 parent 030c78e commit 7190ce8
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,34 @@ describe('MultiSelect', () => {
).toBeInstanceOf(HTMLElement);
});

it('should trigger onChange with selected items', async () => {
let selectedItems = [];
const testFunction = jest.fn((e) => (selectedItems = e?.selectedItems));
const items = generateItems(4, generateGenericItem);
const label = 'test-label';
const { container } = render(
<MultiSelect
id="custom-id"
onChange={testFunction}
selectedItems={selectedItems}
label={label}
items={items}
/>
);

// eslint-disable-next-line testing-library/prefer-screen-queries
const labelNode = getByText(container, label);
await userEvent.click(labelNode);

const [item] = items;
// eslint-disable-next-line testing-library/prefer-screen-queries
const itemNode = getByText(container, item.label);

await userEvent.click(itemNode);
// Assert that the onChange callback returned the selected items and assigned it to selectedItems
expect(testFunction.mock.results[0].value).toEqual(selectedItems);
});

it('should place the given id on the ___ node when passed in as a prop', () => {
const items = generateItems(4, generateGenericItem);
const label = 'test-label';
Expand Down
42 changes: 19 additions & 23 deletions packages/react/src/internal/Selection.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,42 +36,39 @@ export function useSelection({
const [uncontrolledItems, setUncontrolledItems] =
useState(initialSelectedItems);
const isControlled = !!controlledItems;
const selectedItems = isControlled ? controlledItems : uncontrolledItems;
const [selectedItems, setSelectedItems] = useState(
isControlled ? controlledItems : uncontrolledItems
);
useEffect(() => {
callOnChangeHandler({
isControlled,
isMounted: isMounted.current,
onChangeHandlerControlled: savedOnChange.current,
onChangeHandlerUncontrolled: setUncontrolledItems,
selectedItems: selectedItems,
});
}, [isControlled, isMounted, selectedItems]);

const onItemChange = useCallback(
(item) => {
if (disabled) {
return;
}

let selectedIndex;
selectedItems.forEach((selectedItem, index) => {
if (isEqual(selectedItem, item)) {
selectedIndex = index;
}
});
let newSelectedItems;
if (selectedIndex === undefined) {
newSelectedItems = selectedItems.concat(item);
callOnChangeHandler({
isControlled,
isMounted: isMounted.current,
onChangeHandlerControlled: savedOnChange.current,
onChangeHandlerUncontrolled: setUncontrolledItems,
selectedItems: newSelectedItems,
});
setSelectedItems((selectedItems) => selectedItems.concat(item));
return;
}

newSelectedItems = removeAtIndex(selectedItems, selectedIndex);
callOnChangeHandler({
isControlled,
isMounted: isMounted.current,
onChangeHandlerControlled: savedOnChange.current,
onChangeHandlerUncontrolled: setUncontrolledItems,
selectedItems: newSelectedItems,
});
setSelectedItems((selectedItems) =>
removeAtIndex(selectedItems, selectedIndex)
);
},
[disabled, isControlled, selectedItems]
[disabled, selectedItems]
);

const clearSelection = useCallback(() => {
Expand All @@ -83,7 +80,7 @@ export function useSelection({
isMounted: isMounted.current,
onChangeHandlerControlled: savedOnChange.current,
onChangeHandlerUncontrolled: setUncontrolledItems,
selectedItems: [],
selectedItems: setSelectedItems([]),
});
}, [disabled, isControlled]);

Expand Down Expand Up @@ -186,7 +183,6 @@ export default class Selection extends React.Component {
onItemChange: this.handleOnItemChange,
clearSelection: this.handleClearSelection,
};

if (render !== undefined) {
return render(renderProps);
}
Expand Down

0 comments on commit 7190ce8

Please sign in to comment.