Add support for validation to EuiComboBox and fix some bugs (#631)

* Fix bug where clicking a pill's close button would close the list, by refining the focus and blur logic. * Fix bug with positioning the list when there's no scrollbar, caused by the absolute position in the CSS. * Fix bug with logic managing the euiBody-hasPortalContent class. * Add support for validation to EuiComboBox.
elastic · Apr 4, 2018 · ef67350 · ef67350
1 parent cb878e2
commit ef67350
Show file tree

Hide file tree

Showing 10 changed files with 124 additions and 61 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,13 +3,18 @@
 - Modifying drop shadow intensities and color. ([607](https://github.com/elastic/eui/pull/607))
 - Add Sass color functions. Make `$euiColorWarning` color usage more accessible while still being "yellow". ([628](https://github.com/elastic/eui/pull/628))
 - Removed extraneous `global_styling/mixins/_forms.scss` file and importing the correct files in the `filter_group.scss` and `combo_box.scss` files. ([#609](https://github.com/elastic/eui/pull/609))
+- Added `isInvalid` prop to `EuiComboBox` ([#631](https://github.com/elastic/eui/pull/631))
+- Added support for rejecting user input by returning `false` from the `onCreateOption` prop of `EuiComboBox` ([#631](https://github.com/elastic/eui/pull/631))
 
 **Bug fixes**
 
 - Visual fix for the focus state of disabled `EuiButton` ([#603](https://github.com/elastic/eui/pull/603))
 - `EuiSelect` can pass any node as a value rather than just a string ([#603](https://github.com/elastic/eui/pull/603))
 - Fixed a typo in the flex TypeScript definition ([#629](https://github.com/elastic/eui/pull/629))
 - Fixed `EuiComboBox` bug in which the options list wouldn't always match the width of the input ([#611](https://github.com/elastic/eui/pull/611))
+- Fixed `EuiComboBox` bug in which opening the combo box when there's no scrollbar on the window would result in the list being positioned incorrectly ([#631](https://github.com/elastic/eui/pull/631))
+- Fixed `EuiComboBox` bug in which clicking a pill's close button would close the list ([#631](https://github.com/elastic/eui/pull/631))
+- Fixed `EuiComboBox` bug in which moving focus from one combo box to another would remove the `euiBody-hasPortalContent` class from the body. ([#631](https://github.com/elastic/eui/pull/631))
 
 # [`0.0.37`](https://github.com/elastic/eui/tree/v0.0.37)
 

diff --git a/src-docs/src/views/combo_box/combo_box_example.js b/src-docs/src/views/combo_box/combo_box_example.js
@@ -198,7 +198,7 @@ export const ComboBoxExample = {
     props: { EuiComboBox },
     demo: <DisallowCustomOptions />,
   }, {
-    title: 'Hiding suggestions',
+    title: 'Custom options only, with validation',
     source: [{
       type: GuideSectionTypes.JS,
       code: customOptionsOnlySource,

diff --git a/src-docs/src/views/combo_box/custom_options_only.js b/src-docs/src/views/combo_box/custom_options_only.js
@@ -2,22 +2,28 @@ import React, { Component } from 'react';
 
 import {
   EuiComboBox,
+  EuiFormRow,
 } from '../../../../src/components';
 
+const isValid = (value) => {
+  // Only allow letters. No spaces, numbers, or special characters.
+  return value.match(/^[a-zA-Z]+$/) !== null;
+};
+
 export default class extends Component {
   constructor(props) {
     super(props);
 
     this.state = {
+      isInvalid: false,
       selectedOptions: [],
     };
   }
 
   onCreateOption = (searchValue) => {
-    const normalizedSearchValue = searchValue.trim().toLowerCase();
-
-    if (!normalizedSearchValue) {
-      return;
+    if (!isValid(searchValue)) {
+      // Return false to explicitly reject the user's input.
+      return false;
     }
 
     const newOption = {
@@ -30,22 +36,45 @@ export default class extends Component {
     }));
   };
 
+  onSearchChange = (searchValue) => {
+    if (!searchValue) {
+      this.setState({
+        isInvalid: false,
+      });
+
+      return;
+    }
+
+    this.setState({
+      isInvalid: !isValid(searchValue),
+    });
+  };
+
   onChange = (selectedOptions) => {
     this.setState({
       selectedOptions,
+      isInvalid: false,
     });
   };
 
   render() {
-    const { selectedOptions } = this.state;
+    const { selectedOptions, isInvalid } = this.state;
     return (
-      <EuiComboBox
-        noSuggestions
-        placeholder="Create some tags"
-        selectedOptions={selectedOptions}
-        onCreateOption={this.onCreateOption}
-        onChange={this.onChange}
-      />
+      <EuiFormRow
+        label="Only custom options"
+        isInvalid={isInvalid}
+        error={isInvalid ? 'Only letters are allowed' : undefined}
+      >
+        <EuiComboBox
+          noSuggestions
+          placeholder="Create some tags (letters only)"
+          selectedOptions={selectedOptions}
+          onCreateOption={this.onCreateOption}
+          onChange={this.onChange}
+          onSearchChange={this.onSearchChange}
+          isInvalid={isInvalid}
+        />
+      </EuiFormRow>
     );
   }
 }
diff --git a/src/components/combo_box/_combo_box.scss b/src/components/combo_box/_combo_box.scss
@@ -54,4 +54,10 @@
         inset 0 -2px 0 0 $euiColorPrimary;
     }
   }
+
+  &.euiComboBox-isInvalid {
+    .euiComboBox__inputWrap {
+      @include euiFormControlInvalidStyle;
+    }
+  }
 }
diff --git a/src/components/combo_box/combo_box.js b/src/components/combo_box/combo_box.js
@@ -37,6 +37,7 @@ export class EuiComboBox extends Component {
     onSearchChange: PropTypes.func,
     onCreateOption: PropTypes.func,
     renderOption: PropTypes.func,
+    isInvalid: PropTypes.bool,
   }
 
   static defaultProps = {
@@ -190,6 +191,11 @@ export class EuiComboBox extends Component {
     }
   };
 
+  focusSearchInput = () => {
+    this.clearActiveOption();
+    this.searchInput.focus();
+  };
+
   clearSearchValue = () => {
     this.onSearchChange('');
   };
@@ -229,7 +235,13 @@ export class EuiComboBox extends Component {
 
     // Add new custom pill if this is custom input, even if it partially matches an option..
     if (!this.hasActiveOption() || this.doesSearchMatchOnlyOption()) {
-      this.props.onCreateOption(this.state.searchValue, flattenOptionGroups(this.props.options));
+      const isOptionCreated = this.props.onCreateOption(this.state.searchValue, flattenOptionGroups(this.props.options));
+
+      // Expect the consumer to be explicit in rejecting a custom option.
+      if (isOptionCreated === false) {
+        return;
+      }
+
       this.clearSearchValue();
     }
   };
@@ -251,7 +263,19 @@ export class EuiComboBox extends Component {
     return flattenOptionGroups(options).length === selectedOptions.length;
   };
 
-  onFocusChange = event => {
+  onFocus = () => {
+    document.addEventListener('click', this.onDocumentFocusChange);
+    document.addEventListener('focusin', this.onDocumentFocusChange);
+    this.openList();
+  }
+
+  onBlur = () => {
+    document.removeEventListener('click', this.onDocumentFocusChange);
+    document.removeEventListener('focusin', this.onDocumentFocusChange);
+    this.closeList();
+  }
+
+  onDocumentFocusChange = event => {
     // Close the list if the combo box has lost focus.
     if (
       this.comboBox === event.target
@@ -264,7 +288,11 @@ export class EuiComboBox extends Component {
 
     // Wait for the DOM to update.
     requestAnimationFrame(() => {
-      this.closeList();
+      if (document.activeElement === this.searchInput) {
+        return;
+      }
+
+      this.onBlur();
     });
   };
 
@@ -287,8 +315,7 @@ export class EuiComboBox extends Component {
       case ESCAPE:
         // Move focus from options list to input.
         if (this.hasActiveOption()) {
-          this.clearActiveOption();
-          this.searchInput.focus();
+          this.focusSearchInput();
         }
         break;
 
@@ -319,14 +346,14 @@ export class EuiComboBox extends Component {
   onAddOption = (addedOption) => {
     const { onChange, selectedOptions, singleSelection } = this.props;
     onChange(singleSelection ? [addedOption] : selectedOptions.concat(addedOption));
-    this.clearActiveOption();
     this.clearSearchValue();
-    this.searchInput.focus();
+    this.focusSearchInput();
   };
 
   onRemoveOption = (removedOption) => {
     const { onChange, selectedOptions } = this.props;
     onChange(selectedOptions.filter(option => option !== removedOption));
+    this.focusSearchInput();
   };
 
   onComboBoxClick = () => {
@@ -381,9 +408,6 @@ export class EuiComboBox extends Component {
   };
 
   componentDidMount() {
-    document.addEventListener('click', this.onFocusChange);
-    document.addEventListener('focusin', this.onFocusChange);
-
     // TODO: This will need to be called once the actual stylesheet loads.
     setTimeout(() => {
       this.autoSizeInput.copyInputStyles();
@@ -419,8 +443,8 @@ export class EuiComboBox extends Component {
   }
 
   componentWillUnmount() {
-    document.removeEventListener('click', this.onFocusChange);
-    document.removeEventListener('focusin', this.onFocusChange);
+    document.removeEventListener('click', this.onDocumentFocusChange);
+    document.removeEventListener('focusin', this.onDocumentFocusChange);
   }
 
   render() {
@@ -438,13 +462,15 @@ export class EuiComboBox extends Component {
       onChange, // eslint-disable-line no-unused-vars
       onSearchChange, // eslint-disable-line no-unused-vars
       async, // eslint-disable-line no-unused-vars
+      isInvalid,
       ...rest
     } = this.props;
 
     const { searchValue, isListOpen, listPosition } = this.state;
 
     const classes = classNames('euiComboBox', className, {
       'euiComboBox-isOpen': isListOpen,
+      'euiComboBox-isInvalid': isInvalid,
     });
 
     const value = selectedOptions.map(selectedOption => selectedOption.label).join(', ');
@@ -491,7 +517,7 @@ export class EuiComboBox extends Component {
           onRemoveOption={this.onRemoveOption}
           onClick={this.onComboBoxClick}
           onChange={this.onSearchChange}
-          onFocus={this.openList}
+          onFocus={this.onFocus}
           value={value}
           searchValue={searchValue}
           autoSizeInputRef={this.autoSizeInputRef}

diff --git a/src/components/combo_box/combo_box_input/combo_box_input.js b/src/components/combo_box/combo_box_input/combo_box_input.js
@@ -3,7 +3,7 @@ import PropTypes from 'prop-types';
 import AutosizeInput from 'react-input-autosize';
 
 import { EuiScreenReaderOnly } from '../../accessibility';
-import { EuiFormControlLayout, EuiValidatableControl } from '../../form';
+import { EuiFormControlLayout } from '../../form';
 import { EuiComboBoxPill } from './combo_box_pill';
 import { htmlIdGenerator } from '../../../services';
 
@@ -143,20 +143,18 @@ export class EuiComboBoxInput extends Component {
         >
           {pills}
           {placeholderMessage}
-          <EuiValidatableControl isInvalid={false}>
-            <AutosizeInput
-              aria-hidden
-              id={id}
-              style={{ fontSize: 14 }}
-              className="euiComboBox__input"
-              onFocus={this.onFocus}
-              onBlur={this.onBlur}
-              onChange={(e) => onChange(e.target.value)}
-              value={searchValue}
-              ref={autoSizeInputRef}
-              inputRef={inputRef}
-            />
-          </EuiValidatableControl>
+          <AutosizeInput
+            aria-hidden
+            id={id}
+            style={{ fontSize: 14 }}
+            className="euiComboBox__input"
+            onFocus={this.onFocus}
+            onBlur={this.onBlur}
+            onChange={(e) => onChange(e.target.value)}
+            value={searchValue}
+            ref={autoSizeInputRef}
+            inputRef={inputRef}
+          />
           {removeOptionMessage}
         </div>
       </EuiFormControlLayout>

diff --git a/src/components/combo_box/combo_box_input/combo_box_pill.js b/src/components/combo_box/combo_box_input/combo_box_pill.js
@@ -21,19 +21,16 @@ export class EuiComboBoxPill extends Component {
     color: 'hollow',
   };
 
-  onCloseButtonClick(option, e) {
-    // Prevent the combo box from losing focus.
-    e.preventDefault();
-    e.stopPropagation();
-    e.nativeEvent.stopImmediatePropagation();
-    this.props.onClose(option)
-  }
+  onCloseButtonClick = () => {
+    const { onClose, option } = this.props;
+    onClose(option);
+  };
 
   render() {
     const {
       children,
       className,
-      option,
+      option, // eslint-disable-line no-unused-vars
       onClose, // eslint-disable-line no-unused-vars
       color,
       ...rest
@@ -44,7 +41,7 @@ export class EuiComboBoxPill extends Component {
       <EuiBadge
         className={classes}
         title={children}
-        iconOnClick={this.onCloseButtonClick.bind(this, option)}
+        iconOnClick={this.onCloseButtonClick}
         iconType="cross"
         iconSide="right"
         color={color}

diff --git a/src/components/combo_box/combo_box_options_list/_combo_box_options_list.scss b/src/components/combo_box/combo_box_options_list/_combo_box_options_list.scss
@@ -1,11 +1,16 @@
 /**
- * 1. Make width match that of the input.
+ * 1. Make width match that of the input and tweak position to match.
+ * 2. Put the list at the top of the screen, otherwise it will cause a scrollbar to show up when
+ *    the portal is appended to the body. This would throw off our logic for positioning the
+ *    list beneath the input.
  */
 .euiComboBoxOptionsList {
   @include euiFormControlSize;
   box-sizing: content-box; /* 1 */
+  margin-left: -1px; /* 1 */
   z-index: $euiZComboBox;
-  position: absolute;
+  position: absolute; /* 2 */
+  top: 0; /* 2 */
 }
 
 .euiComboBoxOptionsList--bottom {

diff --git a/src/components/combo_box/combo_box_options_list/combo_box_option.js b/src/components/combo_box/combo_box_options_list/combo_box_option.js
@@ -16,11 +16,7 @@ export class EuiComboBoxOption extends Component {
     onEnterKey: PropTypes.func.isRequired,
   }
 
-  onClick = (e) => {
-    // Prevent the combo box from losing focus.
-    e.preventDefault();
-    e.stopPropagation();
-    e.nativeEvent.stopImmediatePropagation();
+  onClick = () => {
     const { onClick, option } = this.props;
     onClick(option);
   };