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

* Allow options in combo box to have an id * Change warning in documentation * Fix eslint error * Added changelog entry * Address review * Update src-docs/src/views/combo_box/combo_box_example.js Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> * Address review feedback * Rename id to key * Address recently merged PRs changes Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>
kshitij86 · Nov 29, 2020 · 3418eb6 · 3418eb6
1 parent 75ca7fd
commit 3418eb6
Show file tree

Hide file tree

Showing 9 changed files with 162 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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))
 

diff --git a/src-docs/src/views/combo_box/combo_box_duplicates.js b/src-docs/src/views/combo_box/combo_box_duplicates.js
@@ -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"
+    />
+  );
+};
diff --git a/src-docs/src/views/combo_box/combo_box_example.js b/src-docs/src/views/combo_box/combo_box_example.js
@@ -8,7 +8,6 @@ import { GuideSectionTypes } from '../../components';
 
 import {
   EuiLink,
-  EuiCallOut,
   EuiCode,
   EuiComboBox,
   EuiSpacer,
@@ -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',
+}]`;
+
 export const ComboBoxExample = {
   title: 'Combo box',
   intro: (
@@ -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>
   ),
@@ -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 />,
+      snippet: duplicateOptionsSnippet,
+    },
   ],
 };
diff --git a/src/components/combo_box/combo_box.tsx b/src/components/combo_box/combo_box.tsx
@@ -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,

diff --git a/src/components/combo_box/combo_box_input/combo_box_input.tsx b/src/components/combo_box/combo_box_input/combo_box_input.tsx
@@ -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
@@ -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}

diff --git a/src/components/combo_box/combo_box_options_list/combo_box_options_list.tsx b/src/components/combo_box/combo_box_options_list/combo_box_options_list.tsx
@@ -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,
@@ -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>
       );
@@ -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);

diff --git a/src/components/combo_box/matching_options.test.ts b/src/components/combo_box/matching_options.test.ts
@@ -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', () => {

diff --git a/src/components/combo_box/matching_options.ts b/src/components/combo_box/matching_options.ts
@@ -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)
   );
 };
 
@@ -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,
+    option.key
   );
   if (selectedOption && !showPrevSelected) {
     return false;
@@ -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);
       }

diff --git a/src/components/combo_box/types.ts b/src/components/combo_box/types.ts
@@ -27,6 +27,7 @@ export type EuiComboBoxOptionOption<
   Omit<ButtonHTMLAttributes<HTMLButtonElement>, 'value'> & {
     isGroupLabelOption?: boolean;
     label: string;
+    key?: string;
     options?: Array<EuiComboBoxOptionOption<T>>;
     value?: T;
   };