component will wait for a value prop to release its internal sentinel…

… instead of releasing after onChange is called. (#2321)

lib/MultiSelection/tests/MultiSelection-test.js

@@ -727,28 +727,65 @@ describe('when a MultiSelect is set as required and placed inside a <form>', ()
   describe('Changing the value prop outside of render', () => {
     const changeSpy = sinon.spy();
     beforeEach(async () => {
+      changeSpy.resetHistory();
+      const harnessFunc = (fn, val) => {
+        changeSpy(val);
+        fn(val);
+      };
       await mountWithContext(
         <MultiSelectionHarness
           label="test multiselection"
           initValue={[listOptions[2]]}
           options={listOptions}
-          onChange={changeSpy}
+          onChange={harnessFunc}
         />
       );
     });
 
-    it('renders initial value as provided', () => multiselection.has({ selected: ['Option 2']}));
+    it('renders initial value as provided', () => multiselection.has({ selected: ['Option 2'] }));
 
-    describe('Reseting the value', () => {
+    describe('Resetting the value', () => {
       beforeEach(async () => {
         await Button('reset').click();
       });
 
       it('removes the value', () => multiselection.has({ selectedCount: 0 }))
 
-      it('calls the supplied change handler', () => {
-        expect(changeSpy.calledOnceWith([]));
-      })
+      it('calls the supplied change handler', async () => converge(() => { if (!changeSpy.calledOnceWith([])) throw new Error('expected change handler to be called') }));
+    });
+  });
+
+  describe('Change handlers slow to resolve', () => {
+    const changeSpy = sinon.spy();
+
+    beforeEach(async () => {
+      changeSpy.resetHistory();
+      const harnessFunc = (fn, val) => {
+        setTimeout(() => {
+          changeSpy(val);
+          fn(val);
+        }, 100)
+      };
+      await mountWithContext(
+        <MultiSelectionHarness
+          label="test multiselection"
+          initValue={[listOptions[2]]}
+          options={listOptions}
+          onChange={harnessFunc}
+        />
+      );
+    });
+
+    it('renders initial value as provided', () => multiselection.has({ selected: ['Option 2'] }));
+
+    describe('Choosing a new value', () => {
+      beforeEach(async () => {
+        await multiselection.choose('Option 0');
+      });
+
+      it('updates the value', () => multiselection.has({ selectedCount: 2 }))
+
+      it('calls the supplied change handler', async () => converge(() => { if (!changeSpy.calledOnceWith([listOptions[2], listOptions[0]])) throw new Error('expected change handler to be called') }));
     });
   });
 });

lib/MultiSelection/tests/MultiSelectionHarness.js

@@ -6,7 +6,7 @@ const MultiSelectionHarness = ({
   initValue,
   label,
   options,
-  onChange = () => {},
+  onChange = (fn, val) => { fn(val) },
 }) => {
   const [fieldVal, setFieldVal] = useState(initValue);
 
@@ -17,7 +17,7 @@ const MultiSelectionHarness = ({
         label={label}
         value={fieldVal}
         dataOptions={options}
-        onChange={(val) => { setFieldVal(val); onChange(val) }}
+        onChange={(val) => { onChange(setFieldVal, val) }}
       />
     </>
   );

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 { useCombobox } from 'downshift';
+import { UseComboboxStateChangeTypes, useCombobox } from 'downshift';
 import classNames from 'classnames';
 import isEqual from 'lodash/isEqual';
 import formField from '../FormField';
@@ -123,6 +123,8 @@ const Selection = ({
   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),
@@ -155,19 +157,38 @@ const Selection = ({
     itemToString: defaultItemToString,
     selectedItem,
     onSelectedItemChange: ({ selectedItem: newSelectedItem }) => {
-      if (onChange) onChange(newSelectedItem.value);
-      updateSelectedItem(newSelectedItem);
+      if (onChange) {
+        onChange(newSelectedItem.value);
+        updateSelectedItem(newSelectedItem);
+      }
     },
     isItemDisabled(item) {
       return ((readOnly || readonly) && !isEqual(item, selectedItem));
+    },
+    stateReducer(state, actionAndChanges) {
+      const { changes, type } = actionAndChanges;
+      switch (type) {
+        case useCombobox.stateChangeTypes.InputKeyDownEnter:
+        case useCombobox.stateChangeTypes.ItemClick:
+          awaitingChange.current = true;
+          chosenItem.current = changes.selectedItem;
+          return changes;
+        default:
+          return changes;
+      }
     }
   });
 
   const valueLabel = defaultItemToString(selectedItem) || placeholder || '';
   const labelId = `sl-label-${testId}`;
   const valueId = `selected-${testId}-item`;
 
-  if (selectedItem !== null && selectedItem?.value !== value) {
+  if (awaitingChange.current) {
+    if (chosenItem.current !== null && chosenItem.current?.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,
     // whether the changed value is something empty like '' or null;
     const newValue = getSelectedObject(value, dataOptions) || { value }

lib/Selection/tests/Selection-test.js

@@ -1,7 +1,7 @@
 import React, { createRef } from 'react';
 import { describe, beforeEach, it } from 'mocha';
 import { expect } from 'chai';
-import Sinon from 'sinon';
+import sinon from 'sinon';
 
 import {
   Button,
@@ -13,6 +13,7 @@ import {
   SelectionOption as SelectionOptionInteractor,
   HTML,
   runAxeTest,
+  converge,
 } from '@folio/stripes-testing';
 
 import { mountWithContext } from '../../../tests/helpers';
@@ -575,7 +576,7 @@ describe('Selection', () => {
   });
 
   describe('Changing the value prop outside of render', () => {
-    const changeSpy = Sinon.spy();
+    const changeSpy = sinon.spy();
     beforeEach(async () => {
       await mountWithContext(
         <SingleSelectionHarness
@@ -596,22 +597,60 @@ describe('Selection', () => {
 
       it('removes the value', () => selection.has({ singleValue: '' }))
 
-      it('calls the supplied change handler', () => {
-        expect(changeSpy.calledOnceWith(''));
+      it('calls the supplied change handler', async () => {
+        await expect(changeSpy.calledOnceWith(''));
       })
     });
   });
 
+  describe('Change handlers slow to resolve', () => {
+    const changeSpy = sinon.spy();
+    beforeEach(async () => {
+      changeSpy.resetHistory();
+      const harnessHandler = (fn, val) => {
+        setTimeout(() => {
+          changeSpy(val);
+          fn(val)
+        }, 200)
+      };
+      await mountWithContext(
+        <SingleSelectionHarness
+          label="test selection"
+          initValue="test2"
+          options={listOptions}
+          onChange={harnessHandler}
+        />
+      );
+    });
+
+    it('renders initial value as provided', () => selection.has({ singleValue: 'Option 2' }));
+
+    describe('Choosing an option', () => {
+      beforeEach(async () => {
+        await selection.choose('Option 0');
+      });
+
+      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('Changing data options after initial render', () => {
-    const changeSpy = Sinon.spy();
+    const changeSpy = sinon.spy();
+    const harnessHandler = (fn, val) => {
+      changeSpy(val);
+      fn(val);
+    };
+
     beforeEach(async () => {
       await mountWithContext(
         <SingleSelectionHarness
           label="test selection"
           initValue="test2"
           options={[]}
           delayedOptions={listOptions}
-          onChange={changeSpy}
+          onChange={harnessHandler}
         />
       );
     });

lib/Selection/tests/SingleSelectionHarness.js

@@ -7,7 +7,7 @@ const SingleSelectionHarness = ({
   label,
   options: optionsProp,
   delayedOptions = [],
-  onChange = () => {},
+  onChange = (fn, val) => { fn(val) },
 }) => {
   const [fieldVal, setFieldVal] = useState(initValue);
   const [options, updateOptions] = useState(optionsProp)
@@ -19,7 +19,7 @@ const SingleSelectionHarness = ({
         label={label}
         value={fieldVal}
         dataOptions={options}
-        onChange={(val) => { setFieldVal(val); onChange(val) }}
+        onChange={(val) => { onChange(setFieldVal, val) }}
       />
     </>
   );