Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
Apr 4, 2018
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@

- Modified drop shadow intensities and color. ([#607](https://github.com/elastic/eui/pull/607))
- 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))

# [`0.0.37`](https://github.com/elastic/eui/tree/v0.0.37)

Expand Down
2 changes: 1 addition & 1 deletion src-docs/src/views/combo_box/combo_box_example.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
53 changes: 41 additions & 12 deletions src-docs/src/views/combo_box/custom_options_only.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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>
);
}
}
6 changes: 6 additions & 0 deletions src/components/combo_box/_combo_box.scss
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,10 @@
inset 0 -2px 0 0 $euiColorPrimary;
}
}

&.euiComboBox-isInvalid {
.euiComboBox__inputWrap {
@include euiFormControlInvalidStyle;
}
}
}
54 changes: 40 additions & 14 deletions src/components/combo_box/combo_box.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export class EuiComboBox extends Component {
onSearchChange: PropTypes.func,
onCreateOption: PropTypes.func,
renderOption: PropTypes.func,
isInvalid: PropTypes.bool,
}

static defaultProps = {
Expand Down Expand Up @@ -116,7 +117,7 @@ export class EuiComboBox extends Component {

const { position, left, top } = calculatePopoverPosition(comboBoxBounds, this.listBounds, 'bottom', 0, ['bottom', 'top']);

this.optionsList.style.top = `${top + window.scrollY}px`;
this.optionsList.style.top = `${top}px`;
this.optionsList.style.left = `${left}px`;
this.optionsList.style.width = `${comboBoxBounds.width}px`;

Expand Down Expand Up @@ -190,6 +191,11 @@ export class EuiComboBox extends Component {
}
};

focusSearchInput = () => {
this.clearActiveOption();
this.searchInput.focus();
};

clearSearchValue = () => {
this.onSearchChange('');
};
Expand Down Expand Up @@ -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();
}
};
Expand All @@ -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
Expand All @@ -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();
});
};

Expand All @@ -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;

Expand Down Expand Up @@ -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 = () => {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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() {
Expand All @@ -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(', ');
Expand Down Expand Up @@ -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}
Expand Down
28 changes: 13 additions & 15 deletions src/components/combo_box/combo_box_input/combo_box_input.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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>
Expand Down
15 changes: 6 additions & 9 deletions src/components/combo_box/combo_box_input/combo_box_pill.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
/**
* 1. Make width match that of the input.
* 1. Make width match that of the input and tweak position to match.
* 2. We can't use absolute position here because this will cause a scrollbar to show up when
* the portal is appended to the body. This throws 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: fixed; /* 2 */
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@snide Do you see any problem with this changing to fixed? Is there a reason it needs to be absolute?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when you scroll the page? Fixed normally breaks in that situation if it's open unless you are constantly calculating it on scroll.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We update the position as the user scrolls

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you're constantly calculating so you should be ok. Lemme double check on IE11 though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Little laggy on IE11 but seems acceptable. FWIW it's broken on master in a similar way to the tooltips (where the top value isn't being applied correctly).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do some more review tonight, but off-hand I think this works fine. Checked the other browsers as well.

}

.euiComboBoxOptionsList--bottom {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ export class EuiComboBoxOptionsList extends Component {
};

componentDidMount() {
document.body.classList.add('euiBody-hasPortalContent');

this.updatePosition();
window.addEventListener('resize', this.updatePosition);
window.addEventListener('scroll', this.updatePosition);
Expand All @@ -67,7 +65,6 @@ export class EuiComboBoxOptionsList extends Component {
}

componentWillUnmount() {
document.body.classList.remove('euiBody-hasPortalContent');
window.removeEventListener('resize', this.updatePosition);
window.removeEventListener('scroll', this.updatePosition);
}
Expand Down