Skip to content

Commit

Permalink
fix(multiselect): add useEffect to selection hook to update externally (
Browse files Browse the repository at this point in the history
#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 <[email protected]>
  • Loading branch information
ychavoya and tw15egan authored May 6, 2024
1 parent f3d8bf4 commit 0bc9c78
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 4 deletions.
9 changes: 9 additions & 0 deletions .all-contributorsrc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ check out our [Contributing Guide](/.github/CONTRIBUTING.md) and our
<td align="center"><a href="https://hollyos.com/"><img src="https://avatars.githubusercontent.com/u/4097509?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Holly Springsteen</b></sub></a><br /><a href="https://github.com/carbon-design-system/carbon/commits?author=hollyos" title="Code">💻</a></td>
<td align="center"><a href="https://github.com/2nikhiltom"><img src="https://avatars.githubusercontent.com/2nikhiltom?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Nikhil Tomar</b></sub></a><br /><a href="https://github.com/carbon-design-system/carbon/commits?author=2nikhiltom" title="Code">💻</a></td>
<td align="center"><a href="https://github.com/aninaantony"><img src="https://avatars.githubusercontent.com/u/164350784?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Anina Antony</b></sub></a><br /><a href="https://github.com/carbon-design-system/carbon/commits?author=aninaantony" title="Code">💻</a></td>
<td align="center"><a href="https://github.com/ychavoya"><img src="https://avatars.githubusercontent.com/u/7907338?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Yael Chavoya</b></sub></a><br /><a href="https://github.com/carbon-design-system/carbon/commits?author=ychavoya" title="Code">💻</a></td>
</tr>
</table>

Expand Down
43 changes: 42 additions & 1 deletion packages/react/src/components/MultiSelect/MultiSelect.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -306,3 +309,41 @@ export const _FilterableWithLayer = () => (
)}
</WithLayer>
);

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 (
<div style={{ width: 300 }}>
<MultiSelect
id="carbon-multiselect-example-controlled"
titleText="Multiselect title"
label="Multiselect label"
items={items}
selectedItems={selectedItems}
onChange={(data) => onSelectionChanged(data.selectedItems)}
itemToString={(item) => (item ? item.text : '')}
selectionFeedback="top-after-reopen"
/>
<br />
<ButtonSet>
<Button id="all" onClick={() => setSelectedItems(items)}>
Select all
</Button>
<Button
id="clear"
kind="secondary"
onClick={() => setSelectedItems([])}>
Clear
</Button>
</ButtonSet>
</div>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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';
Expand All @@ -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(<MultiSelect {...props} />);

expect(handleChange).not.toHaveBeenCalled();

act(() => {
rerender(<MultiSelect {...props} selectedItems={[items[0]]} />);
});

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';
Expand Down
7 changes: 6 additions & 1 deletion packages/react/src/internal/Selection.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -80,7 +85,7 @@ export function useSelection({
isMounted: isMounted.current,
onChangeHandlerControlled: savedOnChange.current,
onChangeHandlerUncontrolled: setUncontrolledItems,
selectedItems: setSelectedItems([]),
selectedItems: [],
});
}, [disabled, isControlled]);

Expand Down

0 comments on commit 0bc9c78

Please sign in to comment.