From 8630d3007eab9a4be8334ab1ec7e40224fdd324a Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Thu, 4 Apr 2019 09:11:39 -0600 Subject: [PATCH 1/7] Update documentation around EuiComboBox's invalid non-custom option, fixed a related UX bug --- .../combo_box/disallow_custom_options.js | 34 +++++++++++++++---- .../__snapshots__/combo_box.test.js.snap | 7 ---- src/components/combo_box/combo_box.js | 9 +++-- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/src-docs/src/views/combo_box/disallow_custom_options.js b/src-docs/src/views/combo_box/disallow_custom_options.js index f3448ac5c0e..b43aa586766 100644 --- a/src-docs/src/views/combo_box/disallow_custom_options.js +++ b/src-docs/src/views/combo_box/disallow_custom_options.js @@ -2,6 +2,7 @@ import React, { Component } from 'react'; import { EuiComboBox, + EuiFormRow, } from '../../../../src/components'; export default class extends Component { @@ -32,6 +33,7 @@ export default class extends Component { }]; this.state = { + error: undefined, selectedOptions: [this.options[2], this.options[4]], }; } @@ -40,16 +42,34 @@ export default class extends Component { this.setState({ selectedOptions, }); - }; + } + + onBlur = () => { + const { value } = this.inputRef; + if (value.length !== 0) { + this.setState({ + error: `"${value}" is not a valid option`, + }); + } + } + + clearError = () => this.setState({ error: undefined }); + + setInputRef = ref => this.inputRef = ref; render() { return ( - + + + ); } } diff --git a/src/components/combo_box/__snapshots__/combo_box.test.js.snap b/src/components/combo_box/__snapshots__/combo_box.test.js.snap index d81d523c000..87119129568 100644 --- a/src/components/combo_box/__snapshots__/combo_box.test.js.snap +++ b/src/components/combo_box/__snapshots__/combo_box.test.js.snap @@ -89,7 +89,6 @@ exports[`props full width is rendered 1`] = ` inputRef={[Function]} isListOpen={false} noIcon={false} - onBlur={[Function]} onChange={[Function]} onClear={[Function]} onClick={[Function]} @@ -131,7 +130,6 @@ exports[`props isClearable=false disallows user from clearing input when no opti inputRef={[Function]} isListOpen={false} noIcon={false} - onBlur={[Function]} onChange={[Function]} onClick={[Function]} onCloseListClick={[Function]} @@ -166,7 +164,6 @@ exports[`props isClearable=false disallows user from clearing input when options inputRef={[Function]} isListOpen={false} noIcon={false} - onBlur={[Function]} onChange={[Function]} onClick={[Function]} onCloseListClick={[Function]} @@ -211,7 +208,6 @@ exports[`props isDisabled is rendered 1`] = ` isDisabled={true} isListOpen={false} noIcon={false} - onBlur={[Function]} onChange={[Function]} onClick={[Function]} onCloseListClick={[Function]} @@ -342,7 +338,6 @@ exports[`props selectedOptions are rendered 1`] = ` inputRef={[Function]} isListOpen={false} noIcon={false} - onBlur={[Function]} onChange={[Function]} onClear={[Function]} onClick={[Function]} @@ -387,7 +382,6 @@ exports[`props singleSelection is rendered 1`] = ` inputRef={[Function]} isListOpen={false} noIcon={false} - onBlur={[Function]} onChange={[Function]} onClear={[Function]} onClick={[Function]} @@ -429,7 +423,6 @@ exports[`props singleSelection selects existing option when opened 1`] = ` inputRef={[Function]} isListOpen={true} noIcon={false} - onBlur={[Function]} onChange={[Function]} onClear={[Function]} onClick={[Function]} diff --git a/src/components/combo_box/combo_box.js b/src/components/combo_box/combo_box.js index da0003aacb3..d69d3484b1a 100644 --- a/src/components/combo_box/combo_box.js +++ b/src/components/combo_box/combo_box.js @@ -290,6 +290,7 @@ export class EuiComboBox extends Component { if (this.props.onBlur) { this.props.onBlur(); } + this.setState({ hasFocus: false }); // If the user tabs away or changes focus to another element, take whatever input they've // typed and convert it into a pill, to prevent the combo box from looking like a text input. @@ -299,10 +300,6 @@ export class EuiComboBox extends Component { } } - onComboBoxBlur = () => { - this.setState({ hasFocus: false }); - } - onKeyDown = (e) => { switch (e.keyCode) { case comboBoxKeyCodes.UP: @@ -573,6 +570,7 @@ export class EuiComboBox extends Component { onSearchChange, // eslint-disable-line no-unused-vars async, // eslint-disable-line no-unused-vars onBlur, // eslint-disable-line no-unused-vars + inputRef, // eslint-disable-line no-unused-vars isInvalid, rowHeight, isClearable, @@ -583,6 +581,8 @@ export class EuiComboBox extends Component { } = this.props; const { hasFocus, searchValue, isListOpen, listPosition, width, activeOptionIndex } = this.state; + // Visually indicate the combobox is in an invalid state if it has lost focus but there is text entered in the input + // this can occur when custom options are disabled and the user leaves the combo box after entering text that does not match any options const markAsInvalid = isInvalid || (hasFocus === false && searchValue); const classes = classNames('euiComboBox', className, { @@ -646,7 +646,6 @@ export class EuiComboBox extends Component { placeholder={placeholder} selectedOptions={selectedOptions} onRemoveOption={this.onRemoveOption} - onBlur={this.onComboBoxBlur} onClick={this.onComboBoxClick} onChange={this.onSearchChange} onFocus={this.onComboBoxFocus} From b98cb0d34128288c557068f066c108e8e39ee98b Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Thu, 4 Apr 2019 09:20:18 -0600 Subject: [PATCH 2/7] changelog --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed10f99450b..98ff5eadf8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ ## [`master`](https://github.com/elastic/eui/tree/master) -No public interface changes since `9.8.0`. +**Bug fixes** + +- Fixed EuiComboBox's internal tracking of its focus state ([#1796](https://github.com/elastic/eui/pull/1796)) ## [`9.8.0`](https://github.com/elastic/eui/tree/v9.8.0) From 5b4bf02126b887cb3d3ba4d3671f256430e033ac Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Thu, 4 Apr 2019 11:44:03 -0600 Subject: [PATCH 3/7] PR feedback --- src/components/combo_box/combo_box.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/combo_box/combo_box.js b/src/components/combo_box/combo_box.js index d69d3484b1a..3b6e88f1aac 100644 --- a/src/components/combo_box/combo_box.js +++ b/src/components/combo_box/combo_box.js @@ -581,8 +581,9 @@ export class EuiComboBox extends Component { } = this.props; const { hasFocus, searchValue, isListOpen, listPosition, width, activeOptionIndex } = this.state; - // Visually indicate the combobox is in an invalid state if it has lost focus but there is text entered in the input - // this can occur when custom options are disabled and the user leaves the combo box after entering text that does not match any options + // Visually indicate the combobox is in an invalid state if it has lost focus but there is text entered in the input. + // When custom options are disabled and the user leaves the combo box after entering text that does not match any + // options, this tells the user that they've entered invalid input. const markAsInvalid = isInvalid || (hasFocus === false && searchValue); const classes = classNames('euiComboBox', className, { From ab13dd72226363f26fff7179fe2427cc879beaf3 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Fri, 5 Apr 2019 10:36:15 -0600 Subject: [PATCH 4/7] Updated example to clear error state on component blur --- .../src/views/combo_box/disallow_custom_options.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src-docs/src/views/combo_box/disallow_custom_options.js b/src-docs/src/views/combo_box/disallow_custom_options.js index b43aa586766..11c053e4255 100644 --- a/src-docs/src/views/combo_box/disallow_custom_options.js +++ b/src-docs/src/views/combo_box/disallow_custom_options.js @@ -40,21 +40,18 @@ export default class extends Component { onChange = (selectedOptions) => { this.setState({ + error: undefined, selectedOptions, }); } onBlur = () => { const { value } = this.inputRef; - if (value.length !== 0) { - this.setState({ - error: `"${value}" is not a valid option`, - }); - } + this.setState({ + error: value.length === 0 ? undefined : `"${value}" is not a valid option`, + }); } - clearError = () => this.setState({ error: undefined }); - setInputRef = ref => this.inputRef = ref; render() { @@ -67,7 +64,6 @@ export default class extends Component { inputRef={this.setInputRef} onChange={this.onChange} onBlur={this.onBlur} - onFocus={this.clearError} /> ); From 1e3d106e6d8d0c8f91611728838e13dfbe835f59 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Thu, 18 Apr 2019 08:40:35 -0600 Subject: [PATCH 5/7] changelog --- CHANGELOG.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a556bf2cb3..369e9c62faf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ **Bug fixes** - Added `toastLifeTimeMs` typescript definition for individual toasts in `EuiGlobalToastList` ([#1846](https://github.com/elastic/eui/pull/1846)) +- Fixed EuiComboBox's internal tracking of its focus state ([#1796](https://github.com/elastic/eui/pull/1796)) ## [`10.0.1`](https://github.com/elastic/eui/tree/v10.0.1) @@ -64,10 +65,6 @@ - Fixed issue where toasts would dismiss when they have focus ([#1803](https://github.com/elastic/eui/pull/1803)) - Fixed issue where `EuiComboBox` placeholder was not read by screen readers ([#1803](https://github.com/elastic/eui/pull/1803)) -**Bug fixes** - -- Fixed EuiComboBox's internal tracking of its focus state ([#1796](https://github.com/elastic/eui/pull/1796)) - ## [`9.8.0`](https://github.com/elastic/eui/tree/v9.8.0) - **[Beta]** Added new `EuiSelectable` component ([#1699](https://github.com/elastic/eui/pull/1699)) From f3c00385bacd4945d3ddb95a73f074a192c8ca44 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Thu, 18 Apr 2019 09:39:41 -0600 Subject: [PATCH 6/7] More messaging hooks for EuiComboBox unsaved value --- src-docs/src/views/combo_box/disallow_custom_options.js | 7 +++++++ src/components/combo_box/combo_box.js | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src-docs/src/views/combo_box/disallow_custom_options.js b/src-docs/src/views/combo_box/disallow_custom_options.js index 11c053e4255..da625e06213 100644 --- a/src-docs/src/views/combo_box/disallow_custom_options.js +++ b/src-docs/src/views/combo_box/disallow_custom_options.js @@ -45,6 +45,12 @@ export default class extends Component { }); } + onSearchChange = (value, hasMatchingOptions) => { + this.setState({ + error: value.length === 0 || hasMatchingOptions ? undefined : `"${value}" is not a valid option`, + }); + } + onBlur = () => { const { value } = this.inputRef; this.setState({ @@ -63,6 +69,7 @@ export default class extends Component { selectedOptions={this.state.selectedOptions} inputRef={this.setInputRef} onChange={this.onChange} + onSearchChange={this.onSearchChange} onBlur={this.onBlur} /> diff --git a/src/components/combo_box/combo_box.js b/src/components/combo_box/combo_box.js index 3b6e88f1aac..7e4f7fa4964 100644 --- a/src/components/combo_box/combo_box.js +++ b/src/components/combo_box/combo_box.js @@ -422,7 +422,8 @@ export class EuiComboBox extends Component { onSearchChange = (searchValue) => { if (this.props.onSearchChange) { - this.props.onSearchChange(searchValue); + const hasMatchingOptions = this.state.matchingOptions.length > 0; + this.props.onSearchChange(searchValue, hasMatchingOptions); } this.setState( From 09af7bb6a5f25fb2d5c0f58f1c4a39b0f043acd0 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Fri, 19 Apr 2019 10:57:39 -0600 Subject: [PATCH 7/7] Also trigger the combobox invalid visual state when the dropdown array is clicked --- src/components/combo_box/combo_box.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/combo_box/combo_box.js b/src/components/combo_box/combo_box.js index 7e4f7fa4964..f78316ad012 100644 --- a/src/components/combo_box/combo_box.js +++ b/src/components/combo_box/combo_box.js @@ -585,7 +585,7 @@ export class EuiComboBox extends Component { // Visually indicate the combobox is in an invalid state if it has lost focus but there is text entered in the input. // When custom options are disabled and the user leaves the combo box after entering text that does not match any // options, this tells the user that they've entered invalid input. - const markAsInvalid = isInvalid || (hasFocus === false && searchValue); + const markAsInvalid = isInvalid || ((hasFocus === false || isListOpen === false) && searchValue); const classes = classNames('euiComboBox', className, { 'euiComboBox-isOpen': isListOpen,