From 193a041ef9c7eea243a02b5712f019b35e5366ee Mon Sep 17 00:00:00 2001
From: Keith Grant <keithjgrant@redhat.com>
Date: Thu, 9 Jan 2020 15:40:06 -0800
Subject: [PATCH] finish usePFSelect hook

---
 .../src/components/MultiSelect/index.js       |  1 +
 .../src/components/MultiSelect/usePFSelect.js | 51 ++++++++++++++++++
 .../screens/Template/shared/LabelSelect.jsx   | 52 ++++---------------
 3 files changed, 63 insertions(+), 41 deletions(-)
 create mode 100644 awx/ui_next/src/components/MultiSelect/usePFSelect.js

diff --git a/awx/ui_next/src/components/MultiSelect/index.js b/awx/ui_next/src/components/MultiSelect/index.js
index d983765272..145029344a 100644
--- a/awx/ui_next/src/components/MultiSelect/index.js
+++ b/awx/ui_next/src/components/MultiSelect/index.js
@@ -1,2 +1,3 @@
 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
new file mode 100644
index 0000000000..d6f2be8646
--- /dev/null
+++ b/awx/ui_next/src/components/MultiSelect/usePFSelect.js
@@ -0,0 +1,51 @@
+import { useState, useEffect } from 'react';
+
+/*
+  Hook for using PatternFly's <Select> component. Guarantees object equality
+  between objects in `value` and the corresponding objects loaded as
+  `options` (based on matched id value).
+ */
+export default function usePFSelect(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 this.id;
+}
diff --git a/awx/ui_next/src/screens/Template/shared/LabelSelect.jsx b/awx/ui_next/src/screens/Template/shared/LabelSelect.jsx
index c9072e2950..24f1c2fe64 100644
--- a/awx/ui_next/src/screens/Template/shared/LabelSelect.jsx
+++ b/awx/ui_next/src/screens/Template/shared/LabelSelect.jsx
@@ -2,40 +2,7 @@ import React, { useState, useEffect } from 'react';
 import { func, arrayOf, number, shape, string, oneOfType } from 'prop-types';
 import { Select, SelectOption, SelectVariant } from '@patternfly/react-core';
 import { LabelsAPI } from '@api';
-
-function setToString(labels) {
-  labels.forEach(label => {
-    label.toString = function toString() {
-      return this.id;
-    };
-  });
-  return labels;
-}
-
-// TODO: extract to shared file
-function usePFSelect(value, options, onChange) {
-  const [currentValue, setCurrentValue] = useState([]);
-
-  useEffect(() => {
-    if (value !== currentValue && options.length) {
-      const syncedValue = value.map(item =>
-        options.find(i => i.id === item.id)
-      );
-      setCurrentValue(syncedValue);
-    }
-    /* eslint-disable-next-line react-hooks/exhaustive-deps */
-  }, [value, options]);
-
-  const handleSelect = (event, item) => {
-    if (currentValue.includes(item)) {
-      onChange(currentValue.filter(i => i.id !== item.id));
-    } else {
-      onChange(currentValue.concat(item));
-    }
-  };
-
-  return [currentValue, handleSelect];
-}
+import usePFSelect from '@components/MultiSelect/usePFSelect';
 
 async function loadLabelOptions(setLabels, onError) {
   let labels;
@@ -45,7 +12,7 @@ async function loadLabelOptions(setLabels, onError) {
       page_size: 200,
       order_by: 'name',
     });
-    labels = setToString(data.results);
+    labels = data.results;
     setLabels(labels);
     if (data.next && data.next.includes('page=2')) {
       const {
@@ -55,7 +22,7 @@ async function loadLabelOptions(setLabels, onError) {
         page_size: 200,
         order_by: 'name',
       });
-      setLabels(labels.concat(setToString(results)));
+      setLabels(labels.concat(results));
     }
   } catch (err) {
     onError(err);
@@ -63,8 +30,10 @@ async function loadLabelOptions(setLabels, onError) {
 }
 
 function LabelSelect({ value, placeholder, onChange, onError }) {
-  const [options, setOptions] = useState([]);
-  const [currentValue, handleSelect] = usePFSelect(value, options, onChange);
+  const { selections, onSelect, options, setOptions } = usePFSelect(
+    value,
+    onChange
+  );
   const [isExpanded, setIsExpanded] = useState(false);
 
   const toggleExpanded = () => {
@@ -73,7 +42,8 @@ function LabelSelect({ value, placeholder, onChange, onError }) {
 
   useEffect(() => {
     loadLabelOptions(setOptions, onError);
-  }, [onError]);
+    /* eslint-disable-next-line react-hooks/exhaustive-deps */
+  }, []);
 
   const renderOptions = opts => {
     return opts.map(option => (
@@ -88,14 +58,14 @@ function LabelSelect({ value, placeholder, onChange, onError }) {
       variant={SelectVariant.typeaheadMulti}
       aria-label="Select a state"
       onToggle={toggleExpanded}
-      onSelect={handleSelect}
+      onSelect={onSelect}
       onClear={() => onChange([])}
       onFilter={event => {
         const str = event.target.value.toLowerCase();
         const matches = options.filter(o => o.name.toLowerCase().includes(str));
         return renderOptions(matches);
       }}
-      selections={options.length ? setToString(currentValue) : []}
+      selections={selections}
       isExpanded={isExpanded}
       ariaLabelledBy="label-select"
       placeholderText={placeholder}