Skip to content

Commit

Permalink
Fixes 1871108: usability bug: the ui selections are unclear
Browse files Browse the repository at this point in the history
  • Loading branch information
Gilad Lekner authored and glekner committed Sep 14, 2020
1 parent 5e61c7c commit 33567bd
Show file tree
Hide file tree
Showing 20 changed files with 683 additions and 185 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export enum FormFieldType {
TEXT = 'TEXT',
TEXT_AREA = 'TEXT_AREA',
SELECT = 'SELECT',
PF_SELECT = 'PF_SELECT',
CHECKBOX = 'CHECKBOX',
INLINE_CHECKBOX = 'INLINE_CHECKBOX',
FILE_UPLOAD = 'FILE_UPLOAD',
Expand All @@ -35,6 +36,7 @@ const hasIsDisabled = new Set([
FormFieldType.INLINE_CHECKBOX,
FormFieldType.FILE_UPLOAD,
FormFieldType.CUSTOM,
FormFieldType.PF_SELECT,
]);
const hasDisabled = new Set([FormFieldType.TEXT_AREA]);
const hasIsChecked = new Set([FormFieldType.CHECKBOX, FormFieldType.INLINE_CHECKBOX]);
Expand All @@ -54,6 +56,8 @@ const hasIsRequired = new Set([
FormFieldType.CUSTOM,
]);
const hasLabel = new Set([FormFieldType.INLINE_CHECKBOX]);
const hasSelections = new Set([FormFieldType.PF_SELECT]);
const hasPlaceholderText = new Set([FormFieldType.PF_SELECT]);

const validatedValidationErrorTypes = new Set([
ValidationErrorType.Error,
Expand Down Expand Up @@ -99,6 +103,8 @@ export const FormField: React.FC<FormFieldProps> = ({ children, isDisabled, valu
validated: set(hasValidated, validated),
id: getFieldId(key),
label: set(hasLabel, getFieldTitle(key)),
selections: set(hasSelections, val),
placeholderText: set(hasPlaceholderText, getPlaceholder(key)),
},
_.isUndefined,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
VMWareProviderField,
OvirtProviderField,
} from '../types';
import { ProvisionSource } from '../../../constants/vm/provision-source';

export const titleResolver: RenderableFieldResolver = {
[OvirtProviderField.OVIRT_ENGINE_SECRET_NAME]: 'RHV Instance',
Expand Down Expand Up @@ -35,7 +34,7 @@ export const titleResolver: RenderableFieldResolver = {
[VMSettingsField.MEMORY]: 'Memory',
[VMSettingsField.CPU]: 'CPUs',
[VMSettingsField.WORKLOAD_PROFILE]: 'Workload Profile',
[VMSettingsField.PROVISION_SOURCE_TYPE]: 'Source',
[VMSettingsField.PROVISION_SOURCE_TYPE]: 'Boot Source',
[VMSettingsField.CONTAINER_IMAGE]: 'Container Image',
[VMSettingsField.IMAGE_URL]: 'URL',
[VMSettingsField.START_VM]: 'Start virtual machine on creation',
Expand All @@ -54,13 +53,6 @@ export const placeholderResolver = {
[VMSettingsField.PROVISION_SOURCE_TYPE]: '--- Select Source ---',
};

const provisionSourceHelpResolver = {
[ProvisionSource.URL.getValue()]: 'An external URL to the .iso, .img, .qcow2 or .raw that the virtual machine should be created from.',
[ProvisionSource.PXE.getValue()]: 'Discover provisionable virtual machines over the network.',
[ProvisionSource.CONTAINER.getValue()]: 'Ephemeral virtual machine disk image which will be pulled from container registry.',
[ProvisionSource.DISK.getValue()]: 'Select an existing PVC in Storage tab',
};

const providerHelpResolver = {
[VMImportProvider.VMWARE]:
'The virtual machine will be imported from a vCenter instance. Please provide connection details and select the virtual machine.',
Expand Down Expand Up @@ -89,6 +81,6 @@ export const helpResolver = {
'The number of virtual CPU cores that will be dedicated to the virtual machine.',
[VMSettingsField.WORKLOAD_PROFILE]: () =>
'The category of workload that this virtual machine will be used for.',
[VMSettingsField.PROVISION_SOURCE_TYPE]: (sourceType: string) =>
provisionSourceHelpResolver[sourceType],
[VMSettingsField.PROVISION_SOURCE_TYPE]: () =>
'Select a method of adding an operating system image source',
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { FormField, FormFieldType } from '../../form/form-field';
import { VMWizardStorage } from '../../types';
import { VolumeWrapper } from '../../../../k8s/wrapper/vm/volume-wrapper';
import { VolumeType } from '../../../../constants/vm/storage';
import { EXAMPLE_CONTAINER } from '../../../../utils/strings';

export const ContainerSource: React.FC<ContainerSourceProps> = React.memo(
({ field, provisionSourceStorage, onProvisionSourceStorageChange }) => {
Expand Down Expand Up @@ -34,6 +35,9 @@ export const ContainerSource: React.FC<ContainerSourceProps> = React.memo(
}
/>
</FormField>
<div className="pf-c-form__helper-text" aria-live="polite">
Example: {EXAMPLE_CONTAINER}
</div>
</FormFieldRow>
);
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
import * as _ from 'lodash';
import * as React from 'react';
import { SelectOption } from '@patternfly/react-core';
import { ValidationErrorType, asValidationObject } from '@console/shared/src/utils/validation';
import {
iGetIsLoaded,
iGetLoadedData,
iGetLoadError,
immutableListToShallowJS,
toShallowJS,
} from '../../../../utils/immutable';
import { FormFieldRow } from '../../form/form-field-row';
import { FormField, FormFieldType } from '../../form/form-field';
import {
getFlavors,
getWorkloadProfiles,
} from '../../../../selectors/vm-template/combined-dependent';
import { flavorSort } from '../../../../utils/sort';
import { VMSettingsField } from '../../types';
import { iGetFieldValue } from '../../selectors/immutable/field';
import { nullOnEmptyChange } from '../../utils/utils';
import { FormPFSelect } from '../../../form/form-pf-select';

export const Flavor: React.FC<FlavorProps> = React.memo(
({
iUserTemplate,
commonTemplates,
os,
flavorField,
workloadProfile,
cnvBaseImages,
onChange,
openshiftFlag,
}) => {
const flavor = iGetFieldValue(flavorField);
const isUserTemplateValid = iGetIsLoaded(iUserTemplate) && !iGetLoadError(iUserTemplate);

const params = {
flavor,
workload: workloadProfile,
os,
};

const templates = iUserTemplate
? isUserTemplateValid
? [toShallowJS(iGetLoadedData(iUserTemplate))]
: []
: immutableListToShallowJS(iGetLoadedData(commonTemplates));

const flavors = flavorSort(getFlavors(templates, params));

const workloadProfiles = getWorkloadProfiles(templates, params);

const loadingResources = openshiftFlag
? {
commonTemplates,
cnvBaseImages,
}
: {};

if (openshiftFlag && iUserTemplate) {
Object.assign(loadingResources, { iUserTemplate });
}

let flavorValidation;

if (
iGetIsLoaded(commonTemplates) &&
(!iUserTemplate || isUserTemplateValid) &&
(flavors.length === 0 || workloadProfiles.length === 0)
) {
const validation = asValidationObject(
'There is no valid template for this combination. Please install required template or select different os/flavor/workload profile combination.',
ValidationErrorType.Info,
);
if (!flavorField.get('validation')) {
flavorValidation = validation;
}
}

return (
<FormFieldRow
field={flavorField}
fieldType={FormFieldType.PF_SELECT}
validation={flavorValidation}
loadingResources={loadingResources}
>
<FormField value={flavor} isDisabled={flavor && flavors.length === 1}>
<FormPFSelect
onSelect={(e, v) => nullOnEmptyChange(onChange, VMSettingsField.FLAVOR)(v.toString())}
>
{flavors.map((f) => {
return (
<SelectOption key={f} value={f}>
{_.capitalize(f)}
</SelectOption>
);
})}
</FormPFSelect>
</FormField>
</FormFieldRow>
);
},
);

type FlavorProps = {
iUserTemplate: any;
commonTemplates: any;
flavorField: any;
os: string;
workloadProfile: string;
cnvBaseImages: any;
openshiftFlag: boolean;
onChange: (key: string, value: string | boolean) => void;
};
Original file line number Diff line number Diff line change
@@ -1,13 +1,5 @@
import * as _ from 'lodash';
import * as React from 'react';
import {
FormSelect,
FormSelectOption,
Checkbox,
Text,
Button,
ButtonVariant,
} from '@patternfly/react-core';
import { Checkbox, Text, Button, ButtonVariant, SelectOption } from '@patternfly/react-core';
import { Link } from 'react-router-dom';
import { ValidationErrorType, asValidationObject } from '@console/shared/src/utils/validation';
import {
Expand All @@ -20,7 +12,6 @@ import {
} from '../../../../utils/immutable';
import { FormFieldRow } from '../../form/form-field-row';
import { FormField, FormFieldType } from '../../form/form-field';
import { FormSelectPlaceholderOption } from '../../../form/form-select-placeholder-option';
import {
getFlavors,
getWorkloadProfiles,
Expand All @@ -29,7 +20,7 @@ import { flavorSort, ignoreCaseSort } from '../../../../utils/sort';
import { pluralize } from '../../../../utils/strings';
import { VMSettingsField } from '../../types';
import { iGetFieldValue } from '../../selectors/immutable/field';
import { getPlaceholder, getFieldId } from '../../utils/renderable-field-utils';
import { getFieldId } from '../../utils/renderable-field-utils';
import { nullOnEmptyChange } from '../../utils/utils';
import { operatingSystemsNative } from '../../../../constants/vm-templates/os';
import { OperatingSystemRecord } from '../../../../types';
Expand All @@ -51,22 +42,22 @@ import {
CDI_UPLOAD_RUNNING,
} from '../../../cdi-upload-provider/consts';
import { getTemplateOperatingSystems } from '../../../../selectors/vm-template/advanced';
import { FormPFSelect } from '../../../form/form-pf-select';

export const OSFlavor: React.FC<OSFlavorProps> = React.memo(
export const OS: React.FC<OSProps> = React.memo(
({
iUserTemplate,
commonTemplates,
operatinSystemField,
cloneBaseDiskImageField,
mountWindowsGuestToolsField,
flavorField,
flavor,
workloadProfile,
cnvBaseImages,
onChange,
openshiftFlag,
goToStorageStep,
}) => {
const flavor = iGetFieldValue(flavorField);
const os = iGetFieldValue(operatinSystemField);
const display = iGet(operatinSystemField, 'display');
const displayOnly = !!display;
Expand Down Expand Up @@ -112,7 +103,6 @@ export const OSFlavor: React.FC<OSFlavorProps> = React.memo(
}

let operatingSystemValidation;
let flavorValidation;

if (
iGetIsLoaded(commonTemplates) &&
Expand All @@ -125,8 +115,6 @@ export const OSFlavor: React.FC<OSFlavorProps> = React.memo(
);
if (!operatinSystemField.get('validation')) {
operatingSystemValidation = validation;
} else if (!flavorField.get('validation')) {
flavorValidation = validation;
}
}

Expand Down Expand Up @@ -199,22 +187,23 @@ export const OSFlavor: React.FC<OSFlavorProps> = React.memo(
<>
<FormFieldRow
field={operatinSystemField}
fieldType={FormFieldType.SELECT}
fieldType={FormFieldType.PF_SELECT}
validation={operatingSystemValidation}
loadingResources={loadingResources}
>
<FormField value={displayOnly ? display : undefined}>
<FormSelect onChange={nullOnEmptyChange(onChange, VMSettingsField.OPERATING_SYSTEM)}>
{!displayOnly && (
<FormSelectPlaceholderOption
placeholder={getPlaceholder(VMSettingsField.OPERATING_SYSTEM)}
isDisabled={!!os}
/>
)}
<FormField value={displayOnly ? display : os}>
<FormPFSelect
onSelect={(e, v) =>
nullOnEmptyChange(onChange, VMSettingsField.OPERATING_SYSTEM)(v.toString())
}
>
{operatingSystemBaseImages.map(({ id, name, message }) => (
<FormSelectOption key={id} value={id} label={`${name || id} ${message}`} />
<SelectOption key={id} value={id}>
{name || id}
{message ? ` ${message}` : ''}
</SelectOption>
))}
</FormSelect>
</FormPFSelect>
</FormField>
{baseImage && baseImage?.longMessage && (
<div className="pf-c-form__helper-text" aria-live="polite">
Expand Down Expand Up @@ -250,33 +239,15 @@ export const OSFlavor: React.FC<OSFlavorProps> = React.memo(
</FormField>
</FormFieldRow>
{mountedDisksHelpMsg}
<FormFieldRow
field={flavorField}
fieldType={FormFieldType.SELECT}
validation={flavorValidation}
loadingResources={loadingResources}
>
<FormField isDisabled={flavor && flavors.length === 1}>
<FormSelect onChange={nullOnEmptyChange(onChange, VMSettingsField.FLAVOR)}>
<FormSelectPlaceholderOption
placeholder={getPlaceholder(VMSettingsField.FLAVOR)}
isDisabled={!!flavor}
/>
{flavors.map((f) => {
return <FormSelectOption key={f} value={f} label={_.capitalize(f)} />;
})}
</FormSelect>
</FormField>
</FormFieldRow>
</>
);
},
);

type OSFlavorProps = {
type OSProps = {
iUserTemplate: any;
commonTemplates: any;
flavorField: any;
flavor: string;
operatinSystemField: any;
cloneBaseDiskImageField: any;
mountWindowsGuestToolsField: any;
Expand Down

0 comments on commit 33567bd

Please sign in to comment.