-
Notifications
You must be signed in to change notification settings - Fork 842
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
EuiComboBox additions #1139
EuiComboBox additions #1139
Changes from 4 commits
6fbfd8d
a12c9a3
0d8f369
aa4cbf5
1f5c562
0d7c788
c1cf96c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,12 @@ export class EuiComboBox extends Component { | |
placeholder: PropTypes.string, | ||
isLoading: PropTypes.bool, | ||
async: PropTypes.bool, | ||
singleSelection: PropTypes.bool, | ||
singleSelection: PropTypes.oneOfType([ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we change this propType to also check for the singleSelection: PropTypes.oneOfType([
PropTypes.bool,
PropTypes.shape({
asPlainText: PropTypes.bool,
}),
]), |
||
PropTypes.bool, | ||
PropTypes.shape({ | ||
asPlainText: PropTypes.bool, | ||
}), | ||
]), | ||
noSuggestions: PropTypes.bool, | ||
options: PropTypes.array, | ||
selectedOptions: PropTypes.array, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,10 @@ export class EuiComboBoxInput extends Component { | |
isListOpen: PropTypes.bool.isRequired, | ||
onOpenListClick: PropTypes.func.isRequired, | ||
onCloseListClick: PropTypes.func.isRequired, | ||
singleSelection: PropTypes.bool, | ||
singleSelection: PropTypes.oneOfType([ | ||
PropTypes.bool, | ||
PropTypes.object, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we use |
||
]), | ||
isDisabled: PropTypes.bool, | ||
toggleButtonRef: PropTypes.func, | ||
fullWidth: PropTypes.bool, | ||
|
@@ -100,15 +103,21 @@ export class EuiComboBoxInput extends Component { | |
const { | ||
label, | ||
color, | ||
onClick, | ||
...rest | ||
} = option; | ||
|
||
const asPlainText = singleSelection && singleSelection.asPlainText; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here's how we can set // Default asPlainText to true if using singleSelection.
const asPlainText = singleSelection && singleSelection.asPlainText !== false; Note that this will be a breaking change for anyone who is depending upon the pill form so we'll need to update the CHANGELOG.md. |
||
|
||
return ( | ||
<EuiComboBoxPill | ||
option={option} | ||
onClose={(isDisabled || singleSelection) ? null : onRemoveOption} | ||
onClose={(isDisabled || singleSelection || onClick) ? null : onRemoveOption} | ||
key={label.toLowerCase()} | ||
color={color} | ||
onClick={onClick} | ||
onClickAriaLabel={onClick ? 'Change' : null} | ||
asPlainText={asPlainText} | ||
{...rest} | ||
> | ||
{label} | ||
|
@@ -171,6 +180,8 @@ export class EuiComboBoxInput extends Component { | |
|
||
const wrapClasses = classNames('euiComboBox__inputWrap', { | ||
'euiComboBox__inputWrap--fullWidth': fullWidth, | ||
'euiComboBox__inputWrap--noWrap': singleSelection, | ||
'euiComboBox__inputWrap-isClearable': onClear, | ||
}); | ||
|
||
return ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,8 @@ | ||
import React, { | ||
Component, | ||
} from 'react'; | ||
import React, { Component } from 'react'; | ||
import PropTypes from 'prop-types'; | ||
import classNames from 'classnames'; | ||
|
||
import { | ||
EuiBadge, | ||
} from '../../badge'; | ||
import { EuiBadge } from '../../badge'; | ||
|
||
export class EuiComboBoxPill extends Component { | ||
static propTypes = { | ||
|
@@ -15,6 +11,7 @@ export class EuiComboBoxPill extends Component { | |
className: PropTypes.string, | ||
color: PropTypes.string, | ||
onClose: PropTypes.func, | ||
asPlainText: PropTypes.bool, | ||
}; | ||
|
||
static defaultProps = { | ||
|
@@ -33,9 +30,16 @@ export class EuiComboBoxPill extends Component { | |
option, // eslint-disable-line no-unused-vars | ||
onClose, // eslint-disable-line no-unused-vars | ||
color, | ||
asPlainText, | ||
...rest | ||
} = this.props; | ||
const classes = classNames('euiComboBoxPill', className); | ||
const classes = classNames( | ||
'euiComboBoxPill', | ||
{ | ||
'euiComboBoxPill--plainText': asPlainText, | ||
}, | ||
className | ||
); | ||
|
||
if (onClose) { | ||
return ( | ||
|
@@ -48,7 +52,7 @@ export class EuiComboBoxPill extends Component { | |
iconSide="right" | ||
color={color} | ||
closeButtonProps={{ | ||
tabIndex: '-1' | ||
tabIndex: '-1', | ||
}} | ||
{...rest} | ||
> | ||
|
@@ -57,13 +61,12 @@ export class EuiComboBoxPill extends Component { | |
); | ||
} | ||
|
||
if (asPlainText) { | ||
return <span className={classes}>{children}</span>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this also pass There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Youp, I'll add it. Good catch. |
||
} | ||
|
||
return ( | ||
<EuiBadge | ||
className={classes} | ||
title={children} | ||
color={color} | ||
{...rest} | ||
> | ||
<EuiBadge className={classes} title={children} color={color} {...rest}> | ||
{children} | ||
</EuiBadge> | ||
); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like this will usually be the desired case, right? I can't think of a situation where we'd want the pill form for single selection. If you agree then how about we update the combo box to use
asPlainText: true
as default, while still giving people the option to specifyfalse
if for some reason they want the pill?