Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
jerader committed Feb 27, 2024
1 parent 77e422e commit 562003c
Show file tree
Hide file tree
Showing 10 changed files with 76 additions and 72 deletions.
4 changes: 2 additions & 2 deletions app/src/organisms/AppSettings/ManualIpHostnameForm.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react'
import { useDispatch } from 'react-redux'
import { useTranslation } from 'react-i18next'
import { FieldError, Resolver, useForm } from 'react-hook-form'
import { useForm } from 'react-hook-form'
import styled from 'styled-components'

import {
Expand All @@ -19,6 +19,7 @@ import { StyledText } from '../../atoms/text'
import { addManualIp } from '../../redux/config'
import { startDiscovery } from '../../redux/discovery'

import type { FieldError, Resolver } from 'react-hook-form'
import type { Dispatch } from '../../redux/types'

const FlexForm = styled.form`
Expand Down Expand Up @@ -90,7 +91,6 @@ export function ManualIpHostnameForm({
): Record<string, FieldError> => {
const ip = data.ip.trim()
let message: string | undefined
// ToDo: kj 12/19/2022 for this, the best way is to use the regex because invisible unicode characters
if (!ip) {
message = t('add_ip_error')
}
Expand Down
81 changes: 41 additions & 40 deletions app/src/organisms/ConfigurePipette/ConfigForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import forOwn from 'lodash/forOwn'
import keys from 'lodash/keys'
import omit from 'lodash/omit'
import set from 'lodash/set'
import { useForm } from 'react-hook-form'
import { Box, OVERFLOW_AUTO } from '@opentrons/components'
import { ConfigFormResetButton } from './ConfigFormResetButton'
import {
Expand All @@ -14,13 +15,13 @@ import {
ConfigQuirkGroup,
} from './ConfigFormGroup'

import type { FieldError, Resolver } from 'react-hook-form'
import type { FormValues } from './ConfigFormGroup'
import type {
PipetteSettingsField,
PipetteSettingsFieldsMap,
UpdatePipetteSettingsData,
} from '@opentrons/api-client'
import { FieldError, Resolver, useForm } from 'react-hook-form'

export interface DisplayFieldProps extends PipetteSettingsField {
name: string
Expand All @@ -46,7 +47,7 @@ const POWER_KEYS = ['plungerCurrent', 'pickUpCurrent', 'dropTipCurrent']
const TIP_KEYS = ['dropTipSpeed', 'pickUpDistance']
const QUIRK_KEY = 'quirks'

export const ConfigForm = (props: ConfigFormProps): JSX.Element => {
export function ConfigForm(props: ConfigFormProps): JSX.Element {
const {
updateInProgress,
formId,
Expand All @@ -55,6 +56,44 @@ export const ConfigForm = (props: ConfigFormProps): JSX.Element => {
groupLabels,
} = props

const getInitialValues: () => FormValues = () => {
const fields = getVisibleFields()
const initialFieldValues = mapValues<
PipetteSettingsFieldsMap,
string | boolean
>(fields, f => {
if (f.value === true || f.value === false) return f.value
// @ts-expect-error(sa, 2021-05-27): avoiding src code change, use optional chain to access f.value
return f.value !== f.default ? f.value.toString() : ''
})
const initialQuirkValues = settings[QUIRK_KEY]
const initialValues = Object.assign(
{},
initialFieldValues,
initialQuirkValues
)

return initialValues
}
const initialValues = getInitialValues()

const resolver: Resolver<FormValues> = values => {
let errors = {}
errors = validate(values, errors)
return { values, errors }
}

const {
handleSubmit,
reset,
getValues,
control,
formState: { errors },
} = useForm<FormValues>({
defaultValues: initialValues,
resolver: resolver,
})

const getFieldsByKey = (
keys: string[],
fields: PipetteSettingsFieldsMap
Expand Down Expand Up @@ -166,32 +205,6 @@ export const ConfigForm = (props: ConfigFormProps): JSX.Element => {
return errors
}

const resolver: Resolver<FormValues> = values => {
let errors = {}
errors = validate(values, errors)
return { values, errors }
}

const getInitialValues: () => FormValues = () => {
const fields = getVisibleFields()
const initialFieldValues = mapValues<
PipetteSettingsFieldsMap,
string | boolean
>(fields, f => {
if (f.value === true || f.value === false) return f.value
// @ts-expect-error(sa, 2021-05-27): avoiding src code change, use optional chain to access f.value
return f.value !== f.default ? f.value.toString() : ''
})
const initialQuirkValues = settings[QUIRK_KEY]
const initialValues = Object.assign(
{},
initialFieldValues,
initialQuirkValues
)

return initialValues
}

const fields = getVisibleFields()
const UNKNOWN_KEYS = getUnknownKeys()
const plungerFields = getFieldsByKey(PLUNGER_KEYS, fields)
Expand All @@ -200,18 +213,6 @@ export const ConfigForm = (props: ConfigFormProps): JSX.Element => {
const quirkFields = getKnownQuirks()
const quirksPresent = quirkFields.length > 0
const unknownFields = getFieldsByKey(UNKNOWN_KEYS, fields)
const initialValues = getInitialValues()

const {
handleSubmit,
reset,
getValues,
control,
formState: { errors },
} = useForm<FormValues>({
defaultValues: initialValues,
resolver: resolver,
})

const handleReset = (): void => {
const newValues = mapValues(getValues(), v => {
Expand Down
3 changes: 2 additions & 1 deletion app/src/organisms/ConfigurePipette/ConfigFormGroup.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react'
import { Control, Controller } from 'react-hook-form'
import { Controller } from 'react-hook-form'
import {
FormGroup,
Flex,
Expand All @@ -12,6 +12,7 @@ import { InputField } from '../../atoms/InputField'
import { StyledText } from '../../atoms/text'
import styles from './styles.css'

import type { Control } from 'react-hook-form'
import type { DisplayFieldProps, DisplayQuirkFieldProps } from './ConfigForm'

export interface FormColumnProps {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react'
import { useSelector, useDispatch } from 'react-redux'
import { useHistory } from 'react-router-dom'
import { useForm, Resolver, Controller, FieldError } from 'react-hook-form'
import { useForm, Controller } from 'react-hook-form'

Check failure on line 4 in app/src/organisms/Devices/RobotSettings/AdvancedTab/AdvancedTabSlideouts/RenameRobotSlideout.tsx

View workflow job for this annotation

GitHub Actions / js checks

'/home/runner/work/opentrons/opentrons/app/node_modules/react-hook-form/dist/index.esm.mjs' imported multiple times

Check failure on line 4 in app/src/organisms/Devices/RobotSettings/AdvancedTab/AdvancedTabSlideouts/RenameRobotSlideout.tsx

View workflow job for this annotation

GitHub Actions / js checks

'/home/runner/work/opentrons/opentrons/app/node_modules/react-hook-form/dist/index.esm.mjs' imported multiple times
import { useTranslation } from 'react-i18next'
import {
Flex,
Expand All @@ -28,6 +28,7 @@ import { InputField } from '../../../../../atoms/InputField'
import { Banner } from '../../../../../atoms/Banner'
import { useIsFlex } from '../../../hooks'

import { Resolver, FieldError } from 'react-hook-form'

Check failure on line 31 in app/src/organisms/Devices/RobotSettings/AdvancedTab/AdvancedTabSlideouts/RenameRobotSlideout.tsx

View workflow job for this annotation

GitHub Actions / js checks

'/home/runner/work/opentrons/opentrons/app/node_modules/react-hook-form/dist/index.esm.mjs' imported multiple times

Check failure on line 31 in app/src/organisms/Devices/RobotSettings/AdvancedTab/AdvancedTabSlideouts/RenameRobotSlideout.tsx

View workflow job for this annotation

GitHub Actions / js checks

'/home/runner/work/opentrons/opentrons/app/node_modules/react-hook-form/dist/index.esm.mjs' imported multiple times
import type { UpdatedRobotName } from '@opentrons/api-client'
import type { State, Dispatch } from '../../../../../redux/types'
interface RenameRobotSlideoutProps {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react'
import { Control, Controller } from 'react-hook-form'
import { Controller } from 'react-hook-form'
import styled, { css } from 'styled-components'

import {
Expand All @@ -14,6 +14,7 @@ import { SecurityField } from './SecurityField'
import { FIELD_TYPE_KEY_FILE, FIELD_TYPE_SECURITY } from '../constants'
import * as Copy from '../i18n'

import type { Control } from 'react-hook-form'
import type { ConnectFormField, ConnectFormValues, WifiNetwork } from '../types'

const fieldStyle = css`
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import get from 'lodash/get'
import { FieldError } from 'react-hook-form'

import * as Constants from '../constants'
import * as Copy from '../i18n'

import type { FieldError } from 'react-hook-form'
import type {
WifiNetwork,
WifiKey,
Expand Down Expand Up @@ -149,16 +149,16 @@ export function validateConnectFormFields(
securityType: formSecurityType,
psk: formPsk,
} = values
let message: string | undefined
let errorMessage: string | undefined

if (network === null && !formSsid) {
message = Copy.FIELD_IS_REQUIRED(Copy.LABEL_SSID)
return message != null
if (network === null && (formSsid == null || formSsid.length === 0)) {
errorMessage = Copy.FIELD_IS_REQUIRED(Copy.LABEL_SSID)
return errorMessage != null
? {
...errors,
ssid: {
type: 'ssidError',
message: message,
message: errorMessage,
},
}
: errors
Expand All @@ -168,13 +168,13 @@ export function validateConnectFormFields(
(network === null || network.securityType === Constants.SECURITY_WPA_EAP) &&
!formSecurityType
) {
message = Copy.FIELD_IS_REQUIRED(Copy.LABEL_SECURITY)
return message != null
errorMessage = Copy.FIELD_IS_REQUIRED(Copy.LABEL_SECURITY)
return errorMessage != null
? {
...errors,
securityType: {
type: 'securityTypeError',
message: message,
message: errorMessage,
},
}
: errors
Expand All @@ -185,16 +185,16 @@ export function validateConnectFormFields(
formSecurityType === Constants.SECURITY_WPA_PSK) &&
(!formPsk || formPsk.length < Constants.CONFIGURE_PSK_MIN_LENGTH)
) {
message = Copy.FIELD_NOT_LONG_ENOUGH(
errorMessage = Copy.FIELD_NOT_LONG_ENOUGH(
Copy.LABEL_PSK,
Constants.CONFIGURE_PSK_MIN_LENGTH
)
return message != null
return errorMessage != null
? {
...errors,
psk: {
type: 'pskError',
message: message,
message: errorMessage,
},
}
: errors
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import * as React from 'react'
import {
useForm,
import { useEffect } from 'react'
import { useForm } from 'react-hook-form'
import { usePrevious } from '@opentrons/components'
import type {
ControllerFieldState,
ControllerRenderProps,
FieldValues,
} from 'react-hook-form'
import { usePrevious } from '@opentrons/components'

import type { ConnectFormValues, ConnectFormFieldProps } from '../types'

export const useResetFormOnSecurityChange = (): void => {
Expand All @@ -25,7 +24,7 @@ export const useResetFormOnSecurityChange = (): void => {
const securityType = getValues('securityType')
const prevSecurityType = usePrevious(securityType)

React.useEffect(() => {
useEffect(() => {
if (prevSecurityType && securityType !== prevSecurityType) {
clearErrors('ssid')
clearErrors('securityType')
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react'
import { Control, useForm, Resolver } from 'react-hook-form'
import { useForm } from 'react-hook-form'

import { useResetFormOnSecurityChange } from './form-state'
import {
Expand All @@ -10,6 +10,7 @@ import {

import { FormModal } from './FormModal'

import type { Control, Resolver } from 'react-hook-form'
import type {
ConnectFormValues,
WifiConfigureRequest,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { FieldError } from 'react-hook-form'
import type { FieldError } from 'react-hook-form'
import type {
WifiNetwork,
EapOption,
Expand Down
14 changes: 7 additions & 7 deletions app/src/pages/NameRobot/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react'
import { Controller, FieldError, Resolver, useForm } from 'react-hook-form'
import { Controller, useForm } from 'react-hook-form'
import { useTranslation } from 'react-i18next'
import { useSelector, useDispatch } from 'react-redux'
import { css } from 'styled-components'
Expand Down Expand Up @@ -37,7 +37,7 @@ import { SmallButton } from '../../atoms/buttons'
import { StepMeter } from '../../atoms/StepMeter'
import { useIsUnboxingFlowOngoing } from '../../organisms/RobotSettingsDashboard/NetworkSettings/hooks'
import { ConfirmRobotName } from '../../organisms/OnDeviceDisplay/NameRobot/ConfirmRobotName'

import type { FieldError, Resolver } from 'react-hook-form'
import type { UpdatedRobotName } from '@opentrons/api-client'
import type { State, Dispatch } from '../../redux/types'

Expand Down Expand Up @@ -84,28 +84,28 @@ export function NameRobot(): JSX.Element {
errors: Record<string, FieldError>
): Record<string, FieldError> => {
const newName = data.newRobotName
let message: string | undefined
let errorMessage: string | undefined
// In ODD users cannot input letters and numbers from software keyboard
// so the app only checks the length of input string
if (newName.length < 1) {
message = t('name_rule_error_name_length')
errorMessage = t('name_rule_error_name_length')
}

if (
[...connectableRobots, ...reachableRobots].some(
robot => newName === robot.name && robot.ip !== ipAddress
)
) {
message = t('name_rule_error_exist')
errorMessage = t('name_rule_error_exist')
}

const updatedErrors =
message != null
errorMessage != null
? {
...errors,
newRobotName: {
type: 'error',
message: message,
message: errorMessage,
},
}
: errors
Expand Down

0 comments on commit 562003c

Please sign in to comment.