Skip to content

Commit

Permalink
fix(HMS-2256): enhance wizard first page interface
Browse files Browse the repository at this point in the history
  • Loading branch information
amirfefer committed Aug 3, 2023
1 parent d107e40 commit ddc1088
Show file tree
Hide file tree
Showing 9 changed files with 99 additions and 112 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
});
Expand Down
34 changes: 17 additions & 17 deletions src/Components/InstanceTypesSelect/index.js
Original file line number Diff line number Diff line change
@@ -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 }) => {
Expand All @@ -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 === '') {
Expand Down Expand Up @@ -51,7 +54,7 @@ const InstanceTypesSelect = ({ setValidation, architecture }) => {
...prevState,
chosenInstanceType: selection,
}));
setValidation('success');
chosenInstanceType.supported ? setValidation('success') : setValidation('warning');
setIsOpen(false);
}
};
Expand Down Expand Up @@ -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}
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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]);

Expand All @@ -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}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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]);

Expand All @@ -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}
Expand Down Expand Up @@ -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>}>
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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]);

Expand All @@ -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}
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/Components/RegionsSelect/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
1 change: 1 addition & 0 deletions src/Components/SourcesSelect/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
2 changes: 2 additions & 0 deletions src/Components/TemplateSelect/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
13 changes: 0 additions & 13 deletions src/mocks/fixtures/instanceTypes.fixtures.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit ddc1088

Please sign in to comment.