diff --git a/cypress/integration/MultiSelect/allows_selecting/index.js b/cypress/integration/MultiSelect/allows_selecting/index.js index 9b6be5c689..386e51d608 100644 --- a/cypress/integration/MultiSelect/allows_selecting/index.js +++ b/cypress/integration/MultiSelect/allows_selecting/index.js @@ -41,7 +41,7 @@ Then('the clicked option is selected', () => { cy.window().then(win => { expect(win.onChange).to.be.calledOnce expect(win.onChange).to.be.calledWith({ - selected: [{ label: 'option one', value: '1' }], + selected: ['1'], }) }) }) @@ -50,10 +50,7 @@ Then('the clicked option is selected as well', () => { cy.window().then(win => { expect(win.onChange).to.be.calledOnce expect(win.onChange).to.be.calledWith({ - selected: [ - { label: 'option one', value: '1' }, - { label: 'option two', value: '2' }, - ], + selected: ['1', '2'], }) }) }) diff --git a/cypress/integration/MultiSelect/duplicate_option_values.feature b/cypress/integration/MultiSelect/duplicate_option_values.feature new file mode 100644 index 0000000000..932dd17aef --- /dev/null +++ b/cypress/integration/MultiSelect/duplicate_option_values.feature @@ -0,0 +1,7 @@ +Feature: Duplicate option values result in a conflict between the input and dropdown UI + + Scenario: The MultiSelect has options with a duplicate value and this value is selected + Given a MultiSelect with options with a duplicate value and this value is selected + And the MultiSelect is open + Then the first option with the selected value is displayed in the input + But both options are highlighted in the dropdown diff --git a/cypress/integration/MultiSelect/duplicate_option_values/index.js b/cypress/integration/MultiSelect/duplicate_option_values/index.js new file mode 100644 index 0000000000..5876097c55 --- /dev/null +++ b/cypress/integration/MultiSelect/duplicate_option_values/index.js @@ -0,0 +1,22 @@ +import '../common' +import { Given, Then } from 'cypress-cucumber-preprocessor/steps' + +Given( + 'a MultiSelect with options with a duplicate value and this value is selected', + () => { + cy.visitStory('MultiSelect', 'With duplicate selected option values') + } +) +Then( + 'the first option with the selected value is displayed in the input', + () => { + cy.get('[data-test="dhis2-uicore-select-input"]').should( + 'contain', + 'option one' + ) + } +) +Then('both options are highlighted in the dropdown', () => { + cy.get('input[name="option one"]').should('have.attr', 'checked') + cy.get('input[name="option one a"]').should('have.attr', 'checked') +}) diff --git a/cypress/integration/SingleSelect/accepts_blur_cb/index.js b/cypress/integration/SingleSelect/accepts_blur_cb/index.js index 0582e9b2e0..d442738bd3 100644 --- a/cypress/integration/SingleSelect/accepts_blur_cb/index.js +++ b/cypress/integration/SingleSelect/accepts_blur_cb/index.js @@ -9,7 +9,7 @@ Then('the onBlur handler is called', () => { cy.window().then(win => { expect(win.onBlur).to.be.calledOnce expect(win.onBlur).to.be.calledWith({ - selected: {}, + selected: '', }) }) }) diff --git a/cypress/integration/SingleSelect/accepts_focus_cb/index.js b/cypress/integration/SingleSelect/accepts_focus_cb/index.js index 9785a27b2b..884a4b8bb0 100644 --- a/cypress/integration/SingleSelect/accepts_focus_cb/index.js +++ b/cypress/integration/SingleSelect/accepts_focus_cb/index.js @@ -9,7 +9,7 @@ Then('the onFocus handler is called', () => { cy.window().then(win => { expect(win.onFocus).to.be.calledOnce expect(win.onFocus).to.be.calledWith({ - selected: {}, + selected: '', }) }) }) diff --git a/cypress/integration/SingleSelect/allows_selecting/index.js b/cypress/integration/SingleSelect/allows_selecting/index.js index 9db09f641e..bb2dbb3c33 100644 --- a/cypress/integration/SingleSelect/allows_selecting/index.js +++ b/cypress/integration/SingleSelect/allows_selecting/index.js @@ -27,7 +27,7 @@ Then('the clicked option is selected', () => { cy.window().then(win => { expect(win.onChange).to.be.calledOnce expect(win.onChange).to.be.calledWith({ - selected: { label: 'option one', value: '1' }, + selected: '1', }) }) }) diff --git a/cypress/integration/SingleSelect/can_be_cleared/index.js b/cypress/integration/SingleSelect/can_be_cleared/index.js index 8f63757515..169349c5f0 100644 --- a/cypress/integration/SingleSelect/can_be_cleared/index.js +++ b/cypress/integration/SingleSelect/can_be_cleared/index.js @@ -18,6 +18,6 @@ When('the clear button is clicked', () => { Then('the SingleSelect is cleared', () => { cy.window().then(win => { expect(win.onChange).to.be.calledOnce - expect(win.onChange).to.be.calledWith({ selected: {} }) + expect(win.onChange).to.be.calledWith({ selected: '' }) }) }) diff --git a/cypress/integration/SingleSelect/duplicate_option_values.feature b/cypress/integration/SingleSelect/duplicate_option_values.feature new file mode 100644 index 0000000000..ada56be850 --- /dev/null +++ b/cypress/integration/SingleSelect/duplicate_option_values.feature @@ -0,0 +1,7 @@ +Feature: Duplicate option values result in a conflict between the input and dropdown UI + + Scenario: The SingleSelect has options with a duplicate value and this value is selected + Given a SingleSelect with options with a duplicate value and this value is selected + And the SingleSelect is open + Then the first option with the selected value is displayed in the input + But both options are highlighted in the dropdown diff --git a/cypress/integration/SingleSelect/duplicate_option_values/index.js b/cypress/integration/SingleSelect/duplicate_option_values/index.js new file mode 100644 index 0000000000..5975467874 --- /dev/null +++ b/cypress/integration/SingleSelect/duplicate_option_values/index.js @@ -0,0 +1,27 @@ +import '../common' +import { Given, Then } from 'cypress-cucumber-preprocessor/steps' + +Given( + 'a SingleSelect with options with a duplicate value and this value is selected', + () => { + cy.visitStory('SingleSelect', 'With duplicate selected option values') + } +) +Then( + 'the first option with the selected value is displayed in the input', + () => { + cy.get('[data-test="dhis2-uicore-select-input"]').should( + 'contain', + 'option one' + ) + } +) +Then('both options are highlighted in the dropdown', () => { + cy.get('[data-test="dhis2-uicore-singleselectoption"]') + .contains('option one') + .should('have.class', 'active') + + cy.get('[data-test="dhis2-uicore-singleselectoption"]') + .contains('option one a') + .should('have.class', 'active') +}) diff --git a/cypress/integration/forms/MultiSelect/can_set_a_value/index.js b/cypress/integration/forms/MultiSelect/can_set_a_value/index.js index 4a7d13afc9..81f536b241 100644 --- a/cypress/integration/forms/MultiSelect/can_set_a_value/index.js +++ b/cypress/integration/forms/MultiSelect/can_set_a_value/index.js @@ -27,12 +27,9 @@ When('the user selects the second option', () => { }) Then("the form state's value equals the first option's value", () => { - cy.get('@options').then(options => { - const [firstOption] = options - cy.getFormValue('multiSelect').then(selected => { - expect(selected).to.have.lengthOf(1) - expect(selected).to.deep.equal([firstOption]) - }) + cy.getFormValue('multiSelect').then(selected => { + expect(selected).to.have.lengthOf(1) + expect(selected).to.deep.equal(['value1']) }) }) @@ -40,7 +37,7 @@ Then("the form state's value contains both options", () => { cy.get('@options').then(options => { cy.getFormValue('multiSelect').then(selected => { expect(selected).to.have.lengthOf(options.length) - expect(selected).to.deep.equal(options) + expect(selected).to.deep.equal(['value1', 'value2']) }) }) }) diff --git a/cypress/integration/forms/SingleSelect/can_set_a_value/index.js b/cypress/integration/forms/SingleSelect/can_set_a_value/index.js index 2b1b5302e4..6bf4174aa3 100644 --- a/cypress/integration/forms/SingleSelect/can_set_a_value/index.js +++ b/cypress/integration/forms/SingleSelect/can_set_a_value/index.js @@ -17,7 +17,7 @@ When('the user selects the first option', () => { Then("the form state's value equals the first option's value", () => { cy.get('@options').then(options => { cy.getFormValue('singleSelect').then(actualValue => { - expect(actualValue).to.deep.equal(options[0]) + expect(actualValue).to.deep.equal(options[0].value) }) }) }) diff --git a/packages/constants/src/shared-prop-types.js b/packages/constants/src/shared-prop-types.js index f8a948214c..598ded80c4 100644 --- a/packages/constants/src/shared-prop-types.js +++ b/packages/constants/src/shared-prop-types.js @@ -35,21 +35,6 @@ export const sizePropType = propTypes.mutuallyExclusive( propTypes.bool ) -/** - * SingleSelect selected item propType - * @return {PropType} Object with props `label` and `value`. - */ -export const singleSelectedPropType = propTypes.shape({ - label: propTypes.string, - value: propTypes.string, -}) - -/** - * SingleSelect selected item propType - * @return {Array} Array of Objects with props `label` and `value`. - */ -export const multiSelectedPropType = propTypes.arrayOf(singleSelectedPropType) - /** * Inside alignment props * @return {PropType} PropType that validates the inside alignment. diff --git a/packages/forms/src/MultiSelectControl/MultiSelectControl.js b/packages/forms/src/MultiSelectControl/MultiSelectControl.js index eddb20f3ba..386959edb8 100644 --- a/packages/forms/src/MultiSelectControl/MultiSelectControl.js +++ b/packages/forms/src/MultiSelectControl/MultiSelectControl.js @@ -57,7 +57,12 @@ MultiSelectControl.propTypes = { error: propTypes.bool, loading: propTypes.bool, - options: MultiSelectField.propTypes.selected, + options: propTypes.arrayOf( + propTypes.shape({ + label: propTypes.string, + value: propTypes.string, + }) + ), showLoadingStatus: propTypes.bool, showValidStatus: propTypes.bool, valid: propTypes.bool, diff --git a/packages/forms/src/MultiSelectControl/MultiSelectControl.stories.js b/packages/forms/src/MultiSelectControl/MultiSelectControl.stories.js index cfdd3660e6..29791231c1 100644 --- a/packages/forms/src/MultiSelectControl/MultiSelectControl.stories.js +++ b/packages/forms/src/MultiSelectControl/MultiSelectControl.stories.js @@ -17,12 +17,7 @@ const options = [ { value: '10', label: 'ten' }, ] -const initialValue = [ - { value: '3', label: 'three' }, - { value: '4', label: 'four' }, - { value: '9', label: 'nine' }, - { value: '10', label: 'ten' }, -] +const initialValue = ['3', '4', '9', '10'] storiesOf('Form/MultiSelectControl', module) .addDecorator(formDecorator) diff --git a/packages/forms/src/SingleSelectControl/SingleSelectControl.js b/packages/forms/src/SingleSelectControl/SingleSelectControl.js index efaef79e4e..78326477fc 100644 --- a/packages/forms/src/SingleSelectControl/SingleSelectControl.js +++ b/packages/forms/src/SingleSelectControl/SingleSelectControl.js @@ -39,9 +39,7 @@ export const SingleSelectControl = ({ onFocus={createFocusHandler(input, onFocus)} onChange={createSelectChangeHandler(input)} onBlur={createBlurHandler(input, onBlur)} - selected={ - input.value || {} - } /* input.value is an empty string initially, so we're providing an empty object if falsey */ + selected={input.value || ''} > {options.map(option => ( @@ -53,7 +51,12 @@ export const SingleSelectControl = ({ SingleSelectControl.propTypes = { input: inputPropType.isRequired, meta: metaPropType.isRequired, - options: propTypes.arrayOf(SingleSelectField.propTypes.selected).isRequired, + options: propTypes.arrayOf( + propTypes.shape({ + label: propTypes.string, + value: propTypes.string, + }) + ).isRequired, error: propTypes.bool, loading: propTypes.bool, diff --git a/packages/forms/src/SingleSelectControl/SingleSelectControl.stories.js b/packages/forms/src/SingleSelectControl/SingleSelectControl.stories.js index 1880a39275..8d376a529b 100644 --- a/packages/forms/src/SingleSelectControl/SingleSelectControl.stories.js +++ b/packages/forms/src/SingleSelectControl/SingleSelectControl.stories.js @@ -17,8 +17,6 @@ const options = [ { value: '10', label: 'ten' }, ] -const initialValue = { value: '4', label: 'four' } - storiesOf('Form/SingleSelectControl', module) .addDecorator(formDecorator) .add('Default', () => ( @@ -35,6 +33,6 @@ storiesOf('Form/SingleSelectControl', module) name="agree" label="Do you agree?" options={options} - initialValue={initialValue} + initialValue="4" /> )) diff --git a/packages/widgets/src/MultiSelect/FilterableMenu.js b/packages/widgets/src/MultiSelect/FilterableMenu.js index dff8df3445..b6ecd2b382 100644 --- a/packages/widgets/src/MultiSelect/FilterableMenu.js +++ b/packages/widgets/src/MultiSelect/FilterableMenu.js @@ -1,6 +1,5 @@ import React from 'react' import propTypes from '@dhis2/prop-types' -import { sharedPropTypes } from '@dhis2/ui-constants' import { FilterableMenu as CommonFilterableMenu } from '../Select/FilterableMenu.js' import { Menu } from './Menu.js' @@ -37,7 +36,7 @@ FilterableMenu.propTypes = { handleFocusInput: propTypes.func, options: propTypes.node, placeholder: propTypes.string, - selected: sharedPropTypes.multiSelectedPropType, + selected: propTypes.arrayOf(propTypes.string), onChange: propTypes.func, } diff --git a/packages/widgets/src/MultiSelect/Input.js b/packages/widgets/src/MultiSelect/Input.js index 7b6480a208..9c52a17cf7 100644 --- a/packages/widgets/src/MultiSelect/Input.js +++ b/packages/widgets/src/MultiSelect/Input.js @@ -1,7 +1,7 @@ import React from 'react' import propTypes from '@dhis2/prop-types' import cx from 'classnames' -import { colors, sharedPropTypes } from '@dhis2/ui-constants' +import { colors } from '@dhis2/ui-constants' import { SelectionList } from './SelectionList.js' import { InputPlaceholder } from '../Select/InputPlaceholder.js' import { InputPrefix } from '../Select/InputPrefix.js' @@ -101,7 +101,7 @@ Input.propTypes = { options: propTypes.node, placeholder: propTypes.string, prefix: propTypes.string, - selected: sharedPropTypes.multiSelectedPropType, + selected: propTypes.arrayOf(propTypes.string), onChange: propTypes.func, } diff --git a/packages/widgets/src/MultiSelect/Menu.js b/packages/widgets/src/MultiSelect/Menu.js index efb3dae4f7..ada2eb6fcd 100644 --- a/packages/widgets/src/MultiSelect/Menu.js +++ b/packages/widgets/src/MultiSelect/Menu.js @@ -1,6 +1,5 @@ import React from 'react' import propTypes from '@dhis2/prop-types' -import { sharedPropTypes } from '@dhis2/ui-constants' import { Empty } from '../Select/Empty.js' import { filterIgnored, @@ -14,17 +13,13 @@ const onDisabledClick = (_, e) => { e.preventDefault() } -const createHandler = ({ isActive, onChange, selected, value, label }) => ( - _, - e -) => { - const clickedOption = { value, label } +const createHandler = ({ isActive, onChange, selected, value }) => (_, e) => { e.stopPropagation() e.preventDefault() // If the option is currently selected remove it from the array of selected options if (isActive) { - const filtered = removeOption(clickedOption, selected) + const filtered = removeOption(value, selected) const data = { selected: filtered } return onChange(data, e) @@ -32,7 +27,7 @@ const createHandler = ({ isActive, onChange, selected, value, label }) => ( // Otherwise, add it to selected const data = { - selected: selected.concat([clickedOption]), + selected: selected.concat([value]), } return onChange(data, e) } @@ -61,7 +56,7 @@ const Menu = ({ options, onChange, selected, empty, dataTest }) => { const { value, label, disabled: isDisabled } = child.props // Active means the option is currently selected - const isActive = !!findOption({ value, label }, selected) + const isActive = !!findOption(value, selected) // Create the appropriate click handler for the option const onClick = isDisabled @@ -92,7 +87,7 @@ Menu.propTypes = { dataTest: propTypes.string.isRequired, empty: propTypes.node, options: propTypes.node, - selected: sharedPropTypes.multiSelectedPropType, + selected: propTypes.arrayOf(propTypes.string), onChange: propTypes.func, } diff --git a/packages/widgets/src/MultiSelect/MultiSelect.js b/packages/widgets/src/MultiSelect/MultiSelect.js index 06100c3fd7..05606ea55d 100644 --- a/packages/widgets/src/MultiSelect/MultiSelect.js +++ b/packages/widgets/src/MultiSelect/MultiSelect.js @@ -126,7 +126,7 @@ MultiSelect.defaultProps = { * @static * * @prop {function} [onChange] - * @prop {Array} [selected] + * @prop {Array.} [selected] * @prop {string} [className] * @prop {string} [tabIndex] * @prop {Node} [children] @@ -175,7 +175,7 @@ MultiSelect.propTypes = { ), placeholder: propTypes.string, prefix: propTypes.string, - selected: sharedPropTypes.multiSelectedPropType, + selected: propTypes.arrayOf(propTypes.string), tabIndex: propTypes.string, valid: sharedPropTypes.statusPropType, warning: sharedPropTypes.statusPropType, diff --git a/packages/widgets/src/MultiSelect/MultiSelect.stories.e2e.js b/packages/widgets/src/MultiSelect/MultiSelect.stories.e2e.js index 691868ecaa..1ab25ee717 100644 --- a/packages/widgets/src/MultiSelect/MultiSelect.stories.e2e.js +++ b/packages/widgets/src/MultiSelect/MultiSelect.stories.e2e.js @@ -137,11 +137,7 @@ storiesOf('MultiSelect', module) )) .add('With options, a selection and disabled', () => ( - + @@ -162,11 +158,7 @@ storiesOf('MultiSelect', module) )) .add('With prefix and selection', () => ( - + @@ -182,7 +174,7 @@ storiesOf('MultiSelect', module) .add('With placeholder and selection', () => ( @@ -199,10 +191,7 @@ storiesOf('MultiSelect', module) )) .add('With options and a selection', () => ( - + @@ -211,7 +200,7 @@ storiesOf('MultiSelect', module) .add('With options, a selection and onChange', () => ( @@ -220,13 +209,7 @@ storiesOf('MultiSelect', module) )) .add('With options and multiple selections', () => ( - + @@ -237,7 +220,7 @@ storiesOf('MultiSelect', module) clearable clearText="Clear" className="select" - selected={[{ value: '1', label: 'option one' }]} + selected={['1']} onChange={window.onChange} > @@ -335,3 +318,11 @@ storiesOf('MultiSelect', module) `} )) + .add('With duplicate selected option values', () => ( + + + + + + + )) diff --git a/packages/widgets/src/MultiSelect/MultiSelect.stories.js b/packages/widgets/src/MultiSelect/MultiSelect.stories.js index ffdb8098b6..1c68671ad0 100644 --- a/packages/widgets/src/MultiSelect/MultiSelect.stories.js +++ b/packages/widgets/src/MultiSelect/MultiSelect.stories.js @@ -135,11 +135,7 @@ storiesOf('MultiSelect', module) )) .add('With options, a selection and disabled', () => ( - + @@ -160,11 +156,7 @@ storiesOf('MultiSelect', module) )) .add('With prefix and selection', () => ( - + @@ -180,7 +172,7 @@ storiesOf('MultiSelect', module) .add('With placeholder and selection', () => ( @@ -197,10 +189,7 @@ storiesOf('MultiSelect', module) )) .add('With options and a selection', () => ( - + @@ -209,7 +198,7 @@ storiesOf('MultiSelect', module) .add('With options, a selection and onChange', () => ( @@ -218,13 +207,7 @@ storiesOf('MultiSelect', module) )) .add('With options and multiple selections', () => ( - + @@ -235,7 +218,7 @@ storiesOf('MultiSelect', module) clearable clearText="Clear" className="select" - selected={[{ value: '1', label: 'option one' }]} + selected={['1']} onChange={window.onChange} > diff --git a/packages/widgets/src/MultiSelect/SelectionList.js b/packages/widgets/src/MultiSelect/SelectionList.js index 8ef79d8cb4..d2e32bce6d 100644 --- a/packages/widgets/src/MultiSelect/SelectionList.js +++ b/packages/widgets/src/MultiSelect/SelectionList.js @@ -1,15 +1,10 @@ import React from 'react' import propTypes from '@dhis2/prop-types' -import { sharedPropTypes } from '@dhis2/ui-constants' import { Chip } from '../Chip/Chip.js' import { removeOption, findOptionChild } from '../Select/option-helpers.js' -const createRemoveHandler = ({ selected, onChange, value, label }) => ( - _, - e -) => { - const clickedOption = { value, label } - const filtered = removeOption(clickedOption, selected) +const createRemoveHandler = ({ selected, onChange, value }) => (_, e) => { + const filtered = removeOption(value, selected) const data = { selected: filtered } onChange(data, e) @@ -17,14 +12,14 @@ const createRemoveHandler = ({ selected, onChange, value, label }) => ( const SelectionList = ({ selected, onChange, disabled, options }) => ( - {selected.map(({ value, label }) => { - const selectedOption = findOptionChild({ value, label }, options) + {selected.map(value => { + const selectedOption = findOptionChild(value, options) if (!selectedOption) { const message = - 'The selected option could not be found as a child of the select. ' + - 'Make sure that the value and label for all options passed to the ' + - '`selected` prop matches an existing option.' + `There is no option with the value: "${value}". ` + + 'Make sure that all the values passed to the selected ' + + 'prop match the value of an existing option.' throw new Error(message) } @@ -38,17 +33,16 @@ const SelectionList = ({ selected, onChange, disabled, options }) => ( selected, onChange, value, - label, }) return ( - {label} + {selectedOption.props.label} ) })} @@ -58,7 +52,7 @@ const SelectionList = ({ selected, onChange, disabled, options }) => ( SelectionList.propTypes = { disabled: propTypes.bool, options: propTypes.node, - selected: sharedPropTypes.multiSelectedPropType, + selected: propTypes.arrayOf(propTypes.string), onChange: propTypes.func, } diff --git a/packages/widgets/src/MultiSelectField/MultiSelectField.js b/packages/widgets/src/MultiSelectField/MultiSelectField.js index 8177817ce6..b38ff1d2fb 100644 --- a/packages/widgets/src/MultiSelectField/MultiSelectField.js +++ b/packages/widgets/src/MultiSelectField/MultiSelectField.js @@ -119,7 +119,7 @@ MultiSelectField.defaultProps = { * * @prop {function} [onChange] * @prop {string} [label] - * @prop {Array} [selected] + * @prop {Array.} [selected] * @prop {string} [className] * @prop {string} [tabIndex] * @prop {Node} [children] @@ -176,7 +176,7 @@ MultiSelectField.propTypes = { placeholder: propTypes.string, prefix: propTypes.string, required: propTypes.bool, - selected: sharedPropTypes.multiSelectedPropType, + selected: propTypes.arrayOf(propTypes.string), tabIndex: propTypes.string, valid: sharedPropTypes.statusPropType, validationText: propTypes.string, diff --git a/packages/widgets/src/MultiSelectField/MultiSelectField.stories.js b/packages/widgets/src/MultiSelectField/MultiSelectField.stories.js index 22dfc89a02..93fefb8475 100644 --- a/packages/widgets/src/MultiSelectField/MultiSelectField.stories.js +++ b/packages/widgets/src/MultiSelectField/MultiSelectField.stories.js @@ -7,7 +7,7 @@ import { MultiSelectField } from './MultiSelectField.js' const defaultProps = { label: 'Default label', - selected: [{ value: '1', label: 'one' }], + selected: ['1'], onChange: selected => alert(`Selected changed to: ${JSON.stringify(selected, null, 2)}`), } diff --git a/packages/widgets/src/Select/FilterableMenu.js b/packages/widgets/src/Select/FilterableMenu.js index bdfa0c0b69..aecf7dbe9d 100644 --- a/packages/widgets/src/Select/FilterableMenu.js +++ b/packages/widgets/src/Select/FilterableMenu.js @@ -1,6 +1,5 @@ import React, { Component } from 'react' import propTypes from '@dhis2/prop-types' -import { sharedPropTypes } from '@dhis2/ui-constants' import { FilterInput } from '../Select/FilterInput.js' import { NoMatch } from '../Select/NoMatch.js' import { filterIgnored, checkIfValidOption } from '../Select/option-helpers.js' @@ -95,8 +94,8 @@ FilterableMenu.propTypes = { dataTest: propTypes.string.isRequired, noMatchText: propTypes.string.isRequired, selected: propTypes.oneOfType([ - sharedPropTypes.singleSelectedPropType, - sharedPropTypes.multiSelectedPropType, + propTypes.string, + propTypes.arrayOf(propTypes.string), ]).isRequired, empty: propTypes.node, handleClose: propTypes.func, diff --git a/packages/widgets/src/Select/Select.js b/packages/widgets/src/Select/Select.js index d728a4f604..9a82f5f06e 100644 --- a/packages/widgets/src/Select/Select.js +++ b/packages/widgets/src/Select/Select.js @@ -116,16 +116,10 @@ export class Select extends Component { dataTest, } = this.props - // We need to update the menu's position on selection because - // that can cause the input area to change size - const handleChange = (data, e) => { - onChange(data, e) - } - // Create the input const inputProps = { selected, - onChange: handleChange, + onChange, options: children, disabled, } @@ -134,7 +128,7 @@ export class Select extends Component { // Create the menu const menuProps = { selected, - onChange: handleChange, + onChange, options: children, handleClose: this.handleClose, handleFocusInput: this.handleFocusInput, @@ -186,8 +180,8 @@ Select.propTypes = { input: propTypes.element.isRequired, menu: propTypes.element.isRequired, selected: propTypes.oneOfType([ - sharedPropTypes.singleSelectedPropType, - sharedPropTypes.multiSelectedPropType, + propTypes.string, + propTypes.arrayOf(propTypes.string), ]).isRequired, children: propTypes.node, className: propTypes.string, diff --git a/packages/widgets/src/Select/option-helpers.js b/packages/widgets/src/Select/option-helpers.js index 14e9acb01f..ad8fcc0987 100644 --- a/packages/widgets/src/Select/option-helpers.js +++ b/packages/widgets/src/Select/option-helpers.js @@ -14,32 +14,21 @@ export const filterIgnored = children => ) // Find an option in an array of react children -export const findOptionChild = (targetOption, optionChildren) => +export const findOptionChild = (value, optionChildren) => React.Children.toArray(optionChildren).find(currentOption => { if (!currentOption.props) { return false } - const matchesLabel = targetOption.label === currentOption.props.label - const matchesValue = targetOption.value === currentOption.props.value - - return matchesLabel && matchesValue + return value === currentOption.props.value }) // Find an option in an array of option objects -export const findOption = (targetOption, optionArray) => - optionArray.find(currentOption => { - const matchesLabel = targetOption.label === currentOption.label - const matchesValue = targetOption.value === currentOption.value - - return matchesLabel && matchesValue - }) +export const findOption = (value, optionArray) => + optionArray.find(optionValue => value === optionValue) // Remove a specific option from an array of options -export const removeOption = (targetOption, optionArray) => - optionArray.filter(currentOption => { - const matchesLabel = targetOption.label === currentOption.label - const matchesValue = targetOption.value === currentOption.value - - return !matchesLabel && !matchesValue +export const removeOption = (value, optionArray) => + optionArray.filter(optionValue => { + return optionValue !== value }) diff --git a/packages/widgets/src/SingleSelect/FilterableMenu.js b/packages/widgets/src/SingleSelect/FilterableMenu.js index e383bc54de..9b336485d7 100644 --- a/packages/widgets/src/SingleSelect/FilterableMenu.js +++ b/packages/widgets/src/SingleSelect/FilterableMenu.js @@ -1,6 +1,5 @@ import React from 'react' import propTypes from '@dhis2/prop-types' -import { sharedPropTypes } from '@dhis2/ui-constants' import { FilterableMenu as CommonFilterableMenu } from '../Select/FilterableMenu.js' import { Menu } from './Menu.js' @@ -37,7 +36,7 @@ FilterableMenu.propTypes = { handleFocusInput: propTypes.func, options: propTypes.node, placeholder: propTypes.string, - selected: sharedPropTypes.singleSelectedPropType, + selected: propTypes.string, onChange: propTypes.func, } diff --git a/packages/widgets/src/SingleSelect/Input.js b/packages/widgets/src/SingleSelect/Input.js index 91542eb2d6..3bacb67e58 100644 --- a/packages/widgets/src/SingleSelect/Input.js +++ b/packages/widgets/src/SingleSelect/Input.js @@ -1,7 +1,7 @@ import React from 'react' import propTypes from '@dhis2/prop-types' import cx from 'classnames' -import { colors, sharedPropTypes } from '@dhis2/ui-constants' +import { colors } from '@dhis2/ui-constants' import { Selection } from './Selection.js' import { InputPlaceholder } from '../Select/InputPlaceholder.js' import { InputPrefix } from '../Select/InputPrefix.js' @@ -20,9 +20,9 @@ const Input = ({ disabled, inputMaxHeight, }) => { - const hasSelection = 'label' in selected && 'value' in selected + const hasSelection = selected && typeof selected === 'string' const onClear = (_, e) => { - const data = { selected: {} } + const data = { selected: '' } e.stopPropagation() onChange(data, e) @@ -96,7 +96,7 @@ Input.propTypes = { options: propTypes.node, placeholder: propTypes.string, prefix: propTypes.string, - selected: sharedPropTypes.singleSelectedPropType, + selected: propTypes.string, onChange: propTypes.func, } diff --git a/packages/widgets/src/SingleSelect/Menu.js b/packages/widgets/src/SingleSelect/Menu.js index 8a7e806f80..4c9198242e 100644 --- a/packages/widgets/src/SingleSelect/Menu.js +++ b/packages/widgets/src/SingleSelect/Menu.js @@ -1,6 +1,5 @@ import React from 'react' import propTypes from '@dhis2/prop-types' -import { sharedPropTypes } from '@dhis2/ui-constants' import { Empty } from '../Select/Empty.js' import { filterIgnored, checkIfValidOption } from '../Select/option-helpers.js' @@ -38,12 +37,12 @@ const Menu = ({ return child } - const { value, label, disabled: isDisabled } = child.props + const { value, disabled: isDisabled } = child.props // Active means the option is currently selected - const isActive = value === selected.value && label === selected.label + const isActive = value === selected const onClick = (_, e) => { - const data = { selected: { value, label } } + const data = { selected: value } e.stopPropagation() onChange(data, e) @@ -74,7 +73,7 @@ Menu.propTypes = { handleClose: propTypes.func, handleFocusInput: propTypes.func, options: propTypes.node, - selected: sharedPropTypes.singleSelectedPropType, + selected: propTypes.string, onChange: propTypes.func, } diff --git a/packages/widgets/src/SingleSelect/Selection.js b/packages/widgets/src/SingleSelect/Selection.js index af44182132..9369bd5982 100644 --- a/packages/widgets/src/SingleSelect/Selection.js +++ b/packages/widgets/src/SingleSelect/Selection.js @@ -1,7 +1,7 @@ import React from 'react' import propTypes from '@dhis2/prop-types' import cx from 'classnames' -import { spacers, sharedPropTypes } from '@dhis2/ui-constants' +import { spacers } from '@dhis2/ui-constants' import { findOptionChild } from '../Select/option-helpers.js' const Selection = ({ options, selected, className }) => { @@ -9,9 +9,9 @@ const Selection = ({ options, selected, className }) => { if (!selectedOption) { const message = - 'The selected option could not be found as a child of the select. ' + - 'Make sure that the value and label passed to the `selected` prop ' + - 'match an existing option.' + `There is no option with the value: "${selected}". ` + + 'Make sure that the value passed to the selected ' + + 'prop matches the value of an existing option.' throw new Error(message) } @@ -44,7 +44,7 @@ const Selection = ({ options, selected, className }) => { Selection.propTypes = { className: propTypes.string, options: propTypes.node, - selected: sharedPropTypes.singleSelectedPropType, + selected: propTypes.string, } export { Selection } diff --git a/packages/widgets/src/SingleSelect/SingleSelect.js b/packages/widgets/src/SingleSelect/SingleSelect.js index 335979d41d..2c0bae0ae7 100644 --- a/packages/widgets/src/SingleSelect/SingleSelect.js +++ b/packages/widgets/src/SingleSelect/SingleSelect.js @@ -118,7 +118,7 @@ const SingleSelect = ({ } SingleSelect.defaultProps = { - selected: {}, + selected: '', dataTest: 'dhis2-uicore-singleselect', } @@ -127,7 +127,7 @@ SingleSelect.defaultProps = { * @static * * @prop {function} [onChange] - * @prop {Object} [selected] + * @prop {String} [selected] * @prop {string} [className] * @prop {string} [tabIndex] * @prop {Node} [children] @@ -176,7 +176,7 @@ SingleSelect.propTypes = { ), placeholder: propTypes.string, prefix: propTypes.string, - selected: sharedPropTypes.singleSelectedPropType, + selected: propTypes.string, tabIndex: propTypes.string, valid: sharedPropTypes.statusPropType, warning: sharedPropTypes.statusPropType, diff --git a/packages/widgets/src/SingleSelect/SingleSelect.stories.e2e.js b/packages/widgets/src/SingleSelect/SingleSelect.stories.e2e.js index a7849125c5..a476351d22 100644 --- a/packages/widgets/src/SingleSelect/SingleSelect.stories.e2e.js +++ b/packages/widgets/src/SingleSelect/SingleSelect.stories.e2e.js @@ -136,11 +136,7 @@ storiesOf('SingleSelect', module) )) .add('With options, a selection and disabled', () => ( - + @@ -161,11 +157,7 @@ storiesOf('SingleSelect', module) )) .add('With prefix and selection', () => ( - + @@ -181,7 +173,7 @@ storiesOf('SingleSelect', module) .add('With placeholder and selection', () => ( @@ -198,10 +190,7 @@ storiesOf('SingleSelect', module) )) .add('With options and a selection', () => ( - + @@ -210,7 +199,7 @@ storiesOf('SingleSelect', module) .add('With options, a selection and onChange', () => ( @@ -223,7 +212,7 @@ storiesOf('SingleSelect', module) clearable clearText="Clear" className="select" - selected={{ value: '1', label: 'option one' }} + selected="1" onChange={window.onChange} > @@ -325,3 +314,11 @@ storiesOf('SingleSelect', module) `} )) + .add('With duplicate selected option values', () => ( + + + + + + + )) diff --git a/packages/widgets/src/SingleSelect/SingleSelect.stories.js b/packages/widgets/src/SingleSelect/SingleSelect.stories.js index ba172fb2e7..347ae67b04 100644 --- a/packages/widgets/src/SingleSelect/SingleSelect.stories.js +++ b/packages/widgets/src/SingleSelect/SingleSelect.stories.js @@ -137,11 +137,7 @@ storiesOf('SingleSelect', module) )) .add('With options, a selection and disabled', () => ( - + @@ -162,11 +158,7 @@ storiesOf('SingleSelect', module) )) .add('With prefix and selection', () => ( - + @@ -182,7 +174,7 @@ storiesOf('SingleSelect', module) .add('With placeholder and selection', () => ( @@ -199,10 +191,7 @@ storiesOf('SingleSelect', module) )) .add('With options and a selection', () => ( - + @@ -211,7 +200,7 @@ storiesOf('SingleSelect', module) .add('With options, a selection and onChange', () => ( @@ -224,7 +213,7 @@ storiesOf('SingleSelect', module) clearable clearText="Clear" className="select" - selected={{ value: '1', label: 'option one' }} + selected="1" onChange={window.onChange} > diff --git a/packages/widgets/src/SingleSelectField/SingleSelectField.js b/packages/widgets/src/SingleSelectField/SingleSelectField.js index 2ddddb0f37..6f57755708 100644 --- a/packages/widgets/src/SingleSelectField/SingleSelectField.js +++ b/packages/widgets/src/SingleSelectField/SingleSelectField.js @@ -109,7 +109,7 @@ class SingleSelectField extends React.Component { } SingleSelectField.defaultProps = { - selected: {}, + selected: '', dataTest: 'dhis2-uicore-singleselectfield', } @@ -119,7 +119,7 @@ SingleSelectField.defaultProps = { * * @prop {function} [onChange] * @prop {string} label - * @prop {Object} [selected] + * @prop {string} [selected] * @prop {string} [className] * @prop {string} [tabIndex] * @prop {Node} [children] @@ -176,10 +176,7 @@ SingleSelectField.propTypes = { placeholder: propTypes.string, prefix: propTypes.string, required: propTypes.bool, - selected: propTypes.shape({ - label: propTypes.string, - value: propTypes.string, - }), + selected: propTypes.string, tabIndex: propTypes.string, valid: sharedPropTypes.statusPropType, validationText: propTypes.string, diff --git a/packages/widgets/src/SingleSelectField/SingleSelectField.stories.js b/packages/widgets/src/SingleSelectField/SingleSelectField.stories.js index 2a438008ad..888cbb13d0 100644 --- a/packages/widgets/src/SingleSelectField/SingleSelectField.stories.js +++ b/packages/widgets/src/SingleSelectField/SingleSelectField.stories.js @@ -6,7 +6,7 @@ import { SingleSelectField } from './SingleSelectField.js' const defaultProps = { label: 'Default label', - selected: { value: '1', label: 'one' }, + selected: '1', onChange: selected => alert(`Selected changed to: ${JSON.stringify(selected, null, 2)}`), }