Merge pull request #5622 from keithjgrant/4613-pf-select-component

PatternFly select component

Reviewed-by: https://github.com/apps/softwarefactory-project-zuul

awx/ui_next/src/components/MultiSelect/MultiSelect.jsx

@@ -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;

awx/ui_next/src/components/MultiSelect/MultiSelect.test.jsx

@@ -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]]);
-  });
-});

awx/ui_next/src/components/MultiSelect/TagMultiSelect.jsx

@@ -1,42 +1,67 @@
 import React, { useState } from 'react';
 import { func, string } from 'prop-types';
-import MultiSelect from './MultiSelect';
+import { Select, SelectOption, SelectVariant } from '@patternfly/react-core';
 
 function arrayToString(tags) {
-  return tags.map(v => v.name).join(',');
+  return tags.join(',');
 }
 
 function stringToArray(value) {
-  return value
+  return value.split(',').filter(val => !!val);
-    .split(',')
-    .filter(val => !!val)
-    .map(val => ({
-      id: val,
-      name: val,
-    }));
 }
 
-/*
- * Adapter providing a simplified API to a MultiSelect. The value
- * is a comma-separated string.
- */
 function TagMultiSelect({ onChange, value }) {
-  const [options, setOptions] = useState(stringToArray(value));
+  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} value={option}>
+        {option}
+      </SelectOption>
+    ));
+  };
 
   return (
-    <MultiSelect
+    <Select
-      onChange={val => {
+      variant={SelectVariant.typeaheadMulti}
-        onChange(arrayToString(val));
+      onToggle={toggleExpanded}
+      onSelect={onSelect}
+      onClear={() => onChange('')}
+      onFilter={event => {
+        const str = event.target.value.toLowerCase();
+        const matches = options.filter(o => o.toLowerCase().includes(str));
+        return renderOptions(matches);
       }}
-      onAddNewItem={newItem => {
+      isCreatable
-        if (!options.find(o => o.name === newItem.name)) {
+      onCreateOption={name => {
-          setOptions(options.concat(newItem));
+        name = name.trim();
+        if (!options.includes(name)) {
+          setOptions(options.concat(name));
         }
+        return name;
       }}
-      value={stringToArray(value)}
+      selections={selections}
-      options={options}
+      isExpanded={isExpanded}
-      createNewItem={name => ({ id: name, name })}
+      ariaLabelledBy="tag-select"
-    />
+    >
+      {renderOptions(options)}
+    </Select>
   );
 }
 

awx/ui_next/src/components/MultiSelect/TagMultiSelect.test.jsx

@@ -3,19 +3,22 @@ import { mount } from 'enzyme';
 import TagMultiSelect from './TagMultiSelect';
 
 describe('<TagMultiSelect />', () => {
-  it('should render MultiSelect', () => {
+  it('should render Select', () => {
     const wrapper = mount(
       <TagMultiSelect value="foo,bar" onChange={jest.fn()} />
     );
-    expect(wrapper.find('MultiSelect').prop('options')).toEqual([
+    wrapper.find('input').simulate('focus');
-      { id: 'foo', name: 'foo' },
+    const options = wrapper.find('SelectOption');
-      { id: 'bar', name: 'bar' },
+    expect(options).toHaveLength(2);
-    ]);
+    expect(options.at(0).prop('value')).toEqual('foo');
+    expect(options.at(1).prop('value')).toEqual('bar');
   });
 
   it('should not treat empty string as an option', () => {
     const wrapper = mount(<TagMultiSelect value="" onChange={jest.fn()} />);
-    expect(wrapper.find('MultiSelect').prop('options')).toEqual([]);
+    wrapper.find('input').simulate('focus');
+    expect(wrapper.find('Select').prop('isExpanded')).toEqual(true);
+    expect(wrapper.find('SelectOption')).toHaveLength(0);
   });
 
   it('should trigger onChange', () => {
@@ -23,13 +26,9 @@ describe('<TagMultiSelect />', () => {
     const wrapper = mount(
       <TagMultiSelect value="foo,bar" onChange={onChange} />
     );
+    wrapper.find('input').simulate('focus');
 
-    const select = wrapper.find('MultiSelect');
+    wrapper.find('Select').invoke('onSelect')(null, 'baz');
-    select.invoke('onChange')([
-      { name: 'foo' },
-      { name: 'bar' },
-      { name: 'baz' },
-    ]);
     expect(onChange).toHaveBeenCalledWith('foo,bar,baz');
   });
 });

awx/ui_next/src/components/MultiSelect/index.js

@@ -1,2 +1,2 @@
-export { default } from './MultiSelect';
 export { default as TagMultiSelect } from './TagMultiSelect';
+export { default as useSyncedSelectValue } from './useSyncedSelectValue';

awx/ui_next/src/components/MultiSelect/useSyncedSelectValue.js

@@ -0,0 +1,52 @@
+import { useState, useEffect } from 'react';
+
+/*
+  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).
+ */
+export default function useSyncedSelectValue(value, onChange) {
+  const [options, setOptions] = useState([]);
+  const [selections, setSelections] = useState([]);
+
+  useEffect(() => {
+    if (value !== selections && options.length) {
+      const syncedValue = value.map(item =>
+        options.find(i => i.id === item.id)
+      );
+      setSelections(syncedValue);
+    }
+    /* eslint-disable-next-line react-hooks/exhaustive-deps */
+  }, [value, options]);
+
+  const onSelect = (event, item) => {
+    if (selections.includes(item)) {
+      onChange(selections.filter(i => i !== item));
+    } else {
+      onChange(selections.concat(item));
+    }
+  };
+
+  return {
+    selections: options.length ? addToStringToObjects(selections) : [],
+    onSelect,
+    options,
+    setOptions: newOpts => setOptions(addToStringToObjects(newOpts)),
+  };
+}
+
+/*
+  PF uses toString to generate React keys. This is used to ensure
+  all objects in the array have a toString method.
+ */
+function addToStringToObjects(items = []) {
+  items.forEach(item => {
+    item.toString = toString;
+  });
+  return items;
+}
+
+function toString() {
+  return String(this.id);
+}

awx/ui_next/src/screens/Template/shared/LabelSelect.jsx

@@ -1,7 +1,8 @@
 import React, { useState, useEffect } from 'react';
 import { func, arrayOf, number, shape, string, oneOfType } from 'prop-types';
-import MultiSelect from '@components/MultiSelect';
+import { Select, SelectOption, SelectVariant } from '@patternfly/react-core';
 import { LabelsAPI } from '@api';
+import { useSyncedSelectValue } from '@components/MultiSelect';
 
 async function loadLabelOptions(setLabels, onError) {
   let labels;
@@ -21,31 +22,55 @@ async function loadLabelOptions(setLabels, onError) {
         page_size: 200,
         order_by: 'name',
       });
-      labels = labels.concat(results);
+      setLabels(labels.concat(results));
     }
-    setLabels(labels);
   } catch (err) {
     onError(err);
   }
 }
 
-function LabelSelect({ value, onChange, onError }) {
+function LabelSelect({ value, placeholder, onChange, onError }) {
-  const [options, setOptions] = useState([]);
+  const { selections, onSelect, options, setOptions } = useSyncedSelectValue(
+    value,
+    onChange
+  );
+  const [isExpanded, setIsExpanded] = useState(false);
+
+  const toggleExpanded = () => {
+    setIsExpanded(!isExpanded);
+  };
+
   useEffect(() => {
     loadLabelOptions(setOptions, onError);
-  }, [onError]);
+    /* eslint-disable-next-line react-hooks/exhaustive-deps */
+  }, []);
+
+  const renderOptions = opts => {
+    return opts.map(option => (
+      <SelectOption key={option.id} value={option}>
+        {option.name}
+      </SelectOption>
+    ));
+  };
 
   return (
-    <MultiSelect
+    <Select
-      onChange={onChange}
+      variant={SelectVariant.typeaheadMulti}
-      value={value}
+      onToggle={toggleExpanded}
-      options={options}
+      onSelect={onSelect}
-      createNewItem={name => ({
+      onClear={() => onChange([])}
-        id: name,
+      onFilter={event => {
-        name,
+        const str = event.target.value.toLowerCase();
-        isNew: true,
+        const matches = options.filter(o => o.name.toLowerCase().includes(str));
-      })}
+        return renderOptions(matches);
-    />
+      }}
+      selections={selections}
+      isExpanded={isExpanded}
+      ariaLabelledBy="label-select"
+      placeholderText={placeholder}
+    >
+      {renderOptions(options)}
+    </Select>
   );
 }
 LabelSelect.propTypes = {
@@ -55,7 +80,12 @@ LabelSelect.propTypes = {
       name: string.isRequired,
     })
   ).isRequired,
+  placeholder: string,
+  onChange: func.isRequired,
   onError: func.isRequired,
 };
+LabelSelect.defaultProps = {
+  placeholder: '',
+};
 
 export default LabelSelect;

awx/ui_next/src/screens/Template/shared/LabelSelect.test.jsx

@@ -19,12 +19,16 @@ describe('<LabelSelect />', () => {
     });
     let wrapper;
     await act(async () => {
-      wrapper = mount(<LabelSelect value={[]} onError={() => {}} />);
+      wrapper = mount(
+        <LabelSelect value={[]} onError={() => {}} onChange={() => {}} />
+      );
     });
-    wrapper.update();
-
     expect(LabelsAPI.read).toHaveBeenCalledTimes(1);
-    expect(wrapper.find('MultiSelect').prop('options')).toEqual(options);
+    wrapper.find('input').simulate('focus');
+    const selectOptions = wrapper.find('SelectOption');
+    expect(selectOptions).toHaveLength(2);
+    expect(selectOptions.at(0).prop('value')).toEqual(options[0]);
+    expect(selectOptions.at(1).prop('value')).toEqual(options[1]);
   });
 
   test('should fetch two pages labels if present', async () => {
@@ -36,19 +40,20 @@ describe('<LabelSelect />', () => {
     });
     LabelsAPI.read.mockReturnValueOnce({
       data: {
-        results: options,
+        results: [{ id: 3, name: 'three' }, { id: 4, name: 'four' }],
       },
     });
     let wrapper;
     await act(async () => {
-      wrapper = mount(<LabelSelect value={[]} onError={() => {}} />);
+      wrapper = mount(
+        <LabelSelect value={[]} onError={() => {}} onChange={() => {}} />
+      );
     });
     wrapper.update();
 
     expect(LabelsAPI.read).toHaveBeenCalledTimes(2);
-    expect(wrapper.find('MultiSelect').prop('options')).toEqual([
+    wrapper.find('input').simulate('focus');
-      ...options,
+    const selectOptions = wrapper.find('SelectOption');
-      ...options,
+    expect(selectOptions).toHaveLength(4);
-    ]);
   });
 });