Merge pull request #3416 from JedWatson/normalise-value-removal

3.x Normalise value removal

Author: gwyneplaine

src/Select.js

@@ -663,8 +663,9 @@ export default class Select extends Component<Props, State> {
   removeValue = (removedValue: OptionType) => {
     const { selectValue } = this.state;
     const candidate = this.getOptionValue(removedValue);
+    const newValue = selectValue.filter(i => this.getOptionValue(i) !== candidate);
     this.onChange(
-      selectValue.filter(i => this.getOptionValue(i) !== candidate),
+      newValue.length ? newValue : null,
       {
         action: 'remove-value',
         removedValue,
@@ -685,13 +686,14 @@ export default class Select extends Component<Props, State> {
   popValue = () => {
     const { selectValue } = this.state;
     const lastSelectedValue = selectValue[selectValue.length - 1];
+    const newValue = selectValue.slice(0, selectValue.length - 1);
     this.announceAriaLiveSelection({
       event: 'pop-value',
       context: {
         value: lastSelectedValue ? this.getOptionLabel(lastSelectedValue) : '',
       },
     });
-    this.onChange(selectValue.slice(0, selectValue.length - 1), {
+    this.onChange(newValue.length ? newValue : null, {
       action: 'pop-value',
       removedValue: lastSelectedValue,
     });

src/__tests__/Select.test.js

@@ -1491,39 +1491,44 @@ test('should not call onChange on hitting backspace even when backspaceRemovesVa
   expect(onChangeSpy).not.toHaveBeenCalled();
 });
 
-test('should call onChange with `null` on hitting backspace when backspaceRemovesValue is true and isMulti is false', () => {
+cases('should call onChange with `null` on hitting backspace when backspaceRemovesValue is true', ({ props = { ...BASIC_PROPS }, expectedValue }) => {
   let onChangeSpy = jest.fn();
   let selectWrapper = mount(
     <Select
-      {...BASIC_PROPS}
+      {...props}
       backspaceRemovesValue
       isClearable
-      isMulti={false}
       onChange={onChangeSpy}
     />
   );
   selectWrapper
     .find(Control)
     .simulate('keyDown', { keyCode: 8, key: 'Backspace' });
-  expect(onChangeSpy).toHaveBeenCalledWith(null, { action: 'clear', name: 'test-input-name' });
+  expect(onChangeSpy).toHaveBeenCalledWith(null, expectedValue);
+}, {
+  'and isMulti is false': {
+    props: {
+      ...BASIC_PROPS,
+      isMulti: false,
+    },
+    expectedValue: {
+      action: 'clear',
+      name: 'test-input-name',
+    }
+  },
+  'and isMulti is true': {
+    props: {
+      ...BASIC_PROPS,
+      isMulti: true,
+    },
+    expectedValue: {
+      action: 'pop-value',
+      name: 'test-input-name',
+      removedValue: undefined
+    }
+  },
 });
 
-test('should call onChange with an array on hitting backspace when backspaceRemovesValue is true and isMulti is true', () => {
-  let onChangeSpy = jest.fn();
-  let selectWrapper = mount(
-    <Select
-      {...BASIC_PROPS}
-      backspaceRemovesValue
-      isClearable
-      isMulti
-      onChange={onChangeSpy}
-    />
-  );
-  selectWrapper
-    .find(Control)
-    .simulate('keyDown', { keyCode: 8, key: 'Backspace' });
-  expect(onChangeSpy).toHaveBeenCalledWith([], { action: 'pop-value', name: 'test-input-name', removedValue: undefined });
-});
 
 test('multi select > clicking on X next to option will call onChange with all options other that the clicked option', () => {
   let onChangeSpy = jest.fn();