fix(select): ent-3507 object values, match selected options (RedHatIn…

…sights#590)
cdcabrera · Mar 25, 2021 · 981fe54 · 981fe54
1 parent ad2892d
commit 981fe54
Show file tree

Hide file tree

Showing 3 changed files with 112 additions and 40 deletions.
diff --git a/src/components/form/__tests__/__snapshots__/select.test.js.snap b/src/components/form/__tests__/__snapshots__/select.test.js.snap
@@ -1,6 +1,6 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`Select Component should allow a alternate array and object options: key value object 1`] = `
+exports[`Select Component should allow alternate array and object options: key value object 1`] = `
 <div
   class="pf-c-select curiosity-select "
   data-ouia-component-id="OUIA-Generated-Select-single-2"
@@ -45,7 +45,7 @@ exports[`Select Component should allow a alternate array and object options: key
 </div>
 `;
 
-exports[`Select Component should allow a alternate array and object options: string array 1`] = `
+exports[`Select Component should allow alternate array and object options: string array 1`] = `
 <div
   class="pf-c-select curiosity-select "
   data-ouia-component-id="OUIA-Generated-Select-single-2"
@@ -297,19 +297,64 @@ exports[`Select Component should allow being disabled with missing options: opti
 />
 `;
 
-exports[`Select Component should allow selected options to match value or title: value or title match 1`] = `
+exports[`Select Component should allow plain objects as values, and be able to select options based on values within the object: select when option values are objects 1`] = `
 <div
   class="pf-c-select curiosity-select "
-  data-ouia-component-id="OUIA-Generated-Select-checkbox-2"
+  data-ouia-component-id="OUIA-Generated-Select-single-3"
   data-ouia-component-type="PF4/Select"
   data-ouia-safe="true"
 >
   <button
     aria-expanded="false"
+    aria-haspopup="listbox"
     aria-labelledby=" pf-select-toggle-id-14"
     class="pf-c-select__toggle"
     id="pf-select-toggle-id-14"
     type="button"
+  >
+    <div
+      class="pf-c-select__toggle-wrapper"
+    >
+      <span
+        class="pf-c-select__toggle-text"
+      >
+        hello
+      </span>
+    </div>
+    <span
+      class="pf-c-select__toggle-arrow"
+    >
+      <svg
+        aria-hidden="true"
+        fill="currentColor"
+        height="1em"
+        role="img"
+        style="vertical-align: -0.125em;"
+        viewBox="0 0 320 512"
+        width="1em"
+      >
+        <path
+          d="M31.3 192h257.3c17.8 0 26.7 21.5 14.1 34.1L174.1 354.8c-7.8 7.8-20.5 7.8-28.3 0L17.2 226.1C4.6 213.5 13.5 192 31.3 192z"
+        />
+      </svg>
+    </span>
+  </button>
+</div>
+`;
+
+exports[`Select Component should allow selected options to match value or title: value or title match 1`] = `
+<div
+  class="pf-c-select curiosity-select "
+  data-ouia-component-id="OUIA-Generated-Select-checkbox-2"
+  data-ouia-component-type="PF4/Select"
+  data-ouia-safe="true"
+>
+  <button
+    aria-expanded="false"
+    aria-labelledby=" pf-select-toggle-id-17"
+    class="pf-c-select__toggle"
+    id="pf-select-toggle-id-17"
+    type="button"
   >
     <div
       class="pf-c-select__toggle-wrapper"
@@ -604,17 +649,17 @@ exports[`Select Component should render a expanded select: expanded 1`] = `
     >
       <div
         className="pf-c-select pf-m-expanded curiosity-select "
-        data-ouia-component-id="OUIA-Generated-Select-single-4"
+        data-ouia-component-id="OUIA-Generated-Select-single-5"
         data-ouia-component-type="PF4/Select"
         data-ouia-safe={true}
       >
         <SelectToggle
           aria-label="Options menu"
-          aria-labelledby=" pf-select-toggle-id-29"
+          aria-labelledby=" pf-select-toggle-id-32"
           className=""
           handleTypeaheadKeys={[Function]}
           hasClearButton={false}
-          id="pf-select-toggle-id-29"
+          id="pf-select-toggle-id-32"
           isActive={false}
           isDisabled={false}
           isOpen={true}
@@ -697,16 +742,16 @@ exports[`Select Component should render a expanded select: expanded 1`] = `
             Object {
               "current": <div
                 class="pf-c-select pf-m-expanded curiosity-select "
-                data-ouia-component-id="OUIA-Generated-Select-single-4"
+                data-ouia-component-id="OUIA-Generated-Select-single-5"
                 data-ouia-component-type="PF4/Select"
                 data-ouia-safe="true"
               >
                 <button
                   aria-expanded="true"
                   aria-haspopup="listbox"
-                  aria-labelledby=" pf-select-toggle-id-29"
+                  aria-labelledby=" pf-select-toggle-id-32"
                   class="pf-c-select__toggle"
-                  id="pf-select-toggle-id-29"
+                  id="pf-select-toggle-id-32"
                   type="button"
                 >
                   <div
@@ -811,10 +856,10 @@ exports[`Select Component should render a expanded select: expanded 1`] = `
           <button
             aria-expanded={true}
             aria-haspopup="listbox"
-            aria-labelledby=" pf-select-toggle-id-29"
+            aria-labelledby=" pf-select-toggle-id-32"
             className="pf-c-select__toggle"
             disabled={false}
-            id="pf-select-toggle-id-29"
+            id="pf-select-toggle-id-32"
             onClick={[Function]}
             onKeyDown={[Function]}
             type="button"
@@ -964,7 +1009,7 @@ exports[`Select Component should render a expanded select: expanded 1`] = `
                 data-value="lorem"
                 id="bG9yZW0tbG9yZW0="
                 index={0}
-                inputId="pf-random-id-5-0"
+                inputId="pf-random-id-6-0"
                 isChecked={false}
                 isDisabled={false}
                 isFavorite={null}
@@ -1003,7 +1048,7 @@ exports[`Select Component should render a expanded select: expanded 1`] = `
                 data-value="ipsum"
                 id="aXBzdW0taXBzdW0="
                 index={1}
-                inputId="pf-random-id-5-1"
+                inputId="pf-random-id-6-1"
                 isChecked={false}
                 isDisabled={false}
                 isFavorite={null}
@@ -1042,7 +1087,7 @@ exports[`Select Component should render a expanded select: expanded 1`] = `
                 data-value="hello"
                 id="aGVsbG8taGVsbG8="
                 index={2}
-                inputId="pf-random-id-5-2"
+                inputId="pf-random-id-6-2"
                 isChecked={false}
                 isDisabled={false}
                 isFavorite={null}
@@ -1081,7 +1126,7 @@ exports[`Select Component should render a expanded select: expanded 1`] = `
                 data-value="world"
                 id="d29ybGQtd29ybGQ="
                 index={3}
-                inputId="pf-random-id-5-3"
+                inputId="pf-random-id-6-3"
                 isChecked={false}
                 isDisabled={false}
                 isFavorite={null}

diff --git a/src/components/form/__tests__/select.test.js b/src/components/form/__tests__/select.test.js
@@ -34,7 +34,7 @@ describe('Select Component', () => {
     expect(component.render()).toMatchSnapshot('checkbox select');
   });
 
-  it('should allow a alternate array and object options', () => {
+  it('should allow alternate array and object options', () => {
     const props = {
       id: 'test',
       options: ['lorem', 'ipsum', 'hello', 'world'],
@@ -53,6 +53,21 @@ describe('Select Component', () => {
     expect(component.render()).toMatchSnapshot('key value object');
   });
 
+  it('should allow plain objects as values, and be able to select options based on values within the object', () => {
+    const props = {
+      id: 'test',
+      options: [
+        { title: 'lorem', value: { dolor: 'sit' } },
+        { title: 'dolor', value: { lorem: 'ipsum' } },
+        { title: 'hello', value: { hello: 'world' } }
+      ],
+      selectedOptions: ['world']
+    };
+
+    const component = mount(<Select {...props} />);
+    expect(component.render()).toMatchSnapshot('select when option values are objects');
+  });
+
   it('should allow selected options to match value or title', () => {
     const props = {
       id: 'test',

diff --git a/src/components/form/select.js b/src/components/form/select.js
@@ -3,20 +3,10 @@ import PropTypes from 'prop-types';
 import { Select as PfSelect, SelectOption as PfSelectOption, SelectVariant } from '@patternfly/react-core';
 import _cloneDeep from 'lodash/cloneDeep';
 import _isEqual from 'lodash/isEqual';
+import _findIndex from 'lodash/findIndex';
 import _isPlainObject from 'lodash/isPlainObject';
 import { helpers } from '../../common/helpers';
 
-/**
- * FixMe: Patternfly React select generates a random ID for select options.
- * On the surface this seems like a legitimate update until you remember unit tests.
- * Quite a few apps use test snapshots causing certain rendered select snapshots to
- * fail consistently. Appears this may have been part of the "favorites" update.
- * The solution centers around updating the  "GenerateId helper" and detecting a
- * test, dev, or prod environment instead of generating a random string every time.
- *
- * This issue also has the side-effect of making the attribute inadvertently
- * "required" anywhere it's used in an effort to squash it.
- */
 /**
  * A wrapper for Patternfly Select. Provides restructured event data for onSelect callback.
  *
@@ -156,8 +146,26 @@ class Select extends React.Component {
       convertedOption.label = convertedOption.label || convertedOption.title;
 
       if (activateOptions) {
-        updatedOptions[index].selected =
-          activateOptions.includes(convertedOption.value) || activateOptions.includes(convertedOption.title);
+        let isSelected;
+
+        if (_isPlainObject(convertedOption.value)) {
+          isSelected = _findIndex(activateOptions, convertedOption.value) > -1;
+
+          if (!isSelected) {
+            const tempSearch = activateOptions.find(activeOption =>
+              Object.values(convertedOption.value).includes(activeOption)
+            );
+            isSelected = !!tempSearch;
+          }
+        } else {
+          isSelected = activateOptions.includes(convertedOption.value);
+        }
+
+        if (!isSelected) {
+          isSelected = activateOptions.includes(convertedOption.title);
+        }
+
+        updatedOptions[index].selected = isSelected;
       }
     });
 
@@ -218,7 +226,7 @@ class Select extends React.Component {
               key={window.btoa(`${option.title}-${option.value}`)}
               id={window.btoa(`${option.title}-${option.value}`)}
               value={option.title}
-              data-value={option.value}
+              data-value={(_isPlainObject(option.value) && JSON.stringify([option.value])) || option.value}
               data-title={option.title}
             />
           ))) ||
@@ -231,10 +239,10 @@ class Select extends React.Component {
 /**
  * Prop types.
  *
- * @type {{isToggleText: boolean, toggleIcon: (Node|Function), name: string, options: (Array|object),
- *     selectedOptions: (number|string|Array), variant: (object|string), className: string,
- *     id: string, isDisabled: boolean, placeholder: string, ariaLabel: string,
- *     onSelect: Function}}
+ * @type {{toggleIcon: (Node|Function), className: string, ariaLabel: string, onSelect: Function,
+ *     isToggleText: boolean, maxHeight: number, name: string, options: (Array|object),
+ *     selectedOptions: (number|string|Array), variant: string, id: string, isDisabled: boolean,
+ *     placeholder: string}}
  */
 Select.propTypes = {
   ariaLabel: PropTypes.string,
@@ -262,18 +270,22 @@ Select.propTypes = {
     PropTypes.object
   ]),
   placeholder: PropTypes.string,
-  selectedOptions: PropTypes.oneOfType([PropTypes.number, PropTypes.string, PropTypes.array]),
+  selectedOptions: PropTypes.oneOfType([
+    PropTypes.number,
+    PropTypes.string,
+    PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.number, PropTypes.string]))
+  ]),
   toggleIcon: PropTypes.element,
   variant: PropTypes.oneOf([...Object.values(SelectVariant)])
 };
 
 /**
  * Default props.
  *
- * @type {{isToggleText: boolean, toggleIcon: (Node|Function), name: string, options: (Array|object),
- *     selectedOptions: (number|string|Array), variant: (object|string), className: string,
- *     id: string, isDisabled: boolean, placeholder: string, ariaLabel: string,
- *     onSelect: Function}}
+ * @type {{toggleIcon: (Node|Function), className: string, ariaLabel: string, onSelect: Function,
+ *     isToggleText: boolean, maxHeight: number, name: string, options: (Array|object),
+ *     selectedOptions: (number|string|Array), variant: SelectVariant.single, id: string,
+ *     isDisabled: boolean, placeholder: string}}
  */
 Select.defaultProps = {
   ariaLabel: 'Select option',