diff --git a/lib/Selection/Selection.js b/lib/Selection/Selection.js
index 70c542a8b..60fe0d051 100644
--- a/lib/Selection/Selection.js
+++ b/lib/Selection/Selection.js
@@ -1,7 +1,7 @@
import React, { useState, useMemo, useEffect, useCallback, useRef } from 'react';
import PropTypes from 'prop-types';
import { useIntl } from 'react-intl';
-import { UseComboboxStateChangeTypes, useCombobox } from 'downshift';
+import { useCombobox } from 'downshift';
import classNames from 'classnames';
import isEqual from 'lodash/isEqual';
import formField from '../FormField';
@@ -120,11 +120,9 @@ const Selection = ({
}) => {
const { formatMessage } = useIntl();
const [filterValue, updateFilterValue] = useState('');
- const [selectedItem, updateSelectedItem] = useState(value ? getSelectedObject(value, dataOptions) : null);
const dataLength = useRef(dataOptions?.length || 0);
const controlRef = useProvidedRefOrCreate(inputRef);
const awaitingChange = useRef(false);
- const chosenItem = useRef(null);
const options = useMemo(
() => (asyncFilter ? dataOptions :
filterValue ? onFilter(filterValue, dataOptions) : dataOptions),
@@ -133,15 +131,6 @@ const Selection = ({
const testId = useProvidedIdOrCreate(id, 'selection-');
- useEffect(() => {
- // if dataOptions populate after the initial render, update the selectedItem state
- // if one hasn't been found yet.
- if (dataOptions?.length !== dataLength.current && value && selectedItem === null) {
- updateSelectedItem(getSelectedObject(value, dataOptions));
- dataLength.current = dataOptions.length;
- }
- }, [dataOptions, selectedItem, value])
-
// we need to skip over group headings since those can neither be selectable or cursored over.
const reducedListItems = reduceOptions(options);
const {
@@ -152,14 +141,17 @@ const Selection = ({
getMenuProps,
highlightedIndex,
getItemProps,
+ selectedItem,
+ selectItem: updateSelectedItem,
} = useCombobox({
items: reducedListItems,
itemToString: defaultItemToString,
- selectedItem,
+ initialSelectedItem: value ? getSelectedObject(value, dataOptions) : null,
onSelectedItemChange: ({ selectedItem: newSelectedItem }) => {
- if (onChange) {
+ // if the newSelectedItem's value matches the incoming value prop, we assume that
+ // onChange isn't necessary.
+ if (onChange && newSelectedItem?.value !== value) {
onChange(newSelectedItem.value);
- updateSelectedItem(newSelectedItem);
}
},
isItemDisabled(item) {
@@ -171,7 +163,6 @@ const Selection = ({
case useCombobox.stateChangeTypes.InputKeyDownEnter:
case useCombobox.stateChangeTypes.ItemClick:
awaitingChange.current = true;
- chosenItem.current = changes.selectedItem;
return changes;
default:
return changes;
@@ -179,23 +170,31 @@ const Selection = ({
}
});
+ useEffect(() => {
+ // if dataOptions populate/change after the initial render, update the selectedItem state
+ // if one hasn't been found yet.
+ if (dataOptions?.length !== dataLength.current && value && selectedItem === null) {
+ const selected = getSelectedObject(value, dataOptions);
+ updateSelectedItem(selected);
+ dataLength.current = dataOptions.length;
+ }
+ }, [dataOptions, selectedItem, value])
+
const valueLabel = defaultItemToString(selectedItem) || placeholder || '';
const labelId = `sl-label-${testId}`;
const valueId = `selected-${testId}-item`;
if (awaitingChange.current) {
- if (chosenItem.current !== null && chosenItem.current?.value === value) {
+ // a user has change the value via the dropdown and we're waiting for the value
+ // to correspond with the state...
+ if (selectedItem !== null && selectedItem?.value === value) {
awaitingChange.current = false;
- chosenItem.current = null;
}
} else if (selectedItem !== null && selectedItem?.value !== value){
- // conform to post-render value prop changes from outside of the component,
+ // conform to post-mount value prop changes from outside of the component,
// whether the changed value is something empty like '' or null;
const newValue = getSelectedObject(value, dataOptions) || { value }
updateSelectedItem(newValue);
- if (onChange) {
- onChange(newValue.value);
- }
}
/** renderOptions
diff --git a/lib/Selection/tests/Selection-test.js b/lib/Selection/tests/Selection-test.js
index 873786aea..d8410c9dc 100644
--- a/lib/Selection/tests/Selection-test.js
+++ b/lib/Selection/tests/Selection-test.js
@@ -577,29 +577,32 @@ describe('Selection', () => {
describe('Changing the value prop outside of render', () => {
const changeSpy = sinon.spy();
+ const harnessHandler = (fn, val) => {
+ changeSpy(val);
+ fn(val);
+ };
beforeEach(async () => {
+ changeSpy.resetHistory();
await mountWithContext(
);
});
it('renders initial value as provided', () => selection.has({ singleValue: 'Option 2' }));
- describe('Reseting the value', () => {
+ describe('Resetting the value outside of the component', () => {
beforeEach(async () => {
await Button('reset').click();
});
- it('removes the value', () => selection.has({ singleValue: '' }))
+ it('removes the value', () => selection.has({ singleValue: '' }));
- it('calls the supplied change handler', async () => {
- await expect(changeSpy.calledOnceWith(''));
- })
+ it('will not trigger a onChange', async () => converge(() => { if (changeSpy.called) throw new Error('expected change handler to be called once') }));
});
});
@@ -633,6 +636,14 @@ describe('Selection', () => {
it('updates the value', () => selection.has({ singleValue: 'Option 0' }));
it('calls the supplied change handler', async () => converge(() => { if (!changeSpy.calledOnceWith('test0')) throw new Error('expected change handler to be called') }));
+
+ describe('additional renders...', () => {
+ beforeEach(async () => {
+ await Button(including('update')).click();
+ });
+
+ it('will not trigger an additional onChange', async () => converge(() => { if (!changeSpy.calledOnce) throw new Error('expected change handler to be called once') }));
+ });
});
});
diff --git a/lib/Selection/tests/SingleSelectionHarness.js b/lib/Selection/tests/SingleSelectionHarness.js
index a53a39449..bb019a88f 100644
--- a/lib/Selection/tests/SingleSelectionHarness.js
+++ b/lib/Selection/tests/SingleSelectionHarness.js
@@ -10,11 +10,13 @@ const SingleSelectionHarness = ({
onChange = (fn, val) => { fn(val) },
}) => {
const [fieldVal, setFieldVal] = useState(initValue);
- const [options, updateOptions] = useState(optionsProp)
+ const [options, updateOptions] = useState(optionsProp);
+ const [increment, updateIncrement] = useState(0);
return (
<>
+ ;