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

[EuiComboBox] Allow options to have a unique key #4048

Merged
merged 13 commits into from
Oct 6, 2020
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

- Added `childrenBetween` prop to `EuiInMemoryTable` to add content between search bar and table ([#4103](https://github.com/elastic/eui/pull/4103))
- Added `max-width: 100%` to `EuiKeyPadMenu` to allow it to shrink when its container is smaller than its fixed width ([ #4092](https://github.com/elastic/eui/pull/4092))
- Added `key` to `EuiComboBoxOptionOption` to allow duplicate labels ([#4048](https://github.com/elastic/eui/pull/4048))
- Changed `EuiIcon` test mock to render as `span` instead of `div` ([#4099](https://github.com/elastic/eui/pull/4099))
- Added `scripts/docker-puppeteer` as the new home for test-related Docker images ([#4062](https://github.com/elastic/eui/pull/4062))

Expand Down
69 changes: 69 additions & 0 deletions src-docs/src/views/combo_box/combo_box_duplicates.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import React, { useState } from 'react';

import { EuiComboBox } from '../../../../src/components';

const optionsStatic = [
{
label: 'Titan',
key: 'titan1',
},
{
label: 'Titan',
key: 'titan2',
},
{
label: 'Enceladus is disabled',
disabled: true,
},
{
label: 'Titan',
key: 'titan3',
},
{
label: 'Dione',
},
];
export default () => {
const [options, setOptions] = useState(optionsStatic);
const [selectedOptions, setSelected] = useState([options[2], options[4]]);

const onChange = selectedOptions => {
setSelected(selectedOptions);
};

const onCreateOption = (searchValue, flattenedOptions = []) => {
const normalizedSearchValue = searchValue.trim().toLowerCase();

if (!normalizedSearchValue) {
return;
}

const newOption = {
label: searchValue,
};

// Create the option if it doesn't exist.
if (
flattenedOptions.findIndex(
option => option.label.trim().toLowerCase() === normalizedSearchValue
) === -1
) {
setOptions([...options, newOption]);
}

// Select the option.
setSelected([...selectedOptions, newOption]);
};

return (
<EuiComboBox
placeholder="Select or create options"
options={options}
selectedOptions={selectedOptions}
onChange={onChange}
onCreateOption={onCreateOption}
isClearable={true}
data-test-subj="demoComboBox"
/>
);
};
49 changes: 36 additions & 13 deletions src-docs/src/views/combo_box/combo_box_example.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { GuideSectionTypes } from '../../components';

import {
EuiLink,
EuiCallOut,
EuiCode,
EuiComboBox,
EuiSpacer,
Expand Down Expand Up @@ -183,6 +182,18 @@ const startingWithSnippet = `<EuiComboBox
isClearable={true}
/>`;

import DuplicateOptions from './combo_box_duplicates';
const duplicateOptionsSource = require('!!raw-loader!./combo_box_duplicates');
const duplicateOptionsHtml = renderToHtml(DuplicateOptions);
const duplicateOptionsSnippet = `const options = [{
label: 'Label',
key: 'label1',
},
{
label: 'Label',
key: 'Label2',
}]`;

This comment was marked as outdated.

export const ComboBoxExample = {
title: 'Combo box',
intro: (
Expand All @@ -197,18 +208,6 @@ export const ComboBoxExample = {
</p>
</EuiText>

<EuiSpacer />

<EuiCallOut title="No duplicate option labels allowed" color="warning">
<p>
The combo box will have errors if any of the options you pass to it
share the same label property. It&rsquo;s OK if options have duplicate
values, though. This is because the label is the only thing the combo
box is concerned about, since this is what the user sees and what is
matched against when the user searches.
</p>
</EuiCallOut>

<EuiSpacer size="l" />
</Fragment>
),
Expand Down Expand Up @@ -567,5 +566,29 @@ export const ComboBoxExample = {
snippet: startingWithSnippet,
demo: <StartingWith />,
},
{
title: 'Duplicate labels',
source: [
{
type: GuideSectionTypes.JS,
code: duplicateOptionsSource,
},
{
type: GuideSectionTypes.HTML,
code: duplicateOptionsHtml,
},
],
text: (
<p>
In general, it is not recommended to use duplicate labels on the
options because the user has no way to distinguish between them. If
you need duplicate labels, you will need to add a unique{' '}
<EuiCode language="js">key</EuiCode> for each option.
</p>
),
props: { EuiComboBox },
demo: <DuplicateOptions />,

This comment was marked as resolved.

snippet: duplicateOptionsSnippet,
},
],
};
4 changes: 3 additions & 1 deletion src/components/combo_box/combo_box.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,9 @@ export class EuiComboBox<T> extends Component<
this.props.selectedOptions.length === 1
) {
const selectedOptionIndex = this.state.matchingOptions.findIndex(
option => option.label === this.props.selectedOptions[0].label
option =>
option.label === this.props.selectedOptions[0].label &&
option.key === this.props.selectedOptions[0].key
);
this.setState({
activeOptionIndex: selectedOptionIndex,
Expand Down
4 changes: 2 additions & 2 deletions src/components/combo_box/combo_box_input/combo_box_input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export class EuiComboBoxInput<T> extends Component<

const pills = selectedOptions
? selectedOptions.map(option => {
const { label, color, onClick, ...rest } = option;
const { key, label, color, onClick, ...rest } = option;
const pillOnClose =
isDisabled || singleSelection || onClick
? undefined
Expand All @@ -179,7 +179,7 @@ export class EuiComboBoxInput<T> extends Component<
<EuiComboBoxPill
option={option}
onClose={pillOnClose}
key={label.toLowerCase()}
key={key ?? label.toLowerCase()}
color={color}
onClick={onClick}
onClickAriaLabel={onClick ? 'Change' : undefined}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ export class EuiComboBoxOptionsList<T> extends Component<

ListRow = ({ data, index, style }: ListChildComponentProps) => {
const option = data[index];
const { isGroupLabelOption, label, value, ...rest } = option;
const { key, isGroupLabelOption, label, value, ...rest } = option;
const {
singleSelection,
selectedOptions,
Expand All @@ -227,7 +227,7 @@ export class EuiComboBoxOptionsList<T> extends Component<

if (isGroupLabelOption) {
return (
<div key={label.toLowerCase()} style={style}>
<div key={key ?? label.toLowerCase()} style={style}>
<EuiComboBoxTitle>{label}</EuiComboBoxTitle>
</div>
);
Expand All @@ -249,7 +249,7 @@ export class EuiComboBoxOptionsList<T> extends Component<
return (
<EuiFilterSelectItem
style={style}
key={option.label.toLowerCase()}
key={option.key ?? option.label.toLowerCase()}
onClick={() => {
if (onOptionClick) {
onOptionClick(option);
Expand Down
35 changes: 35 additions & 0 deletions src/components/combo_box/matching_options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,41 @@ const testCases: GetMatchingOptionsTestCase[] = [
],
sortMatchesBy: 'none',
},
{
options: [{ label: 'Titan' }, { label: 'Titan' }],
selectedOptions: [
{
label: 'Titan',
},
],
searchValue: 'titan',
isPreFiltered: true,
showPrevSelected: false,
expected: [
// Duplicate options without an key will be treated as the same option
],
sortMatchesBy: 'none',
},
{
options: [
{ label: 'Titan', key: 'titan1' },
{ label: 'Titan', key: 'titan2' },
],
selectedOptions: [
{
label: 'Titan',
key: 'titan2',
},
],
searchValue: 'titan',
isPreFiltered: true,
showPrevSelected: false,
expected: [
// Duplicate options with an key will be treated as different items
{ label: 'Titan', key: 'titan1' },
],
sortMatchesBy: 'none',
},
];

describe('getMatchingOptions', () => {
Expand Down
16 changes: 12 additions & 4 deletions src/components/combo_box/matching_options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,14 @@ export const flattenOptionGroups = <T>(

export const getSelectedOptionForSearchValue = <T>(
searchValue: string,
selectedOptions: Array<EuiComboBoxOptionOption<T>>
selectedOptions: Array<EuiComboBoxOptionOption<T>>,
optionKey?: string
) => {
const normalizedSearchValue = searchValue.toLowerCase();
return selectedOptions.find(
option => option.label.toLowerCase() === normalizedSearchValue
option =>
option.label.toLowerCase() === normalizedSearchValue &&
(!optionKey || option.key === optionKey)
);
};

Expand All @@ -59,7 +62,8 @@ const collectMatchingOption = <T>(
// Only show options which haven't yet been selected unless requested.
const selectedOption = getSelectedOptionForSearchValue(
option.label,
selectedOptions
selectedOptions,
chandlerprall marked this conversation as resolved.
Show resolved Hide resolved
option.key
);
if (selectedOption && !showPrevSelected) {
return false;
Expand Down Expand Up @@ -108,7 +112,11 @@ export const getMatchingOptions = <T>(
});
if (matchingOptionsForGroup.length > 0) {
// Add option for group label
matchingOptions.push({ label: option.label, isGroupLabelOption: true });
matchingOptions.push({
key: option.key,
label: option.label,
isGroupLabelOption: true,
});
// Add matching options for group
matchingOptions.push(...matchingOptionsForGroup);
}
Expand Down
1 change: 1 addition & 0 deletions src/components/combo_box/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export type EuiComboBoxOptionOption<
Omit<ButtonHTMLAttributes<HTMLButtonElement>, 'value'> & {
isGroupLabelOption?: boolean;
label: string;
key?: string;
options?: Array<EuiComboBoxOptionOption<T>>;
value?: T;
};
Expand Down