Merge pull request #7081 from keithjgrant/6640-jt-form-loading

JT form loading UX

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

awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx

@@ -1,4 +1,4 @@
-import React, { useState, useEffect } from 'react';
+import React, { useCallback, useEffect } from 'react';
 import { arrayOf, string, func, object, bool } from 'prop-types';
 import { withRouter } from 'react-router-dom';
 import { withI18n } from '@lingui/react';
@@ -8,6 +8,7 @@ import { InstanceGroupsAPI } from '../../api';
 import { getQSConfig, parseQueryString } from '../../util/qs';
 import { FieldTooltip } from '../FormField';
 import OptionsList from '../OptionsList';
+import useRequest from '../../util/useRequest';
 import Lookup from './Lookup';
 import LookupErrorMessage from './shared/LookupErrorMessage';
 
@@ -27,22 +28,27 @@ function InstanceGroupsLookup(props) {
     history,
     i18n,
   } = props;
-  const [instanceGroups, setInstanceGroups] = useState([]);
-  const [count, setCount] = useState(0);
-  const [error, setError] = useState(null);
+
+  const {
+    result: { instanceGroups, count },
+    request: fetchInstanceGroups,
+    error,
+    isLoading,
+  } = useRequest(
+    useCallback(async () => {
+      const params = parseQueryString(QS_CONFIG, history.location.search);
+      const { data } = await InstanceGroupsAPI.read(params);
+      return {
+        instanceGroups: data.results,
+        count: data.count,
+      };
+    }, [history.location]),
+    { instanceGroups: [], count: 0 }
+  );
 
   useEffect(() => {
-    (async () => {
-      const params = parseQueryString(QS_CONFIG, history.location.search);
-      try {
-        const { data } = await InstanceGroupsAPI.read(params);
-        setInstanceGroups(data.results);
-        setCount(data.count);
-      } catch (err) {
-        setError(err);
-      }
-    })();
-  }, [history.location]);
+    fetchInstanceGroups();
+  }, [fetchInstanceGroups]);
 
   return (
     <FormGroup
@@ -59,6 +65,7 @@ function InstanceGroupsLookup(props) {
         qsConfig={QS_CONFIG}
         multiple
         required={required}
+        isLoading={isLoading}
         renderOptionsList={({ state, dispatch, canDelete }) => (
           <OptionsList
             value={state.selectedItems}

awx/ui_next/src/components/Lookup/InventoryLookup.jsx

@@ -19,22 +19,20 @@ const QS_CONFIG = getQSConfig('inventory', {
 
 function InventoryLookup({ value, onChange, onBlur, required, i18n, history }) {
   const {
-    result: { count, inventories },
-    error,
+    result: { inventories, count },
     request: fetchInventories,
+    error,
+    isLoading,
   } = useRequest(
     useCallback(async () => {
       const params = parseQueryString(QS_CONFIG, history.location.search);
       const { data } = await InventoriesAPI.read(params);
       return {
-        count: data.count,
         inventories: data.results,
+        count: data.count,
       };
-    }, [history.location.search]),
-    {
-      count: 0,
-      inventories: [],
-    }
+    }, [history.location]),
+    { inventories: [], count: 0 }
   );
 
   useEffect(() => {
@@ -50,6 +48,7 @@ function InventoryLookup({ value, onChange, onBlur, required, i18n, history }) {
         onChange={onChange}
         onBlur={onBlur}
         required={required}
+        isLoading={isLoading}
         qsConfig={QS_CONFIG}
         renderOptionsList={({ state, dispatch, canDelete }) => (
           <OptionsList

awx/ui_next/src/components/Lookup/Lookup.jsx

@@ -56,6 +56,7 @@ function Lookup(props) {
     header,
     onChange,
     onBlur,
+    isLoading,
     value,
     multiple,
     required,
@@ -124,6 +125,7 @@ function Lookup(props) {
           id={id}
           onClick={() => dispatch({ type: 'TOGGLE_MODAL' })}
           variant={ButtonVariant.tertiary}
+          isDisabled={isLoading}
         >
           <SearchIcon />
         </SearchButton>

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

@@ -159,4 +159,30 @@ describe('<Lookup />', () => {
     const list = wrapper.find('TestList');
     expect(list.prop('canDelete')).toEqual(false);
   });
+
+  test('should be disabled while isLoading is true', async () => {
+    const mockSelected = [{ name: 'foo', id: 1, url: '/api/v2/item/1' }];
+    wrapper = mountWithContexts(
+      <Lookup
+        id="test"
+        multiple
+        header="Foo Bar"
+        value={mockSelected}
+        onChange={onChange}
+        qsConfig={QS_CONFIG}
+        isLoading
+        renderOptionsList={({ state, dispatch, canDelete }) => (
+          <TestList
+            id="options-list"
+            state={state}
+            dispatch={dispatch}
+            canDelete={canDelete}
+          />
+        )}
+      />
+    );
+    checkRootElementNotPresent('body div[role="dialog"]');
+    const button = wrapper.find('button[aria-label="Search"]');
+    expect(button.prop('disabled')).toEqual(true);
+  });
 });

awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx

@@ -1,5 +1,5 @@
 import 'styled-components/macro';
-import React, { Fragment, useState, useEffect } from 'react';
+import React, { Fragment, useState, useCallback, useEffect } from 'react';
 import { withRouter } from 'react-router-dom';
 import PropTypes from 'prop-types';
 import { withI18n } from '@lingui/react';
@@ -9,6 +9,7 @@ import { CredentialsAPI, CredentialTypesAPI } from '../../api';
 import AnsibleSelect from '../AnsibleSelect';
 import CredentialChip from '../CredentialChip';
 import OptionsList from '../OptionsList';
+import useRequest from '../../util/useRequest';
 import { getQSConfig, parseQueryString } from '../../util/qs';
 import Lookup from './Lookup';
 
@@ -26,42 +27,62 @@ async function loadCredentials(params, selectedCredentialTypeId) {
 
 function MultiCredentialsLookup(props) {
   const { value, onChange, onError, history, i18n } = props;
-  const [credentialTypes, setCredentialTypes] = useState([]);
   const [selectedType, setSelectedType] = useState(null);
-  const [credentials, setCredentials] = useState([]);
-  const [credentialsCount, setCredentialsCount] = useState(0);
+
+  const {
+    result: credentialTypes,
+    request: fetchTypes,
+    error: typesError,
+    isLoading: isTypesLoading,
+  } = useRequest(
+    useCallback(async () => {
+      const types = await CredentialTypesAPI.loadAllTypes();
+      const match = types.find(type => type.kind === 'ssh') || types[0];
+      setSelectedType(match);
+      return types;
+    }, []),
+    []
+  );
 
   useEffect(() => {
-    (async () => {
-      try {
-        const types = await CredentialTypesAPI.loadAllTypes();
-        setCredentialTypes(types);
-        const match = types.find(type => type.kind === 'ssh') || types[0];
-        setSelectedType(match);
-      } catch (err) {
-        onError(err);
-      }
-    })();
-  }, [onError]);
+    fetchTypes();
+  }, [fetchTypes]);
 
-  useEffect(() => {
-    (async () => {
+  const {
+    result: { credentials, credentialsCount },
+    request: fetchCredentials,
+    error: credentialsError,
+    isLoading: isCredentialsLoading,
+  } = useRequest(
+    useCallback(async () => {
       if (!selectedType) {
-        return;
+        return {
+          credentials: [],
+          count: 0,
+        };
       }
-      try {
-        const params = parseQueryString(QS_CONFIG, history.location.search);
-        const { results, count } = await loadCredentials(
-          params,
-          selectedType.id
-        );
-        setCredentials(results);
-        setCredentialsCount(count);
-      } catch (err) {
-        onError(err);
-      }
-    })();
-  }, [selectedType, history.location.search, onError]);
+      const params = parseQueryString(QS_CONFIG, history.location.search);
+      const { results, count } = await loadCredentials(params, selectedType.id);
+      return {
+        credentials: results,
+        credentialsCount: count,
+      };
+    }, [selectedType, history.location]),
+    {
+      credentials: [],
+      credentialsCount: 0,
+    }
+  );
+
+  useEffect(() => {
+    fetchCredentials();
+  }, [fetchCredentials]);
+
+  useEffect(() => {
+    if (typesError || credentialsError) {
+      onError(typesError || credentialsError);
+    }
+  }, [typesError, credentialsError, onError]);
 
   const renderChip = ({ item, removeItem, canDelete }) => (
     <CredentialChip
@@ -82,6 +103,7 @@ function MultiCredentialsLookup(props) {
       multiple
       onChange={onChange}
       qsConfig={QS_CONFIG}
+      isLoading={isTypesLoading || isCredentialsLoading}
       renderItemChip={renderChip}
       renderOptionsList={({ state, dispatch, canDelete }) => {
         return (

awx/ui_next/src/components/Lookup/MultiCredentialsLookup.test.jsx

@@ -156,7 +156,7 @@ describe('<MultiCredentialsLookup />', () => {
       });
     });
     wrapper.update();
-    act(() => {
+    await act(async () => {
       wrapper.find('Button[variant="primary"]').invoke('onClick')();
     });
     expect(onChange).toBeCalledWith([
@@ -201,7 +201,7 @@ describe('<MultiCredentialsLookup />', () => {
       });
     });
     wrapper.update();
-    act(() => {
+    await act(async () => {
       wrapper.find('Button[variant="primary"]').invoke('onClick')();
     });
     expect(onChange).toBeCalledWith([
@@ -248,7 +248,7 @@ describe('<MultiCredentialsLookup />', () => {
       });
     });
     wrapper.update();
-    act(() => {
+    await act(async () => {
       wrapper.find('Button[variant="primary"]').invoke('onClick')();
     });
     expect(onChange).toBeCalledWith([
@@ -301,7 +301,7 @@ describe('<MultiCredentialsLookup />', () => {
       });
     });
     wrapper.update();
-    act(() => {
+    await act(async () => {
       wrapper.find('Button[variant="primary"]').invoke('onClick')();
     });
     expect(onChange).toBeCalledWith([

awx/ui_next/src/components/Lookup/ProjectLookup.jsx

@@ -32,9 +32,10 @@ function ProjectLookup({
   history,
 }) {
   const {
-    result: { count, projects },
-    error,
+    result: { projects, count },
     request: fetchProjects,
+    error,
+    isLoading,
   } = useRequest(
     useCallback(async () => {
       const params = parseQueryString(QS_CONFIG, history.location.search);
@@ -74,6 +75,7 @@ function ProjectLookup({
         onBlur={onBlur}
         onChange={onChange}
         required={required}
+        isLoading={isLoading}
         qsConfig={QS_CONFIG}
         renderOptionsList={({ state, dispatch, canDelete }) => (
           <OptionsList

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

@@ -30,6 +30,7 @@ async function loadLabelOptions(setLabels, onError) {
 }
 
 function LabelSelect({ value, placeholder, onChange, onError, createText }) {
+  const [isLoading, setIsLoading] = useState(true);
   const { selections, onSelect, options, setOptions } = useSyncedSelectValue(
     value,
     onChange
@@ -41,7 +42,10 @@ function LabelSelect({ value, placeholder, onChange, onError, createText }) {
   };
 
   useEffect(() => {
-    loadLabelOptions(setOptions, onError);
+    (async () => {
+      await loadLabelOptions(setOptions, onError);
+      setIsLoading(false);
+    })();
     /* eslint-disable-next-line react-hooks/exhaustive-deps */
   }, []);
 
@@ -77,6 +81,7 @@ function LabelSelect({ value, placeholder, onChange, onError, createText }) {
         }
         return label;
       }}
+      isDisabled={isLoading}
       selections={selections}
       isExpanded={isExpanded}
       ariaLabelledBy="label-select"

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

@@ -1,39 +1,51 @@
-import React, { useState, useEffect } from 'react';
+import React, { useCallback, useEffect } from 'react';
 import { number, string, oneOfType } from 'prop-types';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 import AnsibleSelect from '../../../components/AnsibleSelect';
 import { ProjectsAPI } from '../../../api';
+import useRequest from '../../../util/useRequest';
 
 function PlaybookSelect({ projectId, isValid, field, onBlur, onError, i18n }) {
-  const [options, setOptions] = useState([]);
+  const {
+    result: options,
+    request: fetchOptions,
+    isLoading,
+    error,
+  } = useRequest(
+    useCallback(async () => {
+      if (!projectId) {
+        return [];
+      }
+      const { data } = await ProjectsAPI.readPlaybooks(projectId);
+      const opts = (data || []).map(playbook => ({
+        value: playbook,
+        key: playbook,
+        label: playbook,
+        isDisabled: false,
+      }));
+
+      opts.unshift({
+        value: '',
+        key: '',
+        label: i18n._(t`Choose a playbook`),
+        isDisabled: false,
+      });
+      return opts;
+    }, [projectId, i18n]),
+    []
+  );
 
   useEffect(() => {
-    if (!projectId) {
-      return;
-    }
-    (async () => {
-      try {
-        const { data } = await ProjectsAPI.readPlaybooks(projectId);
-        const opts = (data || []).map(playbook => ({
-          value: playbook,
-          key: playbook,
-          label: playbook,
-          isDisabled: false,
-        }));
+    fetchOptions();
+  }, [fetchOptions]);
+
+  useEffect(() => {
+    if (error) {
+      onError(error);
+    }
+  }, [error, onError]);
 
-        opts.unshift({
-          value: '',
-          key: '',
-          label: i18n._(t`Choose a playbook`),
-          isDisabled: false,
-        });
-        setOptions(opts);
-      } catch (contentError) {
-        onError(contentError);
-      }
-    })();
-  }, [projectId, i18n, onError]);
   return (
     <AnsibleSelect
       id="template-playbook"
@@ -41,6 +53,7 @@ function PlaybookSelect({ projectId, isValid, field, onBlur, onError, i18n }) {
       isValid={isValid}
       {...field}
       onBlur={onBlur}
+      isDisabled={isLoading}
     />
   );
 }