From fc524e6c0fc0174294b42cd90d9eb23e96d15d53 Mon Sep 17 00:00:00 2001 From: Shawn Erquhart Date: Fri, 31 Jan 2020 16:51:40 -0800 Subject: [PATCH] fix(widget-select): allow optional field to use min/max (#3175) --- .../src/SelectControl.js | 79 ++++++++----------- .../src/SelectPreview.js | 17 +++- .../src/__tests__/select.spec.js | 74 ++++++++++++++--- 3 files changed, 114 insertions(+), 56 deletions(-) diff --git a/packages/netlify-cms-widget-select/src/SelectControl.js b/packages/netlify-cms-widget-select/src/SelectControl.js index 69bcad05..e577e7cf 100644 --- a/packages/netlify-cms-widget-select/src/SelectControl.js +++ b/packages/netlify-cms-widget-select/src/SelectControl.js @@ -59,47 +59,25 @@ export default class SelectControl extends React.Component { const { field, value, t } = this.props; const min = field.get('min'); const max = field.get('max'); + const minMaxError = messageKey => ({ + error: { + message: t(`editor.editorControlPane.widget.${messageKey}`, { + fieldLabel: field.get('label', field.get('name')), + minValue: min, + maxValue: max, + }), + }, + }); + if (!field.get('multiple')) { return { error: false }; } - if ([min, max].every(isNumber)) { - const isValid = value && value.size >= min && value.size <= max; - const messageKey = min === max ? 'rangeCountExact' : 'rangeCount'; - if (!isValid) { - return { - error: { - message: t(`editor.editorControlPane.widget.${messageKey}`, { - fieldLabel: field.get('label', field.get('name')), - minValue: min, - maxValue: max, - }), - }, - }; - } - } else if (isNumber(min)) { - const isValid = value && value.size >= min; - if (!isValid) { - return { - error: { - message: t('editor.editorControlPane.widget.rangeMin', { - fieldLabel: field.get('label', field.get('name')), - minValue: min, - }), - }, - }; - } - } else if (isNumber(max)) { - const isValid = !value || value.size <= max; - if (!isValid) { - return { - error: { - message: t('editor.editorControlPane.widget.rangeMax', { - fieldLabel: field.get('label', field.get('name')), - maxValue: max, - }), - }, - }; - } + if ([min, max].every(isNumber) && value?.size && (value.size < min || value.size > max)) { + return minMaxError(min === max ? 'rangeCountExact' : 'rangeCount'); + } else if (isNumber(min) && min > 0 && value?.size && value.size < min) { + return minMaxError('rangeMin'); + } else if (isNumber(max) && value?.size && value.size > max) { + return minMaxError('rangeMax'); } return { error: false }; }; @@ -107,18 +85,31 @@ export default class SelectControl extends React.Component { handleChange = selectedOption => { const { onChange, field } = this.props; const isMultiple = field.get('multiple', false); + const isEmpty = isMultiple ? !selectedOption?.length : !selectedOption; - if (Array.isArray(selectedOption)) { - if (!isMultiple && selectedOption.length === 0) { - onChange(null); - } else { - onChange(fromJS(selectedOption.map(optionToString))); - } + if (field.get('required') && isEmpty && isMultiple) { + onChange(List()); + } else if (isEmpty) { + onChange(null); + } else if (isMultiple) { + const options = selectedOption.map(optionToString); + onChange(fromJS(options)); } else { onChange(optionToString(selectedOption)); } }; + componentDidMount() { + const { field, onChange, value } = this.props; + if (field.get('required') && field.get('multiple')) { + if (value && !List.isList(value)) { + onChange(fromJS([value])); + } else if (!value) { + onChange(fromJS([])); + } + } + } + render() { const { field, value, forID, classNameWrapper, setActiveStyle, setInactiveStyle } = this.props; const fieldOptions = field.get('options'); diff --git a/packages/netlify-cms-widget-select/src/SelectPreview.js b/packages/netlify-cms-widget-select/src/SelectPreview.js index 7daf92ca..80152613 100644 --- a/packages/netlify-cms-widget-select/src/SelectPreview.js +++ b/packages/netlify-cms-widget-select/src/SelectPreview.js @@ -1,13 +1,26 @@ import PropTypes from 'prop-types'; import React from 'react'; +import { List } from 'immutable'; +import ImmutablePropTypes from 'react-immutable-proptypes'; import { WidgetPreviewContainer } from 'netlify-cms-ui-default'; +const ListPreview = ({ values }) => ( + +); + const SelectPreview = ({ value }) => ( - {value ? value.toString() : null} + + {value && (List.isList(value) ? : value)} + {!value && null} + ); SelectPreview.propTypes = { - value: PropTypes.string, + value: PropTypes.oneOfType([PropTypes.string, ImmutablePropTypes.list]), }; export default SelectPreview; 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 0fabd204..59474b92 100644 --- a/packages/netlify-cms-widget-select/src/__tests__/select.spec.js +++ b/packages/netlify-cms-widget-select/src/__tests__/select.spec.js @@ -1,5 +1,5 @@ import React from 'react'; -import { fromJS } from 'immutable'; +import { fromJS, List } from 'immutable'; import { render, fireEvent } from '@testing-library/react'; import { NetlifyCmsWidgetSelect } from '../'; @@ -53,7 +53,7 @@ function setup({ field, defaultValue }) { setActiveStyle={setActiveSpy} setInactiveStyle={setInactiveSpy} ref={widgetRef => (ref = widgetRef)} - t={jest.fn(msg => msg)} + t={msg => msg} /> ); }} @@ -161,7 +161,23 @@ describe('Select widget', () => { expect(onChangeSpy).toHaveBeenCalledWith(fromJS([options[2].value])); }); - it('should call onChange with empty list when no item is selected', () => { + it('should call onChange with empty list on mount when required is true', () => { + const field = fromJS({ options, multiple: true, required: true }); + const { onChangeSpy } = setup({ + field, + }); + expect(onChangeSpy).toHaveBeenCalledWith(List()); + }); + + it('should not call onChange with empty list on mount when required is false', () => { + const field = fromJS({ options, multiple: true }); + const { onChangeSpy } = setup({ + field, + }); + expect(onChangeSpy).not.toHaveBeenCalled(); + }); + + it('should call onChange with empty list when no item is selected and required is true', () => { const field = fromJS({ options, multiple: true }); const { input, onChangeSpy } = setup({ field, @@ -172,11 +188,20 @@ describe('Select widget', () => { fireEvent.keyDown(input, { key: 'Delete' }); expect(onChangeSpy).toHaveBeenCalledTimes(1); - expect(onChangeSpy).toHaveBeenCalledWith(fromJS([])); + expect(onChangeSpy).toHaveBeenCalledWith(null); }); - it('should call onChange with empty list when selection is cleared', () => { - const field = fromJS({ options, multiple: true }); + it('should call onChange with value in list on mount when value is not a list and required is true', () => { + const field = fromJS({ options, multiple: true, required: true }); + const { onChangeSpy } = setup({ + field, + defaultValue: options[1].value, + }); + expect(onChangeSpy).toHaveBeenCalledWith(fromJS([options[1].value])); + }); + + it('should call onChange with empty list when selection is cleared and required is true', () => { + const field = fromJS({ options, multiple: true, required: true }); const { container, onChangeSpy } = setup({ field, defaultValue: fromJS([options[1].value]), @@ -185,7 +210,20 @@ describe('Select widget', () => { clickClearButton(container); expect(onChangeSpy).toHaveBeenCalledTimes(1); - expect(onChangeSpy).toHaveBeenCalledWith(fromJS([])); + expect(onChangeSpy).toHaveBeenCalledWith(List()); + }); + + it('should call onChange with null when selection is cleared and required is false', () => { + const field = fromJS({ options, multiple: true, required: false }); + const { container, onChangeSpy } = setup({ + field, + defaultValue: fromJS([options[1].value]), + }); + + clickClearButton(container); + + expect(onChangeSpy).toHaveBeenCalledTimes(1); + expect(onChangeSpy).toHaveBeenCalledWith(null); }); it('should respect default value', () => { @@ -210,7 +248,7 @@ describe('Select widget', () => { expect(getByText('baz')).toBeInTheDocument(); }); }); - describe.only('validation', () => { + describe('validation', () => { function validate(setupOpts) { const { ref } = setup(setupOpts); const { error } = ref.isValid(); @@ -218,7 +256,8 @@ describe('Select widget', () => { } it('should fail with less items than min allows', () => { const opts = { - field: fromJS({ options: stringOptions, multiple: true, min: 1 }), + field: fromJS({ options: stringOptions, multiple: true, min: 2 }), + defaultValue: fromJS([stringOptions[0]]), }; expect(validate(opts)).toMatchInlineSnapshot(`"editor.editorControlPane.widget.rangeMin"`); }); @@ -231,7 +270,8 @@ describe('Select widget', () => { }); it('should enforce min when both min and max are set', () => { const opts = { - field: fromJS({ options: stringOptions, multiple: true, min: 1, max: 2 }), + field: fromJS({ options: stringOptions, multiple: true, min: 2, max: 3 }), + defaultValue: fromJS([stringOptions[0]]), }; expect(validate(opts)).toMatchInlineSnapshot(`"editor.editorControlPane.widget.rangeCount"`); }); @@ -286,5 +326,19 @@ describe('Select widget', () => { }; expect(validate(opts)).toBeUndefined(); }); + it('should not fail for empty field (should work for optional field)', () => { + const opts = { + field: fromJS({ options: stringOptions, multiple: true, min: 2 }), + }; + const { ref, input, getByText, container } = setup(opts); + expect(ref.isValid().error?.message).toBeUndefined(); + fireEvent.keyDown(input, { key: 'ArrowDown' }); + fireEvent.click(getByText('foo')); + expect(ref.isValid().error?.message).toMatchInlineSnapshot( + `"editor.editorControlPane.widget.rangeMin"`, + ); + clickClearButton(container); + expect(ref.isValid().error?.message).toBeUndefined(); + }); }); });