Просмотр исходного кода

Improve UX for wizard options required fields

The following changes are now applied:
- The Next button is no longer disabled in wizard source and destination
options when required field values are missing.
- When clicking the Next button, the required fields with missing values
are *highlighted* and the wizard doesn't go to the next page.
- As part of *highlighting* a field, a small label is displayed besides
that field saying that the field is required. This label is also shown
everywhere there are required fields: new endpoint, new user etc.
Sergiu Miclea 4 лет назад
Родитель
Сommit
dd0d4beafb

+ 3 - 4
src/components/modules/TransferModule/TransferItemModal/TransferItemModal.tsx

@@ -26,9 +26,8 @@ import Button from '@src/components/ui/Button'
 import StatusImage from '@src/components/ui/StatusComponents/StatusImage'
 import Modal from '@src/components/ui/Modal'
 import Panel from '@src/components/ui/Panel'
-import { isOptionsPageValid } from '@src/components/modules/WizardModule/WizardPageContent'
 import WizardNetworks, { WizardNetworksChangeObject } from '@src/components/modules/WizardModule/WizardNetworks'
-import WizardOptions, { INSTANCE_OSMORPHING_MINION_POOL_MAPPINGS } from '@src/components/modules/WizardModule/WizardOptions'
+import WizardOptions, { findInvalidFields, INSTANCE_OSMORPHING_MINION_POOL_MAPPINGS } from '@src/components/modules/WizardModule/WizardOptions'
 import WizardStorage from '@src/components/modules/WizardModule/WizardStorage'
 
 import type {
@@ -440,12 +439,12 @@ class TransferItemModal extends React.Component<Props, State> {
     const env = type === 'source' ? this.props.replica.source_environment : this.props.replica.destination_environment
     const data = type === 'source' ? this.state.sourceData : this.state.destinationData
     const schema = type === 'source' ? providerStore.sourceSchema : providerStore.destinationSchema
-    const isValid = isOptionsPageValid({
+    const invalidFields = findInvalidFields({
       ...env,
       ...data,
     }, schema)
 
-    this.setState({ updateDisabled: !isValid })
+    this.setState({ updateDisabled: invalidFields.length > 0 })
   }
 
   handlePanelChange(panel: string) {

+ 64 - 1
src/components/modules/WizardModule/WizardOptions/WizardOptions.tsx

@@ -123,7 +123,52 @@ const NoSourceFieldsSubMessage = styled.div<any>`
 `
 
 export const shouldRenderField = (field: Field) => (field.type !== 'array' || (field.enum && field.enum.length && field.enum.length > 0))
-    && (field.type !== 'object' || field.properties)
+  && (field.type !== 'object' || field.properties)
+
+export const findInvalidFields = (data: any, schema: Field[]): Field[] => {
+  const isInvalid = (field: Field): boolean => {
+    if (data) {
+      const fieldValue = field.groupName ? data[field.groupName]?.[field.name] : data[field.name]
+      if (fieldValue === null) {
+        return true
+      }
+      if (fieldValue === undefined) {
+        return field.default == null
+      }
+      return !fieldValue
+    }
+    return field.default == null
+  }
+
+  if (!schema || schema.length === 0) {
+    return []
+  }
+
+  let required = schema.filter(f => f.required && f.type !== 'object')
+  schema.forEach(f => {
+    if (f.type === 'object' && f.properties) {
+      required = required.concat(f.properties?.filter(p => p.required).map(p => ({ ...p, groupName: f.name })))
+    }
+
+    if (f.subFields) {
+      if (f.enum) {
+        const value = data && data[f.name]
+        const subField = f.subFields.find(sf => sf.name === `${String(value)}_options`)
+        if (subField?.properties) {
+          required = required.concat(subField.properties.filter(p => p.required))
+        }
+      } else if (f.type === 'boolean') {
+        const subField = data?.[f.name] ? f.subFields[1] : f.subFields[0]
+        if (subField.properties) {
+          required = required.concat(subField.properties.filter(p => p.required))
+        }
+      }
+    }
+  })
+
+  return required.filter(isInvalid)
+}
+
 type FieldRender = {
   field: Field,
   component: React.ReactNode,
@@ -157,8 +202,15 @@ type Props = {
   optionsLoadingSkipFields?: string[],
   dictionaryKey: string,
 }
+type State = {
+  highlightedFields: Field[],
+}
 @observer
 class WizardOptions extends React.Component<Props> {
+  state: State = {
+    highlightedFields: [],
+  }
+
   componentDidMount() {
     window.addEventListener('resize', this.handleResize)
   }
@@ -289,6 +341,16 @@ class WizardOptions extends React.Component<Props> {
     this.setState({})
   }
 
+  // Called only by parent components
+  // eslint-disable-next-line
+  highlightFields(): boolean {
+    const highlightedFields: Field[] = findInvalidFields(this.props.data, this.props.fields)
+
+    this.setState({ highlightedFields })
+
+    return highlightedFields.length > 0
+  }
+
   generateGroups(fields: FieldRender[]) {
     let groups: Array<{ fields: FieldRender[], name?: string }> = [{ fields }]
 
@@ -362,6 +424,7 @@ class WizardOptions extends React.Component<Props> {
         width={this.props.fieldWidth || ThemeProps.inputSizes.wizard.width}
         nullableBoolean={field.nullableBoolean}
         disabled={field.disabled}
+        highlight={Boolean(this.state.highlightedFields.find(f => f.name === field.name || f.groupName === field.name))}
         disabledLoading={this.props.optionsLoading
           && !optionsLoadingReqFields.find(fn => fn === field.name)}
         // eslint-disable-next-line react/jsx-props-no-spreading

+ 27 - 74
src/components/modules/WizardModule/WizardPageContent/WizardPageContent.tsx

@@ -49,6 +49,7 @@ import networkStore from '@src/stores/NetworkStore'
 import { ProviderTypes } from '@src/@types/Providers'
 import minionPoolStore from '@src/stores/MinionPoolStore'
 import LoadingButton from '@src/components/ui/LoadingButton'
+import notificationStore from '@src/stores/NotificationStore'
 import transferItemIcon from './images/transferItemIcon'
 
 const Wrapper = styled.div<any>`
@@ -117,60 +118,6 @@ const WizardTypeIcon = styled.div<any>`
   align-items: center;
   margin: 0 32px;
 `
-export const isOptionsPageValid = (data: any, schema: Field[]) => {
-  const isValid = (field: Field): boolean => {
-    if (data) {
-      const fieldValue = field.groupName ? data[field.groupName]?.[field.name] : data[field.name]
-      if (fieldValue === null) {
-        return false
-      }
-      if (fieldValue === undefined) {
-        return field.default != null
-      }
-      return Boolean(fieldValue)
-    }
-    return field.default != null
-  }
-
-  if (!schema || schema.length === 0) {
-    return true
-  }
-
-  let required = schema.filter(f => f.required && f.type !== 'object')
-  schema.forEach(f => {
-    if (f.type === 'object' && f.properties) {
-      required = required.concat(f.properties?.filter(p => p.required).map(p => ({ ...p, groupName: f.name })))
-    }
-
-    if (f.subFields) {
-      if (f.enum) {
-        const value = data && data[f.name]
-        const subField = f.subFields.find(sf => sf.name === `${String(value)}_options`)
-        if (subField?.properties) {
-          required = required.concat(subField.properties.filter(p => p.required))
-        }
-      } else if (f.type === 'boolean') {
-        const subField = data?.[f.name] ? f.subFields[1] : f.subFields[0]
-        if (subField.properties) {
-          required = required.concat(subField.properties.filter(p => p.required))
-        }
-      }
-    }
-  })
-
-  let validFieldsCount = 0
-  required.forEach(f => {
-    if (isValid(f)) {
-      validFieldsCount += 1
-    }
-  })
-
-  if (validFieldsCount === required.length) {
-    return true
-  }
-
-  return false
-}
 type Props = {
   page: { id: string, title: string },
   type: 'replica' | 'migration',
@@ -226,6 +173,8 @@ class WizardPageContent extends React.Component<Props, State> {
     timezone: 'local',
   }
 
+  optionsRef?: WizardOptions | null
+
   componentDidMount() {
     this.props.onContentRef(this)
   }
@@ -290,7 +239,15 @@ class WizardPageContent extends React.Component<Props, State> {
     return false
   }
 
-  isNextButtonDisabled() {
+  handleAdvancedOptionsToggle(useAdvancedOptions: boolean) {
+    this.setState({ useAdvancedOptions })
+  }
+
+  handleTimezoneChange(timezone: TimezoneValue) {
+    this.setState({ timezone })
+  }
+
+  get nextButtonDisabled() {
     if (this.props.nextButtonDisabled) {
       return true
     }
@@ -302,17 +259,7 @@ class WizardPageContent extends React.Component<Props, State> {
         return !this.props.wizardData.target
       case 'vms':
         return !this.props.wizardData.selectedInstances
-          || !this.props.wizardData.selectedInstances.length
-      case 'source-options':
-        return !isOptionsPageValid(
-          this.props.wizardData.sourceOptions,
-          this.props.providerStore.sourceSchema,
-        )
-      case 'dest-options':
-        return !isOptionsPageValid(
-          this.props.wizardData.destOptions,
-          this.props.providerStore.destinationSchema,
-        )
+        || !this.props.wizardData.selectedInstances.length
       case 'networks':
         return !this.isNetworksPageValid()
       default:
@@ -320,12 +267,16 @@ class WizardPageContent extends React.Component<Props, State> {
     }
   }
 
-  handleAdvancedOptionsToggle(useAdvancedOptions: boolean) {
-    this.setState({ useAdvancedOptions })
-  }
-
-  handleTimezoneChange(timezone: TimezoneValue) {
-    this.setState({ timezone })
+  handleNextClick() {
+    let goNext = true
+    if (this.optionsRef?.highlightFields()) {
+      goNext = false
+    }
+    if (goNext) {
+      this.props.onNextClick()
+    } else {
+      notificationStore.alert('Please fill the required fields', 'error')
+    }
   }
 
   renderHeader() {
@@ -463,6 +414,7 @@ class WizardPageContent extends React.Component<Props, State> {
       case 'source-options':
         body = (
           <WizardOptions
+            ref={ref => { this.optionsRef = ref }}
             loading={this.areOptionsLoading('source')}
             minionPools={this.props.minionPoolStore.minionPools
               .filter(m => m.platform === 'source' && m.endpoint_id === this.props.wizardData.source?.id)}
@@ -483,6 +435,7 @@ class WizardPageContent extends React.Component<Props, State> {
       case 'dest-options':
         body = (
           <WizardOptions
+            ref={ref => { this.optionsRef = ref }}
             loading={this.areOptionsLoading('destination')}
             minionPools={this.props.minionPoolStore.minionPools
               .filter(m => m.platform === 'destination' && m.endpoint_id === this.props.wizardData.target?.id)}
@@ -608,8 +561,8 @@ class WizardPageContent extends React.Component<Props, State> {
           <LoadingButton>Loading ...</LoadingButton>
         ) : (
           <Button
-            onClick={this.props.onNextClick}
-            disabled={this.isNextButtonDisabled()}
+            onClick={() => { this.handleNextClick() }}
+            disabled={this.nextButtonDisabled}
           >{isLastPage ? 'Finish' : 'Next'}
           </Button>
         )}

+ 2 - 2
src/components/smart/WizardPage/WizardPage.tsx

@@ -154,8 +154,8 @@ class WizardPage extends React.Component<Props, State> {
   }
 
   handleEnterKey() {
-    if (this.contentRef && !this.contentRef.isNextButtonDisabled()) {
-      this.handleNextClick()
+    if (this.contentRef && !this.contentRef.nextButtonDisabled) {
+      this.contentRef.handleNextClick()
     }
   }
 

+ 10 - 4
src/components/ui/FieldInput/FieldInput.tsx

@@ -37,6 +37,7 @@ import FileInput from '@src/components/ui/FileInput'
 import asteriskImage from './images/asterisk.svg'
 
 const Wrapper = styled.div<any>`
+  position: relative;
   ${props => (props.layout === 'page' ? css`
     display: flex;
     flex-direction: ${props.inline ? 'row' : 'column'};
@@ -78,6 +79,13 @@ const Asterisk = styled.div<any>`
   margin-bottom: -3px;
   margin-left: ${props => props.marginLeft || '0px'};
 `
+const HighlightLabel = styled.div<{ rightMargin: number }>`
+  position: absolute;
+  bottom: -13px;
+  font-size: 10px;
+  right: ${props => `${props.rightMargin}px`};
+  color: ${ThemePalette.alert};
+`
 
 type Props = {
   name: string,
@@ -194,6 +202,7 @@ class FieldInput extends React.Component<Props> {
 
     return (
       <PropertiesTable
+        highlight={this.props.highlight}
         width={this.props.width}
         properties={this.props.properties}
         valueCallback={field => this.props.valueCallback && this.props.valueCallback(field)}
@@ -414,10 +423,6 @@ class FieldInput extends React.Component<Props> {
           {this.props.label}
         </LabelText>
         {description ? <InfoIcon text={description} marginLeft={-20} marginBottom={this.props.layout === 'page' ? null : 0} /> : null}
-        {/*
-        {warning ? <InfoIcon warning filled text={warning}
-         marginLeft={3} marginBottom={this.props.layout === 'page' ? null : 0} style={{ transform: 'scale(0.8)' }} /> : null}
-        */}
         {this.props.layout === 'page' && Boolean(this.props.required) ? <Asterisk marginLeft={description ? '4px' : '-16px'} /> : null}
         {warning ? <WarningLabel fontSize={this.props.layout === 'page' ? 10 : 6}>{warning}</WarningLabel> : null}
       </Label>
@@ -434,6 +439,7 @@ class FieldInput extends React.Component<Props> {
       >
         {this.renderLabel()}
         {this.renderInput()}
+        {this.props.highlight ? <HighlightLabel rightMargin={this.props.type === 'object' ? 21 : 0}>Required field</HighlightLabel> : null}
       </Wrapper>
     )
   }

+ 9 - 2
src/components/ui/PropertiesTable/PropertiesTable.tsx

@@ -25,11 +25,16 @@ import Dropdown from '@src/components/ui/Dropdowns/Dropdown'
 import AutocompleteDropdown from '@src/components/ui/Dropdowns/AutocompleteDropdown'
 import { Field, EnumItem, isEnumSeparator } from '@src/@types/Field'
 
-const Wrapper = styled.div<any>`
+const Wrapper = styled.div<{
+  width?: number,
+  highlight?: boolean,
+  disabled?: boolean,
+  disabledLoading?: boolean
+}>`
   display: flex;
   ${props => (props.width ? `width: ${props.width - 2}px;` : '')}
   flex-direction: column;
-  border: 1px solid ${ThemePalette.grayscale[2]};
+  border: 1px solid ${props => (props.highlight ? ThemePalette.alert : ThemePalette.grayscale[2])};
   border-radius: ${ThemeProps.borderRadius};
   ${props => (props.disabled ? css`
     opacity: 0.5;
@@ -70,6 +75,7 @@ const Row = styled.div<any>`
 `
 type Props = {
   properties: Field[],
+  highlight?: boolean,
   onChange: (property: Field, value: any) => void,
   valueCallback: (property: Field) => any,
   hideRequiredSymbol?: boolean,
@@ -200,6 +206,7 @@ class PropertiesTable extends React.Component<Props> {
         disabled={this.props.disabled}
         disabledLoading={this.props.disabledLoading}
         width={width}
+        highlight={this.props.highlight}
       >
         {this.props.properties.map(prop => (
           <Row key={prop.name}>