fix(select): correct single variant check (#217)

* fixes single variant check * expands component update check * expands provided mock target data, aligns “checked” to pf result * expands selected values check to include both title and value
RedHatInsights · Mar 5, 2020 · fabf6f1 · fabf6f1
1 parent c6f44b6
commit fabf6f1
Show file tree

Hide file tree

Showing 3 changed files with 244 additions and 6 deletions.
diff --git a/src/components/select/__tests__/__snapshots__/select.test.js.snap b/src/components/select/__tests__/__snapshots__/select.test.js.snap
@@ -80,6 +80,54 @@ exports[`Select Component should allow a alternate array and object options: str
 </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 "
+>
+  <button
+    aria-expanded="false"
+    aria-labelledby=" pf-toggle-id-14"
+    class="pf-c-select__toggle"
+    id="pf-toggle-id-14"
+    type="button"
+  >
+    <div
+      class="pf-c-select__toggle-wrapper"
+    >
+      <span
+        class="pf-c-select__toggle-text"
+      >
+        Select option
+      </span>
+      <div
+        class="pf-c-select__toggle-badge"
+      >
+        <span
+          class="pf-c-badge pf-m-read"
+        >
+          2
+        </span>
+      </div>
+    </div>
+    <svg
+      aria-hidden="true"
+      class="pf-c-select__toggle-arrow"
+      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"
+        transform=""
+      />
+    </svg>
+  </button>
+</div>
+`;
+
 exports[`Select Component should render a basic component: basic dropdown 1`] = `
 <div
   class="pf-c-select curiosity-select "
@@ -207,6 +255,14 @@ Object {
         "value": "world",
       },
     ],
+    "selected": Object {
+      "label": "hello",
+      "selected": true,
+      "text": "hello",
+      "textContent": "hello",
+      "title": "hello",
+      "value": "hello",
+    },
     "selectedIndex": 2,
     "type": "select-one",
     "value": "hello",
@@ -248,6 +304,14 @@ Object {
     },
   ],
   "persist": [Function],
+  "selected": Object {
+    "label": "hello",
+    "selected": true,
+    "text": "hello",
+    "textContent": "hello",
+    "title": "hello",
+    "value": "hello",
+  },
   "selectedIndex": 2,
   "target": Object {
     "id": "test",
@@ -286,6 +350,14 @@ Object {
         "value": "world",
       },
     ],
+    "selected": Object {
+      "label": "hello",
+      "selected": true,
+      "text": "hello",
+      "textContent": "hello",
+      "title": "hello",
+      "value": "hello",
+    },
     "selectedIndex": 2,
     "type": "select-one",
     "value": "hello",
@@ -294,3 +366,145 @@ Object {
   "value": "hello",
 }
 `;
+
+exports[`Select Component should return an emulated onchange event: emulated event 2`] = `
+Object {
+  "checked": true,
+  "currentTarget": Object {
+    "checked": true,
+    "id": "test",
+    "name": "test",
+    "options": Array [
+      Object {
+        "label": "lorem",
+        "selected": false,
+        "text": "lorem",
+        "textContent": "lorem",
+        "title": "lorem",
+        "value": "lorem",
+      },
+      Object {
+        "label": "ipsum",
+        "selected": false,
+        "text": "ipsum",
+        "textContent": "ipsum",
+        "title": "ipsum",
+        "value": "ipsum",
+      },
+      Object {
+        "label": "hello",
+        "selected": true,
+        "text": "hello",
+        "textContent": "hello",
+        "title": "hello",
+        "value": "hello",
+      },
+      Object {
+        "label": "world",
+        "selected": true,
+        "text": "world",
+        "textContent": "world",
+        "title": "world",
+        "value": "world",
+      },
+    ],
+    "selected": Array [
+      "hello",
+      "world",
+    ],
+    "selectedIndex": 3,
+    "type": "select-multiple",
+    "value": "world",
+  },
+  "id": "test",
+  "name": "test",
+  "options": Array [
+    Object {
+      "label": "lorem",
+      "selected": false,
+      "text": "lorem",
+      "textContent": "lorem",
+      "title": "lorem",
+      "value": "lorem",
+    },
+    Object {
+      "label": "ipsum",
+      "selected": false,
+      "text": "ipsum",
+      "textContent": "ipsum",
+      "title": "ipsum",
+      "value": "ipsum",
+    },
+    Object {
+      "label": "hello",
+      "selected": true,
+      "text": "hello",
+      "textContent": "hello",
+      "title": "hello",
+      "value": "hello",
+    },
+    Object {
+      "label": "world",
+      "selected": true,
+      "text": "world",
+      "textContent": "world",
+      "title": "world",
+      "value": "world",
+    },
+  ],
+  "persist": [Function],
+  "selected": Array [
+    "hello",
+    "world",
+  ],
+  "selectedIndex": 3,
+  "target": Object {
+    "checked": true,
+    "id": "test",
+    "name": "test",
+    "options": Array [
+      Object {
+        "label": "lorem",
+        "selected": false,
+        "text": "lorem",
+        "textContent": "lorem",
+        "title": "lorem",
+        "value": "lorem",
+      },
+      Object {
+        "label": "ipsum",
+        "selected": false,
+        "text": "ipsum",
+        "textContent": "ipsum",
+        "title": "ipsum",
+        "value": "ipsum",
+      },
+      Object {
+        "label": "hello",
+        "selected": true,
+        "text": "hello",
+        "textContent": "hello",
+        "title": "hello",
+        "value": "hello",
+      },
+      Object {
+        "label": "world",
+        "selected": true,
+        "text": "world",
+        "textContent": "world",
+        "title": "world",
+        "value": "world",
+      },
+    ],
+    "selected": Array [
+      "hello",
+      "world",
+    ],
+    "selectedIndex": 3,
+    "type": "select-multiple",
+    "value": "world",
+  },
+  "type": "select-multiple",
+  "value": "world",
+}
+`;
diff --git a/src/components/select/__tests__/select.test.js b/src/components/select/__tests__/select.test.js
@@ -52,6 +52,18 @@ describe('Select Component', () => {
     expect(component.render()).toMatchSnapshot('key value object');
   });
 
+  it('should allow selected options to match value or title', () => {
+    const props = {
+      id: 'test',
+      options: { lorem: 'ipsum', hello: 'world', dolor: 'set' },
+      selectedOptions: ['world', 'lorem', 'fail'],
+      variant: SelectVariant.checkbox
+    };
+
+    const component = mount(<Select {...props} />);
+    expect(component.render()).toMatchSnapshot('value or title match');
+  });
+
   it('should return an emulated onchange event', done => {
     const props = {
       id: 'test',
@@ -65,7 +77,12 @@ describe('Select Component', () => {
     };
 
     const component = mount(<Select {...props} />);
-    const componentInstance = component.instance();
-    componentInstance.onSelect({}, 'hello');
+    component.instance().onSelect({}, 'hello');
+
+    component.setProps({
+      variant: SelectVariant.checkbox
+    });
+
+    component.instance().onSelect({}, 'world');
   });
 });
diff --git a/src/components/select/select.js b/src/components/select/select.js
@@ -18,9 +18,9 @@ class Select extends React.Component {
   }
 
   componentDidUpdate(prevProps) {
-    const { options } = this.props;
+    const { options, selectedOptions } = this.props;
 
-    if (!_isEqual(prevProps.options, options)) {
+    if (!_isEqual(prevProps.options, options) || !_isEqual(prevProps.selectedOptions, selectedOptions)) {
       this.formatOptions();
     }
   }
@@ -34,7 +34,7 @@ class Select extends React.Component {
     updatedOptions[optionsIndex].selected =
       variant === SelectVariant.single ? true : !updatedOptions[optionsIndex].selected;
 
-    if (SelectVariant.single) {
+    if (variant === SelectVariant.single) {
       updatedOptions.forEach((option, index) => {
         if (optionsIndex !== index) {
           updatedOptions[index].selected = false;
@@ -59,11 +59,17 @@ class Select extends React.Component {
           id,
           name: name || id,
           value: mockUpdatedOptions[optionsIndex].value,
+          selected:
+            (variant === SelectVariant.single && mockUpdatedOptions[optionsIndex]) || _cloneDeep(updateSelected),
           selectedIndex: optionsIndex,
           type: `select-${(variant === SelectVariant.single && 'one') || 'multiple'}`,
           options: mockUpdatedOptions
         };
 
+        if (variant === SelectVariant.checkbox) {
+          mockTarget.checked = mockUpdatedOptions[optionsIndex].selected;
+        }
+
         const mockEvent = {
           ...mockTarget,
           target: { ...mockTarget },
@@ -116,7 +122,8 @@ class Select extends React.Component {
       convertedOption.label = convertedOption.label || convertedOption.title;
 
       if (activateOptions) {
-        updatedOptions[index].selected = activateOptions.includes(convertedOption.value);
+        updatedOptions[index].selected =
+          activateOptions.includes(convertedOption.value) || activateOptions.includes(convertedOption.title);
       }
     });