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
@@ -1,6 +1,7 @@
## [`master`](https://github.com/elastic/eui/tree/master)

- Added footer row to `EuiDataGrid` via the `renderFooterCellValue` prop ([#3770](https://github.com/elastic/eui/pull/3770))
- Added `id` to `EuiComboBoxOptionOption` to allow duplicate labels ([#4048](https://github.com/elastic/eui/pull/4048))
- Added column header menu to `EuiDataGrid` ([#3087](https://github.com/elastic/eui/pull/3087))

**Bug fixes**
Expand Down
70 changes: 70 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,70 @@
import React, { useState } from 'react';

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

const optionsStatic = [
{
label: 'Titan',
id: 'titan1',
},
{
label: 'Titan',
id: 'titan2',
},
{
label: 'Enceladus is disabled',
disabled: true,
},
{
label: 'Titan',
id: '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 (
/* DisplayToggles wrapper for Docs only */
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this comment as it doesn't apply here.

Suggested change
/* DisplayToggles wrapper for Docs only */

<EuiComboBox
placeholder="Select or create options"
options={options}
selectedOptions={selectedOptions}
onChange={onChange}
onCreateOption={onCreateOption}
isClearable={true}
data-test-subj="demoComboBox"
/>
);
};
41 changes: 35 additions & 6 deletions src-docs/src/views/combo_box/combo_box_example.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ const startingWithSnippet = `<EuiComboBox
isClearable={true}
/>`;

import DuplicateOptions from './combo_box_duplicates';
const duplicateOptionsSource = require('!!raw-loader!./combo_box_duplicates');
const duplicateOptionsHtml = renderToHtml(DuplicateOptions);

This comment was marked as outdated.

export const ComboBoxExample = {
title: 'Combo box',
intro: (
Expand All @@ -199,13 +203,15 @@ export const ComboBoxExample = {

<EuiSpacer />

<EuiCallOut title="No duplicate option labels allowed" color="warning">
<EuiCallOut title="Duplicate labels require an id" color="warning">
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's actually remove this entire callout now that we have a specific example section that explains this more succinctly.

<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.
The combo box will have errors by default 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. You can pass
an unique <EuiCode>id</EuiCode> to an option if you require duplicate
labels. See the example below.
</p>
</EuiCallOut>

Expand Down Expand Up @@ -567,5 +573,28 @@ export const ComboBoxExample = {
snippet: startingWithSnippet,
demo: <StartingWith />,
},
{
title: 'Duplicate labels',
source: [
{
type: GuideSectionTypes.JS,
code: duplicateOptionsSource,
},
{
type: GuideSectionTypes.HTML,
code: duplicateOptionsHtml,
},
],
text: (
<p>
If you want to use options with a duplicate label (which in general is
chandlerprall marked this conversation as resolved.
Show resolved Hide resolved
not recommended since the user has no way to distinguish those
options), you need to set a unique <EuiCode language="js">id</EuiCode>{' '}
for each option.
timroes marked this conversation as resolved.
Show resolved Hide resolved
</p>
),
props: { EuiComboBox },
demo: <DuplicateOptions />,

This comment was marked as resolved.

},
],
};
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 { id, 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={id ?? label.toLowerCase()}
chandlerprall marked this conversation as resolved.
Show resolved Hide resolved
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 { id, 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={id ?? 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.id ?? 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 id will be treated as the same option
],
sortMatchesBy: 'none',
},
{
options: [
{ label: 'Titan', id: 'titan1' },
{ label: 'Titan', id: 'titan2' },
],
selectedOptions: [
{
label: 'Titan',
id: 'titan2',
},
],
searchValue: 'titan',
isPreFiltered: true,
showPrevSelected: false,
expected: [
// Duplicate options with an id will be treated as different items
{ label: 'Titan', id: '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>>,
optionId?: string
) => {
const normalizedSearchValue = searchValue.toLowerCase();
return selectedOptions.find(
option => option.label.toLowerCase() === normalizedSearchValue
option =>
option.label.toLowerCase() === normalizedSearchValue &&
(!optionId || option.id === optionId)
);
};

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.id
);
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({
id: option.id,
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;
id?: string;
options?: Array<EuiComboBoxOptionOption<T>>;
value?: T;
};
Expand Down