From 1289ca91037123716de12d18d622fc360f1cc3ae Mon Sep 17 00:00:00 2001
From: Keith Grant <keithjgrant@redhat.com>
Date: Mon, 13 Jan 2020 10:26:48 -0800
Subject: [PATCH] update TagMultiSelect to use PF <Select>

---
 .../components/MultiSelect/MultiSelect.jsx    | 227 ------------------
 .../MultiSelect/MultiSelect.test.jsx          | 104 --------
 .../components/MultiSelect/TagMultiSelect.jsx |  64 ++---
 .../src/components/MultiSelect/index.js       |   1 -
 .../src/components/MultiSelect/usePFSelect.js |   6 +-
 5 files changed, 26 insertions(+), 376 deletions(-)
 delete mode 100644 awx/ui_next/src/components/MultiSelect/MultiSelect.jsx
 delete mode 100644 awx/ui_next/src/components/MultiSelect/MultiSelect.test.jsx

diff --git a/awx/ui_next/src/components/MultiSelect/MultiSelect.jsx b/awx/ui_next/src/components/MultiSelect/MultiSelect.jsx
deleted file mode 100644
index b8afb15855..0000000000
--- a/awx/ui_next/src/components/MultiSelect/MultiSelect.jsx
+++ /dev/null
@@ -1,227 +0,0 @@
-import React, { Component, Fragment } from 'react';
-import { shape, number, string, func, arrayOf, oneOfType } from 'prop-types';
-import { Chip, ChipGroup } from '@components/Chip';
-import {
-  Dropdown as PFDropdown,
-  DropdownItem,
-  TextInput as PFTextInput,
-  DropdownToggle,
-} from '@patternfly/react-core';
-import styled from 'styled-components';
-
-const InputGroup = styled.div`
-  border: 1px solid black;
-  margin-top: 2px;
-`;
-
-const TextInput = styled(PFTextInput)`
-  border: none;
-  width: 100%;
-  padding-left: 8px;
-`;
-
-const Dropdown = styled(PFDropdown)`
-  width: 100%;
-  .pf-c-dropdown__toggle.pf-m-plain {
-    display: none;
-  }
-  display: block;
-  .pf-c-dropdown__menu {
-    max-height: 200px;
-    overflow: scroll;
-  }
-  && button[disabled] {
-    color: var(--pf-c-button--m-plain--Color);
-    pointer-events: initial;
-    cursor: not-allowed;
-    color: var(--pf-global--disabled-color--200);
-  }
-`;
-
-const Item = shape({
-  id: oneOfType([number, string]).isRequired,
-  name: string.isRequired,
-});
-
-class MultiSelect extends Component {
-  static propTypes = {
-    value: arrayOf(Item).isRequired,
-    options: arrayOf(Item),
-    onAddNewItem: func,
-    onRemoveItem: func,
-    onChange: func,
-    createNewItem: func,
-  };
-
-  static defaultProps = {
-    onAddNewItem: () => {},
-    onRemoveItem: () => {},
-    onChange: () => {},
-    options: [],
-    createNewItem: null,
-  };
-
-  constructor(props) {
-    super(props);
-    this.state = {
-      input: '',
-      isExpanded: false,
-    };
-    this.handleKeyDown = this.handleKeyDown.bind(this);
-    this.handleInputChange = this.handleInputChange.bind(this);
-    this.removeItem = this.removeItem.bind(this);
-    this.handleClick = this.handleClick.bind(this);
-    this.createNewItem = this.createNewItem.bind(this);
-  }
-
-  componentDidMount() {
-    document.addEventListener('mousedown', this.handleClick, false);
-  }
-
-  componentWillUnmount() {
-    document.removeEventListener('mousedown', this.handleClick, false);
-  }
-
-  handleClick(e, option) {
-    if (this.node && this.node.contains(e.target)) {
-      if (option) {
-        e.preventDefault();
-        this.addItem(option);
-      }
-    } else {
-      this.setState({ input: '', isExpanded: false });
-    }
-  }
-
-  addItem(item) {
-    const { value, onAddNewItem, onChange } = this.props;
-    const items = value.concat(item);
-    onAddNewItem(item);
-    onChange(items);
-    this.close();
-  }
-
-  // TODO: UpArrow & DownArrow for menu navigation
-  handleKeyDown(event) {
-    const { value, options } = this.props;
-    const { input } = this.state;
-    if (event.key === 'Tab') {
-      this.close();
-      return;
-    }
-    if (!input || event.key !== 'Enter') {
-      return;
-    }
-
-    const isAlreadySelected = value.some(i => i.name === input);
-    if (isAlreadySelected) {
-      event.preventDefault();
-      this.close();
-      return;
-    }
-
-    const match = options.find(item => item.name === input);
-    const isNewItem = !match || !value.find(item => item.id === match.id);
-    if (isNewItem) {
-      event.preventDefault();
-      this.addItem(match || this.createNewItem(input));
-    }
-  }
-
-  close() {
-    this.setState({
-      isExpanded: false,
-      input: '',
-    });
-  }
-
-  createNewItem(name) {
-    const { createNewItem } = this.props;
-    if (createNewItem) {
-      return createNewItem(name);
-    }
-    return {
-      id: Math.random(),
-      name,
-    };
-  }
-
-  handleInputChange(value) {
-    this.setState({ input: value, isExpanded: true });
-  }
-
-  removeItem(item) {
-    const { value, onRemoveItem, onChange } = this.props;
-    const remainingItems = value.filter(chip => chip.id !== item.id);
-
-    onRemoveItem(item);
-    onChange(remainingItems);
-  }
-
-  render() {
-    const { value, options } = this.props;
-    const { input, isExpanded } = this.state;
-
-    const dropdownOptions = options.map(option => (
-      <Fragment key={option.id}>
-        {option.name.includes(input) ? (
-          <DropdownItem
-            component="button"
-            isDisabled={value.some(item => item.id === option.id)}
-            value={option.name}
-            onClick={e => {
-              this.handleClick(e, option);
-            }}
-          >
-            {option.name}
-          </DropdownItem>
-        ) : null}
-      </Fragment>
-    ));
-
-    return (
-      <Fragment>
-        <InputGroup>
-          <div
-            ref={node => {
-              this.node = node;
-            }}
-          >
-            <TextInput
-              type="text"
-              aria-label="labels"
-              value={input}
-              onFocus={() => this.setState({ isExpanded: true })}
-              onChange={this.handleInputChange}
-              onKeyDown={this.handleKeyDown}
-            />
-            <Dropdown
-              type="button"
-              isPlain
-              value={value}
-              toggle={<DropdownToggle isPlain>Labels</DropdownToggle>}
-              // Above is not visible but is a required prop from Patternfly
-              isOpen={isExpanded}
-              dropdownItems={dropdownOptions}
-            />
-          </div>
-          <div css="margin: 10px">
-            <ChipGroup defaultIsOpen numChips={5}>
-              {value.map(item => (
-                <Chip
-                  key={item.id}
-                  onClick={() => {
-                    this.removeItem(item);
-                  }}
-                >
-                  {item.name}
-                </Chip>
-              ))}
-            </ChipGroup>
-          </div>
-        </InputGroup>
-      </Fragment>
-    );
-  }
-}
-export default MultiSelect;
diff --git a/awx/ui_next/src/components/MultiSelect/MultiSelect.test.jsx b/awx/ui_next/src/components/MultiSelect/MultiSelect.test.jsx
deleted file mode 100644
index 82627b4949..0000000000
--- a/awx/ui_next/src/components/MultiSelect/MultiSelect.test.jsx
+++ /dev/null
@@ -1,104 +0,0 @@
-import React from 'react';
-import { mount, shallow } from 'enzyme';
-import MultiSelect from './MultiSelect';
-
-describe('<MultiSelect />', () => {
-  const value = [
-    { name: 'Foo', id: 1, organization: 1 },
-    { name: 'Bar', id: 2, organization: 1 },
-  ];
-  const options = [{ name: 'Angry', id: 3 }, { name: 'Potato', id: 4 }];
-
-  test('should render successfully', () => {
-    const wrapper = shallow(
-      <MultiSelect
-        onAddNewItem={jest.fn()}
-        onRemoveItem={jest.fn()}
-        value={value}
-        options={options}
-      />
-    );
-
-    expect(wrapper.find('Chip')).toHaveLength(2);
-  });
-
-  test('should add item when typed', async () => {
-    const onChange = jest.fn();
-    const onAdd = jest.fn();
-    const wrapper = mount(
-      <MultiSelect
-        onAddNewItem={onAdd}
-        onRemoveItem={jest.fn()}
-        onChange={onChange}
-        value={value}
-        options={options}
-      />
-    );
-    const component = wrapper.find('MultiSelect');
-    const input = component.find('TextInputBase');
-    input.invoke('onChange')('Flabadoo');
-    input.simulate('keydown', { key: 'Enter' });
-
-    expect(onAdd.mock.calls[0][0].name).toEqual('Flabadoo');
-    const newVal = onChange.mock.calls[0][0];
-    expect(newVal).toHaveLength(3);
-    expect(newVal[2].name).toEqual('Flabadoo');
-  });
-
-  test('should add item when clicked from menu', () => {
-    const onAddNewItem = jest.fn();
-    const onChange = jest.fn();
-    const wrapper = mount(
-      <MultiSelect
-        onAddNewItem={onAddNewItem}
-        onRemoveItem={jest.fn()}
-        onChange={onChange}
-        value={value}
-        options={options}
-      />
-    );
-
-    const input = wrapper.find('TextInputBase');
-    input.simulate('focus');
-    wrapper.update();
-    const event = {
-      preventDefault: () => {},
-      target: wrapper
-        .find('DropdownItem')
-        .at(1)
-        .getDOMNode(),
-    };
-    wrapper
-      .find('DropdownItem')
-      .at(1)
-      .invoke('onClick')(event);
-
-    expect(onAddNewItem).toHaveBeenCalledWith(options[1]);
-    const newVal = onChange.mock.calls[0][0];
-    expect(newVal).toHaveLength(3);
-    expect(newVal[2]).toEqual(options[1]);
-  });
-
-  test('should remove item', () => {
-    const onRemoveItem = jest.fn();
-    const onChange = jest.fn();
-    const wrapper = mount(
-      <MultiSelect
-        onAddNewItem={jest.fn()}
-        onRemoveItem={onRemoveItem}
-        onChange={onChange}
-        value={value}
-        options={options}
-      />
-    );
-
-    const chips = wrapper.find('PFChip');
-    expect(chips).toHaveLength(2);
-    chips.at(1).invoke('onClick')();
-
-    expect(onRemoveItem).toHaveBeenCalledWith(value[1]);
-    const newVal = onChange.mock.calls[0][0];
-    expect(newVal).toHaveLength(1);
-    expect(newVal).toEqual([value[0]]);
-  });
-});
diff --git a/awx/ui_next/src/components/MultiSelect/TagMultiSelect.jsx b/awx/ui_next/src/components/MultiSelect/TagMultiSelect.jsx
index 4c988436f3..c52de1032f 100644
--- a/awx/ui_next/src/components/MultiSelect/TagMultiSelect.jsx
+++ b/awx/ui_next/src/components/MultiSelect/TagMultiSelect.jsx
@@ -1,41 +1,38 @@
 import React, { useState } from 'react';
 import { func, string } from 'prop-types';
 import { Select, SelectOption, SelectVariant } from '@patternfly/react-core';
-import usePFSelect from '@components/MultiSelect/usePFSelect';
 
 function arrayToString(tags) {
-  return tags.map(v => v.name).join(',');
+  return tags.join(',');
 }
 
 function stringToArray(value) {
-  return value
-    .split(',')
-    .filter(val => !!val)
-    .map(val => ({
-      id: val,
-      name: val,
-    }));
+  return value.split(',').filter(val => !!val);
 }
 
-/*
- * Adapter providing a simplified API to a MultiSelect. The value
- * is a comma-separated string.
- */
 function TagMultiSelect({ onChange, value }) {
-  const { selections, onSelect, options, setOptions } = usePFSelect(
-    value, // TODO: convert with stringToArray without triggering re-render loop
-    val => onChange(arrayToString(val))
-  );
+  const selections = stringToArray(value);
+  const [options, setOptions] = useState(selections);
   const [isExpanded, setIsExpanded] = useState(false);
 
+  const onSelect = (event, item) => {
+    let newValue;
+    if (selections.includes(item)) {
+      newValue = selections.filter(i => i !== item);
+    } else {
+      newValue = selections.concat(item);
+    }
+    onChange(arrayToString(newValue));
+  };
+
   const toggleExpanded = () => {
     setIsExpanded(!isExpanded);
   };
 
   const renderOptions = opts => {
     return opts.map(option => (
-      <SelectOption key={option.id} value={option}>
-        {option.name}
+      <SelectOption key={option} value={option}>
+        {option}
       </SelectOption>
     ));
   };
@@ -45,18 +42,19 @@ function TagMultiSelect({ onChange, value }) {
       variant={SelectVariant.typeaheadMulti}
       onToggle={toggleExpanded}
       onSelect={onSelect}
-      onClear={() => onChange([])}
+      onClear={() => onChange('')}
       onFilter={event => {
         const str = event.target.value.toLowerCase();
-        const matches = options.filter(o => o.name.toLowerCase().includes(str));
+        const matches = options.filter(o => o.toLowerCase().includes(str));
         return renderOptions(matches);
       }}
       isCreatable
       onCreateOption={name => {
-        // TODO check for duplicate in options
-        const newItem = { id: name, name };
-        setOptions(options.concat(newItem));
-        return newItem;
+        name = name.trim();
+        if (!options.includes(name)) {
+          setOptions(options.concat(name));
+        }
+        return name;
       }}
       selections={selections}
       isExpanded={isExpanded}
@@ -65,22 +63,6 @@ function TagMultiSelect({ onChange, value }) {
       {renderOptions(options)}
     </Select>
   );
-  //
-  // return (
-  //   <MultiSelect
-  //     onChange={val => {
-  //       onChange(arrayToString(val));
-  //     }}
-  //     onAddNewItem={newItem => {
-  //       if (!options.find(o => o.name === newItem.name)) {
-  //         setOptions(options.concat(newItem));
-  //       }
-  //     }}
-  //     value={stringToArray(value)}
-  //     options={options}
-  //     createNewItem={name => ({ id: name, name })}
-  //   />
-  // );
 }
 
 TagMultiSelect.propTypes = {
diff --git a/awx/ui_next/src/components/MultiSelect/index.js b/awx/ui_next/src/components/MultiSelect/index.js
index 145029344a..fc192d36cf 100644
--- a/awx/ui_next/src/components/MultiSelect/index.js
+++ b/awx/ui_next/src/components/MultiSelect/index.js
@@ -1,3 +1,2 @@
-export { default } from './MultiSelect';
 export { default as TagMultiSelect } from './TagMultiSelect';
 export { default as usePFSelect } from './usePFSelect';
diff --git a/awx/ui_next/src/components/MultiSelect/usePFSelect.js b/awx/ui_next/src/components/MultiSelect/usePFSelect.js
index 74d4e89a1b..b347b7a919 100644
--- a/awx/ui_next/src/components/MultiSelect/usePFSelect.js
+++ b/awx/ui_next/src/components/MultiSelect/usePFSelect.js
@@ -1,7 +1,8 @@
 import { useState, useEffect } from 'react';
 
 /*
-  Hook for using PatternFly's <Select> component. Guarantees object equality
+  Hook for using PatternFly's <Select> component when a pre-existing value
+  is loaded from somewhere other than the options. Guarantees object equality
   between objects in `value` and the corresponding objects loaded as
   `options` (based on matched id value).
  */
@@ -11,7 +12,6 @@ export default function usePFSelect(value, onChange) {
 
   useEffect(() => {
     if (value !== selections && options.length) {
-      console.log(value, typeof value);
       const syncedValue = value.map(item =>
         options.find(i => i.id === item.id)
       );
@@ -48,5 +48,5 @@ function addToStringToObjects(items = []) {
 }
 
 function toString() {
-  return this.id;
+  return String(this.id);
 }