From 6cf6c05b12076ffb9a517ce8dc7eece96b57abf7 Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Fri, 31 Jul 2020 10:05:00 -0700 Subject: [PATCH 1/5] timeout between addCustomOption calls --- src-docs/src/views/combo_box/combo_box_delimiter.js | 5 +++-- src/components/combo_box/combo_box.tsx | 10 ++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src-docs/src/views/combo_box/combo_box_delimiter.js b/src-docs/src/views/combo_box/combo_box_delimiter.js index c5b011cff2a..76bb2d76cb3 100644 --- a/src-docs/src/views/combo_box/combo_box_delimiter.js +++ b/src-docs/src/views/combo_box/combo_box_delimiter.js @@ -2,7 +2,7 @@ import React, { useState } from 'react'; import { EuiComboBox } from '../../../../src/components'; -const options = [ +const staticOptions = [ { label: 'Titan', 'data-test-subj': 'titanOption', @@ -39,6 +39,7 @@ const options = [ ]; export default () => { + const [options, setOptions] = useState(staticOptions); const [selectedOptions, setSelected] = useState([options[2], options[4]]); const onChange = selectedOptions => { @@ -62,7 +63,7 @@ export default () => { option => option.label.trim().toLowerCase() === normalizedSearchValue ) === -1 ) { - options.push(newOption); + setOptions([...options, newOption]); } // Select the option. diff --git a/src/components/combo_box/combo_box.tsx b/src/components/combo_box/combo_box.tsx index 932e43feb39..c389a6ae5ea 100644 --- a/src/components/combo_box/combo_box.tsx +++ b/src/components/combo_box/combo_box.tsx @@ -535,7 +535,11 @@ export class EuiComboBox extends Component< const { delimiter } = this.props; if (delimiter) { searchValue.split(delimiter).forEach((option: string) => { - if (option.length > 0) this.addCustomOption(true, option); + if (option.length > 0) { + // Wait a tick between each `addCustomOption` call to avoid + // potential race condition that resets state between calls. + setTimeout(() => this.addCustomOption(isContainerBlur, option)); + } }); } else { this.addCustomOption(isContainerBlur, searchValue); @@ -769,9 +773,7 @@ export class EuiComboBox extends Component< if (searchValue && this.state.isListOpen === false) this.openList(); }); if (delimiter && searchValue.endsWith(delimiter)) { - searchValue.split(delimiter).forEach(value => { - if (value.length > 0) this.addCustomOption(false, value); - }); + this.setCustomOptions(false); } }; From fce0f2c5a769b3e3a0d8525a8c761f240cbda041 Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Fri, 31 Jul 2020 12:37:07 -0700 Subject: [PATCH 2/5] fix the docs example --- src-docs/src/views/combo_box/combo_box_delimiter.js | 4 +++- src/components/combo_box/combo_box.tsx | 6 +----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src-docs/src/views/combo_box/combo_box_delimiter.js b/src-docs/src/views/combo_box/combo_box_delimiter.js index 76bb2d76cb3..1a8536b2e14 100644 --- a/src-docs/src/views/combo_box/combo_box_delimiter.js +++ b/src-docs/src/views/combo_box/combo_box_delimiter.js @@ -67,7 +67,9 @@ export default () => { } // Select the option. - setSelected([...selectedOptions, newOption]); + // Use the previousState parameter (prevSelected) from the setState + // instance (setSelected) to ensure looped calls do not override each other + setSelected(prevSelected => [...prevSelected, newOption]); }; return ( diff --git a/src/components/combo_box/combo_box.tsx b/src/components/combo_box/combo_box.tsx index c389a6ae5ea..d2dfb57139e 100644 --- a/src/components/combo_box/combo_box.tsx +++ b/src/components/combo_box/combo_box.tsx @@ -535,11 +535,7 @@ export class EuiComboBox extends Component< const { delimiter } = this.props; if (delimiter) { searchValue.split(delimiter).forEach((option: string) => { - if (option.length > 0) { - // Wait a tick between each `addCustomOption` call to avoid - // potential race condition that resets state between calls. - setTimeout(() => this.addCustomOption(isContainerBlur, option)); - } + if (option.length > 0) this.addCustomOption(isContainerBlur, option); }); } else { this.addCustomOption(isContainerBlur, searchValue); From a6954fbecca930d520ec58b92273d3404995a943 Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Mon, 10 Aug 2020 11:20:17 -0600 Subject: [PATCH 3/5] delimeter empty state prompt --- .../combo_box_options_list.tsx | 84 +++++++++---------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/src/components/combo_box/combo_box_options_list/combo_box_options_list.tsx b/src/components/combo_box/combo_box_options_list/combo_box_options_list.tsx index 297d23a35d0..d5409ab1d7b 100644 --- a/src/components/combo_box/combo_box_options_list/combo_box_options_list.tsx +++ b/src/components/combo_box/combo_box_options_list/combo_box_options_list.tsx @@ -331,40 +331,6 @@ export class EuiComboBoxOptionsList extends Component< ); } else if (searchValue && matchingOptions && matchingOptions.length === 0) { if (onCreateOption && getSelectedOptionForSearchValue) { - const selectedOptionForValue = getSelectedOptionForSearchValue( - searchValue, - selectedOptions - ); - if (selectedOptionForValue) { - // Disallow duplicate custom options. - emptyStateContent = ( -

- {selectedOptionForValue.label}, - }} - /> -

- ); - } else { - emptyStateContent = ( -
-

- {searchValue}, - }} - /> -

- {hitEnterBadge} -
- ); - } - } else { if (delimiter && searchValue.includes(delimiter)) { emptyStateContent = (
@@ -379,16 +345,50 @@ export class EuiComboBoxOptionsList extends Component<
); } else { - emptyStateContent = ( -

- {searchValue} }} - /> -

+ const selectedOptionForValue = getSelectedOptionForSearchValue( + searchValue, + selectedOptions ); + if (selectedOptionForValue) { + // Disallow duplicate custom options. + emptyStateContent = ( +

+ {selectedOptionForValue.label}, + }} + /> +

+ ); + } else { + emptyStateContent = ( +
+

+ {searchValue}, + }} + /> +

+ {hitEnterBadge} +
+ ); + } } + } else { + emptyStateContent = ( +

+ {searchValue} }} + /> +

+ ); } } else if (!options.length) { emptyStateContent = ( From fd2635cb69a88e1e8f9e14c4f0db0dcd0c054b8c Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Mon, 10 Aug 2020 11:26:47 -0600 Subject: [PATCH 4/5] CL --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e64c2a0a42a..26d9afff165 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ **Bug fixes** - Fixed `EuiFacetGroup` container expansion due to negative margin value ([#3871](https://github.com/elastic/eui/pull/3871)) +- Fixed `EuiComboBox` delimeter-separated option creation and empty state prompt text ([#3841](https://github.com/elastic/eui/pull/3841)) **Breaking changes** From 9bc23758299dc02fa412122f870c7e82d0add03b Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Mon, 10 Aug 2020 14:26:41 -0600 Subject: [PATCH 5/5] keep changeable options in state --- src-docs/src/views/combo_box/async.js | 7 +++--- src-docs/src/views/combo_box/colors.js | 7 +++--- src-docs/src/views/combo_box/combo_box.js | 5 ++-- src-docs/src/views/combo_box/containers.js | 7 +++--- src-docs/src/views/combo_box/disabled.js | 2 +- src-docs/src/views/combo_box/groups.js | 24 ++++++++++++++----- src-docs/src/views/combo_box/render_option.js | 6 +++-- src-docs/src/views/combo_box/startingWith.js | 7 +++--- 8 files changed, 42 insertions(+), 23 deletions(-) diff --git a/src-docs/src/views/combo_box/async.js b/src-docs/src/views/combo_box/async.js index 49495c00d5c..ce657888891 100644 --- a/src-docs/src/views/combo_box/async.js +++ b/src-docs/src/views/combo_box/async.js @@ -2,7 +2,7 @@ import React, { useState, useEffect, useCallback } from 'react'; import { EuiComboBox } from '../../../../src/components'; -const allOptions = [ +const allOptionsStatic = [ { label: 'Titan', 'data-test-subj': 'titanOption', @@ -38,6 +38,7 @@ const allOptions = [ ]; export default () => { + const [allOptions, setAllOptions] = useState(allOptionsStatic); const [selectedOptions, setSelected] = useState([]); const [isLoading, setLoading] = useState(false); const [options, setOptions] = useState([]); @@ -82,12 +83,12 @@ export default () => { ) === -1 ) { // Simulate creating this option on the server. - allOptions.push(newOption); + setAllOptions([...allOptions, newOption]); setOptions([...options, newOption]); } // Select the option. - setSelected([...selectedOptions, newOption]); + setSelected(prevSelected => [...prevSelected, newOption]); }; useEffect(() => { diff --git a/src-docs/src/views/combo_box/colors.js b/src-docs/src/views/combo_box/colors.js index 045db52465a..4f876f845d1 100644 --- a/src-docs/src/views/combo_box/colors.js +++ b/src-docs/src/views/combo_box/colors.js @@ -4,7 +4,7 @@ import { EuiComboBox } from '../../../../src/components'; import { euiPaletteColorBlindBehindText } from '../../../../src/services'; const visColorsBehindText = euiPaletteColorBlindBehindText(); -const options = [ +const optionsStatic = [ { label: 'Titan', 'data-test-subj': 'titanOption', @@ -50,6 +50,7 @@ const options = [ ]; export default () => { + const [options, setOptions] = useState(optionsStatic); const [selectedOptions, setSelected] = useState([options[2], options[5]]); const onChange = selectedOptions => { @@ -77,11 +78,11 @@ export default () => { option => option.label.trim().toLowerCase() === normalizedSearchValue ) === -1 ) { - options.push(newOption); + setOptions([...options, newOption]); } // Select the option. - setSelected([...selectedOptions, newOption]); + setSelected(prevSelected => [...prevSelected, newOption]); }; return ( diff --git a/src-docs/src/views/combo_box/combo_box.js b/src-docs/src/views/combo_box/combo_box.js index 323ee8f5cce..08d8a2aeaee 100644 --- a/src-docs/src/views/combo_box/combo_box.js +++ b/src-docs/src/views/combo_box/combo_box.js @@ -3,7 +3,7 @@ import React, { useState } from 'react'; import { EuiComboBox } from '../../../../src/components'; import { DisplayToggles } from '../form_controls/display_toggles'; -const options = [ +const optionsStatic = [ { label: 'Titan', 'data-test-subj': 'titanOption', @@ -39,6 +39,7 @@ const options = [ }, ]; export default () => { + const [options, setOptions] = useState(optionsStatic); const [selectedOptions, setSelected] = useState([options[2], options[4]]); const onChange = selectedOptions => { @@ -62,7 +63,7 @@ export default () => { option => option.label.trim().toLowerCase() === normalizedSearchValue ) === -1 ) { - options.push(newOption); + setOptions([...options, newOption]); } // Select the option. diff --git a/src-docs/src/views/combo_box/containers.js b/src-docs/src/views/combo_box/containers.js index c28693ff944..0f84e859fea 100644 --- a/src-docs/src/views/combo_box/containers.js +++ b/src-docs/src/views/combo_box/containers.js @@ -13,7 +13,7 @@ import { EuiSpacer, } from '../../../../src/components'; -const options = [ +const optionsStatic = [ { label: 'Titan', 'data-test-subj': 'titanOption', @@ -49,6 +49,7 @@ const options = [ ]; export default () => { + const [options, setOptions] = useState(optionsStatic); const [selectedOptions, setSelected] = useState([options[2], options[4]]); const [isModalVisible, setModalVisible] = useState(false); const [isPopoverOpen, setPopover] = useState(false); @@ -94,11 +95,11 @@ export default () => { option => option.label.trim().toLowerCase() === normalizedSearchValue ) === -1 ) { - options.push(newOption); + setOptions([...options, newOption]); } // Select the option. - setSelected([...selectedOptions, newOption]); + setSelected(prevSelected => [...prevSelected, newOption]); }; const comboBox = ( diff --git a/src-docs/src/views/combo_box/disabled.js b/src-docs/src/views/combo_box/disabled.js index 37fe809ff11..66d94b65749 100644 --- a/src-docs/src/views/combo_box/disabled.js +++ b/src-docs/src/views/combo_box/disabled.js @@ -66,7 +66,7 @@ export default () => { } // Select the option. - setSelected([...selectedOptions, newOption]); + setSelected(prevSelected => [...prevSelected, newOption]); }; return ( diff --git a/src-docs/src/views/combo_box/groups.js b/src-docs/src/views/combo_box/groups.js index 469a6e74179..d1d12bcc6e7 100644 --- a/src-docs/src/views/combo_box/groups.js +++ b/src-docs/src/views/combo_box/groups.js @@ -38,9 +38,10 @@ const soundGroup = { ], }; -const allOptions = [colorGroup, soundGroup]; +const allOptionsStatic = [colorGroup, soundGroup]; export default () => { + const [allOptions, setAllOptions] = useState(allOptionsStatic); const [selectedOptions, setSelected] = useState([ colorGroup.options[2], soundGroup.options[3], @@ -72,12 +73,23 @@ export default () => { ) === -1 ) { if (allOptions[allOptions.length - 1].label !== 'Custom') { - allOptions.push({ - label: 'Custom', - options: [], - }); + setAllOptions([ + ...allOptions, + { + label: 'Custom', + options: [], + }, + ]); } - allOptions[allOptions.length - 1].options.push(newOption); + const [colors, sounds, custom] = allOptions; + setAllOptions([ + colors, + sounds, + { + ...custom, + options: [...custom.options, newOption], + }, + ]); } // Select the option. diff --git a/src-docs/src/views/combo_box/render_option.js b/src-docs/src/views/combo_box/render_option.js index e980ae692c8..28cb9cf1e5f 100644 --- a/src-docs/src/views/combo_box/render_option.js +++ b/src-docs/src/views/combo_box/render_option.js @@ -12,7 +12,7 @@ import { const visColors = euiPaletteColorBlind(); const visColorsBehindText = euiPaletteColorBlindBehindText(); -const options = [ +const optionsStatic = [ { value: { size: 5, @@ -88,6 +88,7 @@ const options = [ ]; export default () => { + const [options, setOptions] = useState(optionsStatic); const [selectedOptions, setSelected] = useState([options[2], options[5]]); const onChange = selectedOptions => { @@ -117,10 +118,11 @@ export default () => { ) === -1 ) { options.push(newOption); + setOptions([...options, newOption]); } // Select the option. - setSelected([...selectedOptions, newOption]); + setSelected(prevSelected => [...prevSelected, newOption]); }; const renderOption = (option, searchValue, contentClassName) => { diff --git a/src-docs/src/views/combo_box/startingWith.js b/src-docs/src/views/combo_box/startingWith.js index 026b1160f9a..9e40ddc92f5 100644 --- a/src-docs/src/views/combo_box/startingWith.js +++ b/src-docs/src/views/combo_box/startingWith.js @@ -2,7 +2,7 @@ import React, { useState } from 'react'; import { EuiComboBox } from '../../../../src/components'; -const options = [ +const optionsStatic = [ { label: 'Titan', 'data-test-subj': 'titanOption', @@ -39,6 +39,7 @@ const options = [ ]; export default () => { + const [options, setOptions] = useState(optionsStatic); const [selectedOptions, setSelected] = useState([options[2], options[4]]); const onChange = selectedOptions => { @@ -62,11 +63,11 @@ export default () => { option => option.label.trim().toLowerCase() === normalizedSearchValue ) === -1 ) { - options.push(newOption); + setOptions([...options, newOption]); } // Select the option. - setSelected([...selectedOptions, newOption]); + setSelected(prevSelected => [...prevSelected, newOption]); }; return (