From ade5809dff41b448f1b7b3a8b8d736b491a73d6d Mon Sep 17 00:00:00 2001 From: Stefan Lau Date: Tue, 5 Feb 2019 23:18:36 +0100 Subject: [PATCH] fix(netlify-cms-widget-select) show label instead of value when multiple is true (#2054) --- .../src/SelectControl.js | 14 ++- .../src/__tests__/select.spec.js | 102 +++++++++++++++++- 2 files changed, 107 insertions(+), 9 deletions(-) diff --git a/packages/netlify-cms-widget-select/src/SelectControl.js b/packages/netlify-cms-widget-select/src/SelectControl.js index 3a53dabf..f94a5824 100644 --- a/packages/netlify-cms-widget-select/src/SelectControl.js +++ b/packages/netlify-cms-widget-select/src/SelectControl.js @@ -50,7 +50,7 @@ const styles = { }; function optionToString(option) { - return option && option.value ? option.value : ''; + return option && option.value ? option.value : null; } function convertToOption(raw) { @@ -82,10 +82,15 @@ export default class SelectControl extends React.Component { }; handleChange = selectedOption => { - const { onChange } = this.props; + const { onChange, field } = this.props; + const isMultiple = field.get('multiple', false); if (Array.isArray(selectedOption)) { - onChange(fromJS(selectedOption.map(optionToString))); + if (!isMultiple && selectedOption.length === 0) { + onChange(null); + } else { + onChange(fromJS(selectedOption.map(optionToString))); + } } else { onChange(optionToString(selectedOption)); } @@ -100,7 +105,8 @@ export default class SelectControl extends React.Component { } return selectedOptions - .filter(i => options.find(o => o.value === (i.value || i))) + .map(i => options.find(o => o.value === (i.value || i))) + .filter(Boolean) .map(convertToOption); } else { return find(options, ['value', value]) || null; diff --git a/packages/netlify-cms-widget-select/src/__tests__/select.spec.js b/packages/netlify-cms-widget-select/src/__tests__/select.spec.js index 44ca472b..5d7e10a8 100644 --- a/packages/netlify-cms-widget-select/src/__tests__/select.spec.js +++ b/packages/netlify-cms-widget-select/src/__tests__/select.spec.js @@ -6,10 +6,11 @@ import 'jest-dom/extend-expect'; import { SelectControl } from '../'; const options = [ - { value: 'Foo', label: 'Foo' }, - { value: 'Bar', label: 'Bar' }, - { value: 'Baz', label: 'Baz' }, + { value: 'foo', label: 'Foo' }, + { value: 'bar', label: 'Bar' }, + { value: 'baz', label: 'Baz' }, ]; +const stringOptions = ['foo', 'bar', 'baz']; class SelectController extends React.Component { state = { @@ -69,6 +70,15 @@ function setup({ field, defaultValue }) { }; } +function clickClearButton(container) { + const allSvgs = container.querySelectorAll('svg'); + const clear = allSvgs[allSvgs.length - 2]; + + fireEvent.mouseDown(clear, { + button: 0, + }); +} + describe('Select widget', () => { it('should call onChange with correct selectedItem', () => { const field = fromJS({ options }); @@ -82,13 +92,44 @@ describe('Select widget', () => { expect(onChangeSpy).toHaveBeenCalledWith(options[0].value); }); - it('should respect value', () => { + it('should call onChange with null when no item is selected', () => { + const field = fromJS({ options }); + const { input, onChangeSpy } = setup({ field, defaultValue: options[0].value }); + + fireEvent.focus(input); + fireEvent.keyDown(input, { key: 'Delete' }); + + expect(onChangeSpy).toHaveBeenCalledTimes(1); + expect(onChangeSpy).toHaveBeenCalledWith(null); + }); + + it('should call onChange with null when selection is cleared', () => { + const field = fromJS({ options, required: false }); + const { onChangeSpy, container } = setup({ field, defaultValue: options[0].value }); + + clickClearButton(container); + + expect(onChangeSpy).toHaveBeenCalledTimes(1); + expect(onChangeSpy).toHaveBeenCalledWith(null); + }); + + it('should respect default value', () => { const field = fromJS({ options }); const { getByText } = setup({ field, defaultValue: options[2].value }); expect(getByText('Baz')).toBeInTheDocument(); }); + it('should respect default value when options are string only', () => { + const field = fromJS({ options: stringOptions }); + const { getByText } = setup({ + field, + defaultValue: stringOptions[2], + }); + + expect(getByText('baz')).toBeInTheDocument(); + }); + describe('with multiple', () => { it('should call onChange with correct selectedItem', () => { const field = fromJS({ options, multiple: true }); @@ -104,7 +145,47 @@ describe('Select widget', () => { expect(onChangeSpy).toHaveBeenCalledWith(fromJS([options[0].value, options[2].value])); }); - it('should respect value', () => { + it('should call onChange with correct selectedItem when item is removed', () => { + const field = fromJS({ options, multiple: true }); + const { container, onChangeSpy } = setup({ + field, + defaultValue: fromJS([options[1].value, options[2].value]), + }); + + fireEvent.click(container.querySelector('svg'), { button: 0 }); + + expect(onChangeSpy).toHaveBeenCalledTimes(1); + expect(onChangeSpy).toHaveBeenCalledWith(fromJS([options[2].value])); + }); + + it('should call onChange with empty list when no item is selected', () => { + const field = fromJS({ options, multiple: true }); + const { input, onChangeSpy } = setup({ + field, + defaultValue: fromJS([options[1].value]), + }); + + fireEvent.focus(input); + fireEvent.keyDown(input, { key: 'Delete' }); + + expect(onChangeSpy).toHaveBeenCalledTimes(1); + expect(onChangeSpy).toHaveBeenCalledWith(fromJS([])); + }); + + it('should call onChange with empty list when selection is cleared', () => { + const field = fromJS({ options, multiple: true }); + const { container, onChangeSpy } = setup({ + field, + defaultValue: fromJS([options[1].value]), + }); + + clickClearButton(container); + + expect(onChangeSpy).toHaveBeenCalledTimes(1); + expect(onChangeSpy).toHaveBeenCalledWith(fromJS([])); + }); + + it('should respect default value', () => { const field = fromJS({ options, multiple: true }); const { getByText } = setup({ field, @@ -114,5 +195,16 @@ describe('Select widget', () => { expect(getByText('Bar')).toBeInTheDocument(); expect(getByText('Baz')).toBeInTheDocument(); }); + + it('should respect default value when options are string only', () => { + const field = fromJS({ options: stringOptions, multiple: true }); + const { getByText } = setup({ + field, + defaultValue: fromJS([stringOptions[1], stringOptions[2]]), + }); + + expect(getByText('bar')).toBeInTheDocument(); + expect(getByText('baz')).toBeInTheDocument(); + }); }); });