From 0bc9c7860916cd63cf81c515557a1e9343acd122 Mon Sep 17 00:00:00 2001 From: Yael Chavoya Date: Mon, 6 May 2024 11:33:51 -0600 Subject: [PATCH] fix(multiselect): add useEffect to selection hook to update externally (#16139) * fix(multiselect): add useEffect to selection hook to update externally * chore: add ychavoya to contributors * fix(multiselect): pass value instead of the result of the state setter the setter function of a useState hook returns undefined, this was mistakenly used as the value to be set instead of sending the value directly --------- Co-authored-by: TJ Egan --- .all-contributorsrc | 9 ++++ README.md | 1 + .../MultiSelect/MultiSelect.stories.js | 43 ++++++++++++++++++- .../MultiSelect/__tests__/MultiSelect-test.js | 28 +++++++++++- packages/react/src/internal/Selection.js | 7 ++- 5 files changed, 84 insertions(+), 4 deletions(-) diff --git a/.all-contributorsrc b/.all-contributorsrc index 3d2b75b72faf..498dc98e1004 100644 --- a/.all-contributorsrc +++ b/.all-contributorsrc @@ -1515,6 +1515,15 @@ "contributions": [ "code" ] + }, + { + "login": "ychavoya", + "name": "Yael Chavoya", + "avatar_url": "https://avatars.githubusercontent.com/u/7907338?v=4", + "profile": "https://github.com/ychavoya", + "contributions": [ + "code" + ] } ], "commitConvention": "none" diff --git a/README.md b/README.md index b1ecf063a618..747a79c89137 100644 --- a/README.md +++ b/README.md @@ -289,6 +289,7 @@ check out our [Contributing Guide](/.github/CONTRIBUTING.md) and our
Holly Springsteen
💻
Nikhil Tomar
💻
Anina Antony
💻 +
Yael Chavoya
💻 diff --git a/packages/react/src/components/MultiSelect/MultiSelect.stories.js b/packages/react/src/components/MultiSelect/MultiSelect.stories.js index dcd6b3f52253..e48d90f96d54 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect.stories.js +++ b/packages/react/src/components/MultiSelect/MultiSelect.stories.js @@ -5,13 +5,16 @@ * LICENSE file in the root directory of this source tree. */ -import React from 'react'; +import React, { useState } from 'react'; +import { action } from '@storybook/addon-actions'; import { WithLayer } from '../../../.storybook/templates/WithLayer'; import mdx from './MultiSelect.mdx'; import MultiSelect from '.'; import FilterableMultiSelect from './FilterableMultiSelect'; +import Button from '../Button'; +import ButtonSet from '../ButtonSet'; export default { title: 'Components/MultiSelect', @@ -306,3 +309,41 @@ export const _FilterableWithLayer = () => ( )} ); + +export const _Controlled = () => { + const [selectedItems, setSelectedItems] = useState( + items.filter((item) => item.id === 'downshift-1-item-0') + ); + + const onSelectionChanged = (value) => { + action('changed items')(value); + setSelectedItems(value); + }; + + return ( +
+ onSelectionChanged(data.selectedItems)} + itemToString={(item) => (item ? item.text : '')} + selectionFeedback="top-after-reopen" + /> +
+ + + + +
+ ); +}; diff --git a/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js b/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js index d90f77d8ee68..819d65bacac4 100644 --- a/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js +++ b/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js @@ -6,7 +6,7 @@ */ import { getByText, isElementVisible } from '@carbon/test-utils/dom'; -import { render, screen } from '@testing-library/react'; +import { act, render, screen } from '@testing-library/react'; import React from 'react'; import MultiSelect from '../'; import { generateItems, generateGenericItem } from '../../ListBox/test-helpers'; @@ -455,7 +455,7 @@ describe('MultiSelect', () => { expect(translateWithId).toHaveBeenCalled(); }); - it('should call onChange when the selection changes', async () => { + it('should call onChange when the selection changes from user selection', async () => { const testFunction = jest.fn(); const items = generateItems(4, generateGenericItem); const label = 'test-label'; @@ -482,6 +482,30 @@ describe('MultiSelect', () => { expect(testFunction).toHaveBeenCalledTimes(1); }); + it('should call onChange when the selection changes outside of the component', () => { + const handleChange = jest.fn(); + const items = generateItems(4, generateGenericItem); + const props = { + id: 'custom-id', + onChange: handleChange, + selectedItems: [], + label: 'test-label', + items, + }; + const { rerender } = render(); + + expect(handleChange).not.toHaveBeenCalled(); + + act(() => { + rerender(); + }); + + expect(handleChange).toHaveBeenCalledTimes(1); + expect(handleChange.mock.lastCall[0]).toMatchObject({ + selectedItems: [items[0]], + }); + }); + it('should support an invalid state with invalidText that describes the field', () => { const items = generateItems(4, generateGenericItem); const label = 'test-label'; diff --git a/packages/react/src/internal/Selection.js b/packages/react/src/internal/Selection.js index 35bc753c8ddd..4c8e5a1ae90f 100644 --- a/packages/react/src/internal/Selection.js +++ b/packages/react/src/internal/Selection.js @@ -39,6 +39,11 @@ export function useSelection({ const [selectedItems, setSelectedItems] = useState( isControlled ? controlledItems : uncontrolledItems ); + + useEffect(() => { + setSelectedItems(isControlled ? controlledItems : uncontrolledItems); + }, [isControlled, controlledItems, uncontrolledItems]); + useEffect(() => { callOnChangeHandler({ isControlled, @@ -80,7 +85,7 @@ export function useSelection({ isMounted: isMounted.current, onChangeHandlerControlled: savedOnChange.current, onChangeHandlerUncontrolled: setUncontrolledItems, - selectedItems: setSelectedItems([]), + selectedItems: [], }); }, [disabled, isControlled]);