From ddc1088c78aeec24da559eae362f2f1eae5007a1 Mon Sep 17 00:00:00 2001
From: Amir Fefer <amirfefer@gmail.com>
Date: Thu, 3 Aug 2023 13:18:53 +0300
Subject: [PATCH] fix(HMS-2256): enhance wizard first page interface

---
 .../InstanceTypesSelect.test.js               |  4 +-
 src/Components/InstanceTypesSelect/index.js   | 34 ++++-----
 .../steps/AccountCustomizations/aws.js        | 73 +++++++++----------
 .../steps/AccountCustomizations/azure.js      | 11 ++-
 .../steps/AccountCustomizations/gcp.js        | 71 +++++++++---------
 src/Components/RegionsSelect/index.js         |  2 +-
 src/Components/SourcesSelect/index.js         |  1 +
 src/Components/TemplateSelect/index.js        |  2 +
 src/mocks/fixtures/instanceTypes.fixtures.js  | 13 ----
 9 files changed, 99 insertions(+), 112 deletions(-)

diff --git a/src/Components/InstanceTypesSelect/InstanceTypesSelect.test.js b/src/Components/InstanceTypesSelect/InstanceTypesSelect.test.js
index c4cb53ad..069392f8 100644
--- a/src/Components/InstanceTypesSelect/InstanceTypesSelect.test.js
+++ b/src/Components/InstanceTypesSelect/InstanceTypesSelect.test.js
@@ -40,8 +40,8 @@ describe('InstanceTypesSelect', () => {
       await mountSelectAndClick();
       const unsupportedInstance = await screen.findByText('t1.nano');
       await userEvent.click(unsupportedInstance);
-      const alert = await screen.findByTestId('unsupported_type_alert');
-      expect(alert).toBeDefined();
+      const selectWrapper = document.querySelector('[data-ouia-component-id=select_instance_type]');
+      expect(selectWrapper).toHaveClass('pf-m-warning');
     });
   });
 });
diff --git a/src/Components/InstanceTypesSelect/index.js b/src/Components/InstanceTypesSelect/index.js
index 6b5b7b01..cd65ab43 100644
--- a/src/Components/InstanceTypesSelect/index.js
+++ b/src/Components/InstanceTypesSelect/index.js
@@ -1,11 +1,11 @@
 import PropTypes from 'prop-types';
 import React from 'react';
-import { Alert, Spinner, Select, SelectOption, TextInput } from '@patternfly/react-core';
+import { Spinner, Select, SelectOption, TextInput } from '@patternfly/react-core';
 import { useQuery } from 'react-query';
 import { fetchInstanceTypesList } from '../../API';
 import { useWizardContext } from '../Common/WizardContext';
 
-const OPTIONS_PER_SCREEN = 5;
+const OPTIONS_PER_SCREEN = 3;
 const sanitizeSearchValue = (str) => str.replace(/\\+$/, '');
 
 const InstanceTypesSelect = ({ setValidation, architecture }) => {
@@ -23,6 +23,9 @@ const InstanceTypesSelect = ({ setValidation, architecture }) => {
     staleTime: 5 * (60 * 1000), // data is considered fresh for 5 minutes (same as cacheTime)
     select: (types) => types.filter((type) => type.architecture === architecture),
     enabled: !!chosenRegion && !!chosenSource,
+    onError: () => {
+      setValidation('error');
+    },
   });
 
   if (!chosenSource || chosenSource === '') {
@@ -51,7 +54,7 @@ const InstanceTypesSelect = ({ setValidation, architecture }) => {
         ...prevState,
         chosenInstanceType: selection,
       }));
-      setValidation('success');
+      chosenInstanceType.supported ? setValidation('success') : setValidation('warning');
       setIsOpen(false);
     }
   };
@@ -96,38 +99,35 @@ const InstanceTypesSelect = ({ setValidation, architecture }) => {
   };
 
   if (error) {
-    console.warn('Failed to fetch instance types list');
     return (
       <>
-        <Alert ouiaId="instance_type_alert" variant="warning" isInline title="There are problems fetching instance types" />
-        <Select ouiaId="instance_type_empty" isDisabled placeholderText="No instance types found" toggleAriaLabel="Select instance type" />
+        <Select
+          validated="error"
+          ouiaId="instance_type_empty"
+          isDisabled
+          placeholderText="No instance types found"
+          toggleAriaLabel="Select instance type"
+        />
       </>
     );
   }
   if (isLoading) {
-    return <Spinner isSVG size="sm" aria-label="Contents of the small example" />;
+    return <Spinner isSVG size="sm" aria-label="loading instance type select" />;
   }
 
   const types = filteredTypes || instanceTypes;
 
   return (
     <>
-      {!isTypeSupported && (
-        <Alert
-          data-testid="unsupported_type_alert"
-          ouiaId="instance_type_not_supported_alert"
-          variant="warning"
-          isInline
-          title="Warning: The selected specification does not meet minimum requirements for this image."
-        />
-      )}
       <Select
         ouiaId="select_instance_type"
         variant="typeahead"
+        id="instance_type_select"
+        validated={!isTypeSupported && 'warning'}
         typeAheadAriaLabel="Selected instance type"
         toggleAriaLabel="Select instance type"
         placeholderText="Select instance type"
-        maxHeight="450px"
+        maxHeight="180px"
         isOpen={isOpen}
         selections={chosenInstanceType}
         onToggle={onToggle}
diff --git a/src/Components/ProvisioningWizard/steps/AccountCustomizations/aws.js b/src/Components/ProvisioningWizard/steps/AccountCustomizations/aws.js
index 50558485..a659067b 100644
--- a/src/Components/ProvisioningWizard/steps/AccountCustomizations/aws.js
+++ b/src/Components/ProvisioningWizard/steps/AccountCustomizations/aws.js
@@ -1,6 +1,6 @@
 import PropTypes from 'prop-types';
 import React from 'react';
-import { Form, FormGroup, Popover, Title, Text, Button } from '@patternfly/react-core';
+import { Form, FormGroup, Popover, Title, Button } from '@patternfly/react-core';
 import { HelpIcon } from '@patternfly/react-icons';
 
 import { AWS_PROVIDER } from '../../../../constants';
@@ -29,7 +29,7 @@ const AccountCustomizationsAWS = ({ setStepValidated, image }) => {
 
   React.useEffect(() => {
     // This effect checks if the entire step is validated
-    const errorExists = Object.values(validations).some((valid) => valid !== 'success');
+    const errorExists = Object.values(validations).some((valid) => valid == 'error' || valid == 'default');
     setStepValidated(!errorExists);
   }, [validations]);
 
@@ -38,9 +38,6 @@ const AccountCustomizationsAWS = ({ setStepValidated, image }) => {
       <Title ouiaId="account_custom_title" headingLevel="h1" size="xl">
         Account and customizations | Amazon
       </Title>
-      <Text ouiaId="account_custom_description">
-        Configure instances that will run on your AWS. All the instances will launch with the same configuration.
-      </Text>
       <FormGroup
         label="Select account"
         validated={validations.sources}
@@ -80,41 +77,12 @@ const AccountCustomizationsAWS = ({ setStepValidated, image }) => {
       >
         <RegionsSelect provider={AWS_PROVIDER} onChange={onRegionChange} composeID={image.id} currentRegion={chosenRegion} />
       </FormGroup>
-      <FormGroup
-        label="Select template"
-        fieldId="aws-select-template"
-        labelIcon={
-          <Popover
-            bodyContent={
-              <span>
-                Launch templates contains the configuration information to launch an instance. Note that instance type and public SSH key will be
-                still required and will override template values. For further information and for creating launch templates{' '}
-                <a rel="noreferrer" target="_blank" href="https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-launch-templates.html">
-                  click here
-                </a>
-              </span>
-            }
-          >
-            <Button
-              ouiaId="template_help"
-              type="button"
-              aria-label="template field info"
-              onClick={(e) => e.preventDefault()}
-              aria-describedby="aws-select-template"
-              className="pf-c-form__group-label-help"
-              variant="plain"
-            >
-              <HelpIcon noVerticalAlign />
-            </Button>
-          </Popover>
-        }
-      >
-        <TemplatesSelect />
-      </FormGroup>
       <FormGroup
         label="Select instance type"
         isRequired
-        helperTextInvalid="Please pick a value"
+        validated={validations.types}
+        helperTextInvalid="There are problems fetching instance types."
+        helperText={validations.types === 'warning' && 'The selected specification does not meet minimum requirements for this image'}
         fieldId="aws-select-instance-types"
         labelIcon={
           <Popover
@@ -145,6 +113,37 @@ const AccountCustomizationsAWS = ({ setStepValidated, image }) => {
           }
         />
       </FormGroup>
+      <FormGroup
+        label="Select template"
+        fieldId="aws-select-template"
+        labelIcon={
+          <Popover
+            bodyContent={
+              <span>
+                Launch templates contains the configuration information to launch an instance. Note that instance type and public SSH key will be
+                still required and will override template values. For further information and for creating launch templates{' '}
+                <a rel="noreferrer" target="_blank" href="https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-launch-templates.html">
+                  click here
+                </a>
+              </span>
+            }
+          >
+            <Button
+              ouiaId="template_help"
+              type="button"
+              aria-label="template field info"
+              onClick={(e) => e.preventDefault()}
+              aria-describedby="aws-select-template"
+              className="pf-c-form__group-label-help"
+              variant="plain"
+            >
+              <HelpIcon noVerticalAlign />
+            </Button>
+          </Popover>
+        }
+      >
+        <TemplatesSelect />
+      </FormGroup>
       <FormGroup
         label="Count"
         isRequired
diff --git a/src/Components/ProvisioningWizard/steps/AccountCustomizations/azure.js b/src/Components/ProvisioningWizard/steps/AccountCustomizations/azure.js
index a9227cc7..0164b558 100644
--- a/src/Components/ProvisioningWizard/steps/AccountCustomizations/azure.js
+++ b/src/Components/ProvisioningWizard/steps/AccountCustomizations/azure.js
@@ -1,6 +1,6 @@
 import PropTypes from 'prop-types';
 import React from 'react';
-import { Form, FormGroup, Popover, Title, Text, Button } from '@patternfly/react-core';
+import { Form, FormGroup, Popover, Title, Button } from '@patternfly/react-core';
 import { HelpIcon } from '@patternfly/react-icons';
 
 import { AZURE_PROVIDER } from '../../../../constants';
@@ -20,7 +20,7 @@ const AccountCustomizationsAzure = ({ setStepValidated, image }) => {
 
   React.useEffect(() => {
     // This effect checks if the entire step is validated
-    const errorExists = Object.values(validations).some((valid) => valid !== 'success');
+    const errorExists = Object.values(validations).some((valid) => valid == 'error' || valid == 'default');
     setStepValidated(!errorExists);
   }, [validations]);
 
@@ -37,9 +37,6 @@ const AccountCustomizationsAzure = ({ setStepValidated, image }) => {
       <Title ouiaId="account_custom_title" headingLevel="h1" size="xl">
         Account and customizations | Azure
       </Title>
-      <Text ouiaId="account_custom_description">
-        Configure instances that will run on your Azure cloud. All the instances will launch with the same configuration.
-      </Text>
       <FormGroup
         label="Select account"
         validated={validations.sources}
@@ -82,7 +79,9 @@ const AccountCustomizationsAzure = ({ setStepValidated, image }) => {
       <FormGroup
         label="Select instance size"
         isRequired
-        helperTextInvalid="Please pick a value"
+        validated={validations.types}
+        helperTextInvalid="There are problems fetching instance types."
+        helperText={validations.types === 'warning' && 'The selected specification does not meet minimum requirements for this image'}
         fieldId="azure-select-instance-size"
         labelIcon={
           <Popover headerContent={<div>Azure instance sizes</div>}>
diff --git a/src/Components/ProvisioningWizard/steps/AccountCustomizations/gcp.js b/src/Components/ProvisioningWizard/steps/AccountCustomizations/gcp.js
index 20a7849d..ae568e2c 100644
--- a/src/Components/ProvisioningWizard/steps/AccountCustomizations/gcp.js
+++ b/src/Components/ProvisioningWizard/steps/AccountCustomizations/gcp.js
@@ -1,6 +1,6 @@
 import PropTypes from 'prop-types';
 import React from 'react';
-import { Form, FormGroup, Popover, Title, Text, Button } from '@patternfly/react-core';
+import { Form, FormGroup, Popover, Title, Button } from '@patternfly/react-core';
 import { HelpIcon } from '@patternfly/react-icons';
 
 import { GCP_PROVIDER } from '../../../../constants';
@@ -21,7 +21,7 @@ const AccountCustomizationsGCP = ({ setStepValidated, image }) => {
 
   React.useEffect(() => {
     // This effect checks if the entire step is validated
-    const errorExists = Object.values(validations).some((valid) => valid !== 'success');
+    const errorExists = Object.values(validations).some((valid) => valid == 'error' || valid == 'default');
     setStepValidated(!errorExists);
   }, [validations]);
 
@@ -38,9 +38,6 @@ const AccountCustomizationsGCP = ({ setStepValidated, image }) => {
       <Title ouiaId="account_custom_title" headingLevel="h1" size="xl">
         Account and customizations | Google cloud
       </Title>
-      <Text ouiaId="account_custom_description">
-        Configure instances that will run on your Google cloud. All the instances will launch with the same configuration.
-      </Text>
       <FormGroup
         label="Select account"
         validated={validations.sources}
@@ -80,6 +77,39 @@ const AccountCustomizationsGCP = ({ setStepValidated, image }) => {
       >
         <RegionsSelect provider={GCP_PROVIDER} onChange={onRegionChange} composeID={image.id} currentRegion={wizardContext.chosenRegion} />
       </FormGroup>
+      <FormGroup
+        label="Select machine type"
+        isRequired
+        validated={validations.types}
+        helperTextInvalid="There are problems fetching instance types."
+        helperText={validations.types === 'warning' && 'The selected specification does not meet minimum requirements for this image'}
+        fieldId="gcp-select-machine-types"
+        labelIcon={
+          <Popover headerContent={<div>GCP machine types</div>}>
+            <Button
+              ouiaId="machine_type_help"
+              type="button"
+              aria-label="More info for machine types field"
+              onClick={(e) => e.preventDefault()}
+              aria-describedby="gcp-select-machine-types"
+              className="pf-c-form__group-label-help"
+              variant="plain"
+            >
+              <HelpIcon noVerticalAlign />
+            </Button>
+          </Popover>
+        }
+      >
+        <InstanceTypesSelect
+          architecture={image.architecture}
+          setValidation={(validation) =>
+            setValidation((prevValidations) => ({
+              ...prevValidations,
+              types: validation,
+            }))
+          }
+        />
+      </FormGroup>
       <FormGroup
         label="Select template"
         fieldId="gcp-select-template"
@@ -111,37 +141,6 @@ const AccountCustomizationsGCP = ({ setStepValidated, image }) => {
       >
         <TemplatesSelect />
       </FormGroup>
-      <FormGroup
-        label="Select machine type"
-        isRequired
-        helperTextInvalid="Please pick a value"
-        fieldId="gcp-select-machine-types"
-        labelIcon={
-          <Popover headerContent={<div>GCP machine types</div>}>
-            <Button
-              ouiaId="machine_type_help"
-              type="button"
-              aria-label="More info for machine types field"
-              onClick={(e) => e.preventDefault()}
-              aria-describedby="gcp-select-machine-types"
-              className="pf-c-form__group-label-help"
-              variant="plain"
-            >
-              <HelpIcon noVerticalAlign />
-            </Button>
-          </Popover>
-        }
-      >
-        <InstanceTypesSelect
-          architecture={image.architecture}
-          setValidation={(validation) =>
-            setValidation((prevValidations) => ({
-              ...prevValidations,
-              types: validation,
-            }))
-          }
-        />
-      </FormGroup>
       <FormGroup
         label="Count"
         isRequired
diff --git a/src/Components/RegionsSelect/index.js b/src/Components/RegionsSelect/index.js
index c528bad7..3fcf3108 100644
--- a/src/Components/RegionsSelect/index.js
+++ b/src/Components/RegionsSelect/index.js
@@ -66,7 +66,7 @@ const RegionsSelect = ({ provider, currentRegion, composeID, onChange }) => {
       variant="typeahead"
       aria-label="Select region"
       label="Select region"
-      maxHeight="450px"
+      maxHeight="180px"
       isOpen={isOpen}
       selections={currentRegion}
       onToggle={onToggle}
diff --git a/src/Components/SourcesSelect/index.js b/src/Components/SourcesSelect/index.js
index 03e82e51..d769f61e 100644
--- a/src/Components/SourcesSelect/index.js
+++ b/src/Components/SourcesSelect/index.js
@@ -70,6 +70,7 @@ const SourcesSelect = ({ setValidation, image }) => {
       isOpen={isOpen}
       onToggle={(openState) => setIsOpen(openState)}
       selections={selected}
+      maxHeight="180px"
       // TODO decide if to disable the select
       // isDisabled={sources?.length === 1}
       onSelect={onSelect}
diff --git a/src/Components/TemplateSelect/index.js b/src/Components/TemplateSelect/index.js
index 885f5469..d1e94be4 100644
--- a/src/Components/TemplateSelect/index.js
+++ b/src/Components/TemplateSelect/index.js
@@ -48,9 +48,11 @@ const TemplatesSelect = () => {
     <Select
       ouiaId="select_templates"
       isOpen={isOpen}
+      direction="up"
       onToggle={(openState) => setIsOpen(openState)}
       selections={chosenTemplateName}
       onSelect={onSelect}
+      maxHeight="180px"
       placeholderText={templates?.length === 0 ? 'No template found' : 'Select templates'}
       aria-label="Select templates"
       clearSelectionsAriaLabel="clear template selection"
diff --git a/src/mocks/fixtures/instanceTypes.fixtures.js b/src/mocks/fixtures/instanceTypes.fixtures.js
index 1f28d08c..88954d80 100644
--- a/src/mocks/fixtures/instanceTypes.fixtures.js
+++ b/src/mocks/fixtures/instanceTypes.fixtures.js
@@ -76,19 +76,6 @@ export const azureInstanceTypeList = [
       gen_v2: false,
     },
   },
-  {
-    name: 'Standard_B1ls',
-    vcpus: 1,
-    cores: 1,
-    memory_mib: 500,
-    storage_gb: 4,
-    supported: false,
-    architecture: 'x86_64',
-    azure: {
-      gen_v1: true,
-      gen_v2: true,
-    },
-  },
   {
     name: 'Standard_D16ps_v5',
     vcpus: 16,