feat(protocol-designer): update error style and render logic (#16589)

This PR fixes some styling bugs for both form-level and timeline errors. Also, I update the logic of the step form 'save' button to always be enabled, and conditionally render form errors and warnings should any exist. To consolidate components that are providing essentially the same function in `ToggleExpandStepFormField` and `CheckboxExpandStepFormField`, I refactor `ToggleExpandStepFormField` to render either a toggle button or a checkbox. Lastly, I update copy for thermocycler step form toggle input field menus.
Opentrons · Oct 24, 2024 · 545d0f7 · 545d0f7
1 parent 02236b3
commit 545d0f7
Show file tree

Hide file tree

Showing 9 changed files with 93 additions and 63 deletions.
diff --git a/protocol-designer/src/assets/localization/en/form.json b/protocol-designer/src/assets/localization/en/form.json
@@ -209,20 +209,20 @@
       },
       "thermocyclerState": {
         "block": {
-          "engage": "Engage block temperature",
+          "engage": "Block temperature",
           "label": "Thermocycler block",
           "temperature": "Block temperature",
-          "toggleOff": "Deactivated",
-          "toggleOn": "Active",
+          "toggleOff": "Deactivate",
+          "toggleOn": "Activate",
           "valid_range": "Valid range between 4 and 96ºC"
         },
         "ending_hold": "Ending hold",
         "lid": {
-          "engage": "Engage lid temperature",
+          "engage": "Lid temperature",
           "label": "Lid",
           "temperature": "Lid temperature",
-          "toggleOff": "Deactivated",
-          "toggleOn": "Active",
+          "toggleOff": "Deactivate",
+          "toggleOn": "Activate",
           "valid_range": "Valid range between 37 and 110ºC"
         },
         "lidPosition": {

diff --git a/protocol-designer/src/molecules/ToggleExpandStepFormField/index.tsx b/protocol-designer/src/molecules/ToggleExpandStepFormField/index.tsx
@@ -1,6 +1,8 @@
 import {
   ALIGN_CENTER,
+  Btn,
   COLORS,
+  Check,
   DIRECTION_COLUMN,
   Flex,
   JUSTIFY_SPACE_BETWEEN,
@@ -23,6 +25,7 @@ interface ToggleExpandStepFormFieldProps extends FieldProps {
   onLabel?: string
   offLabel?: string
   caption?: string
+  toggleElement?: 'toggle' | 'checkbox'
 }
 export function ToggleExpandStepFormField(
   props: ToggleExpandStepFormFieldProps
@@ -37,6 +40,7 @@ export function ToggleExpandStepFormField(
     toggleUpdateValue,
     toggleValue,
     caption,
+    toggleElement = 'toggle',
     ...restProps
   } = props
 
@@ -58,6 +62,7 @@ export function ToggleExpandStepFormField(
     }
   }
 
+  const label = isSelected ? onLabel : offLabel ?? null
   return (
     <ListItem type="noActive">
       <Flex
@@ -68,16 +73,25 @@ export function ToggleExpandStepFormField(
         <Flex justifyContent={JUSTIFY_SPACE_BETWEEN} alignItems={ALIGN_CENTER}>
           <StyledText desktopStyle="bodyDefaultRegular">{title}</StyledText>
           <Flex alignItems={ALIGN_CENTER} gridGap={SPACING.spacing8}>
-            <StyledText desktopStyle="bodyDefaultRegular" color={COLORS.grey60}>
-              {isSelected ? onLabel : offLabel ?? null}
-            </StyledText>
-            <ToggleButton
-              onClick={() => {
-                onToggleUpdateValue()
-              }}
-              label={isSelected ? onLabel : offLabel}
-              toggledOn={isSelected}
-            />
+            {label != null ? (
+              <StyledText
+                desktopStyle="bodyDefaultRegular"
+                color={COLORS.grey60}
+              >
+                {isSelected ? onLabel : offLabel ?? null}
+              </StyledText>
+            ) : null}
+            {toggleElement === 'toggle' ? (
+              <ToggleButton
+                onClick={onToggleUpdateValue}
+                label={isSelected ? onLabel : offLabel}
+                toggledOn={isSelected}
+              />
+            ) : (
+              <Btn onClick={onToggleUpdateValue}>
+                <Check color={COLORS.blue50} isChecked={isSelected} />
+              </Btn>
+            )}
           </Flex>
         </Flex>
         <Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing10}>

diff --git a/protocol-designer/src/organisms/Alerts/FormAlerts.tsx b/protocol-designer/src/organisms/Alerts/FormAlerts.tsx
@@ -32,7 +32,7 @@ interface FormAlertsProps {
   dirtyFields?: StepFieldName[]
 }
 
-function FormAlertsComponent(props: FormAlertsProps): JSX.Element {
+function FormAlertsComponent(props: FormAlertsProps): JSX.Element | null {
   const { focusedField, dirtyFields } = props
   const { t } = useTranslation('alert')
   const dispatch = useDispatch()
@@ -95,7 +95,7 @@ function FormAlertsComponent(props: FormAlertsProps): JSX.Element {
   }
 
   const makeAlert: MakeAlert = (alertType, data, key) => (
-    <Flex padding={`${SPACING.spacing16} ${SPACING.spacing16} 0`}>
+    <Flex>
       <Banner
         type={alertType === 'error' ? 'error' : 'warning'}
         key={`${alertType}:${key}`}
@@ -104,14 +104,17 @@ function FormAlertsComponent(props: FormAlertsProps): JSX.Element {
             ? makeHandleCloseWarning(data.dismissId)
             : undefined
         }
+        width="100%"
       >
         <Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing4}>
           <StyledText desktopStyle="bodyDefaultSemiBold">
             {data.title}
           </StyledText>
-          <StyledText desktopStyle="bodyDefaultRegular">
-            {data.description}
-          </StyledText>
+          {data.description != null ? (
+            <StyledText desktopStyle="bodyDefaultRegular">
+              {data.description}
+            </StyledText>
+          ) : null}
         </Flex>
       </Banner>
     </Flex>
@@ -151,15 +154,19 @@ function FormAlertsComponent(props: FormAlertsProps): JSX.Element {
       )
     }
   }
-  return (
-    <Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing8}>
+  return [...formErrors, ...formWarnings, ...timelineWarnings].length > 0 ? (
+    <Flex
+      flexDirection={DIRECTION_COLUMN}
+      gridGap={SPACING.spacing4}
+      padding={`${SPACING.spacing16} ${SPACING.spacing16} 0`}
+    >
       {formErrors.map((error, key) => makeAlert('error', error, key))}
       {formWarnings.map((warning, key) => makeAlert('warning', warning, key))}
       {timelineWarnings.map((warning, key) =>
         makeAlert('warning', warning, key)
       )}
     </Flex>
-  )
+  ) : null
 }
 
 export const FormAlerts = memo(FormAlertsComponent)
diff --git a/protocol-designer/src/organisms/Alerts/TimelineAlerts.tsx b/protocol-designer/src/organisms/Alerts/TimelineAlerts.tsx
@@ -11,10 +11,12 @@ import {
 } from '@opentrons/components'
 import { getRobotStateTimeline } from '../../file-data/selectors'
 import { ErrorContents } from './ErrorContents'
+
+import type { StyleProps } from '@opentrons/components'
 import type { CommandCreatorError } from '@opentrons/step-generation'
 import type { MakeAlert } from './types'
 
-function TimelineAlertsComponent(): JSX.Element {
+function TimelineAlertsComponent(props: StyleProps): JSX.Element | null {
   const { t } = useTranslation('alert')
 
   const timeline = useSelector(getRobotStateTimeline)
@@ -26,6 +28,10 @@ function TimelineAlertsComponent(): JSX.Element {
     })
   )
 
+  if (timelineErrors.length === 0) {
+    return null
+  }
+
   const makeAlert: MakeAlert = (alertType, data, key) => (
     <Banner
       type={alertType === 'error' ? 'error' : 'warning'}
@@ -42,7 +48,9 @@ function TimelineAlertsComponent(): JSX.Element {
   )
 
   return (
-    <>{timelineErrors.map((error, key) => makeAlert('error', error, key))}</>
+    <Flex {...props}>
+      {timelineErrors.map((error, key) => makeAlert('error', error, key))}
+    </Flex>
   )
 }
 

diff --git a/protocol-designer/src/pages/Designer/ProtocolSteps/StepForm/StepFormToolbox.tsx b/protocol-designer/src/pages/Designer/ProtocolSteps/StepForm/StepFormToolbox.tsx
@@ -99,6 +99,10 @@ export function StepFormToolbox(props: StepFormToolboxProps): JSX.Element {
       ? 1
       : 0
   )
+  const [
+    showFormErrorsAndWarnings,
+    setShowFormErrorsAndWarnings,
+  ] = useState<boolean>(false)
   const [isRename, setIsRename] = useState<boolean>(false)
   const icon = stepIconsByType[formData.stepType]
 
@@ -126,18 +130,22 @@ export function StepFormToolbox(props: StepFormToolboxProps): JSX.Element {
   const numErrors = timeline.errors?.length ?? 0
 
   const handleSaveClick = (): void => {
-    handleSave()
-    makeSnackbar(
-      getSaveStepSnackbarText({
-        numWarnings,
-        numErrors,
-        stepTypeDisplayName: i18n.format(
-          t(`stepType.${formData.stepType}`),
-          'capitalize'
-        ),
-        t,
-      }) as string
-    )
+    if (canSave) {
+      handleSave()
+      makeSnackbar(
+        getSaveStepSnackbarText({
+          numWarnings,
+          numErrors,
+          stepTypeDisplayName: i18n.format(
+            t(`stepType.${formData.stepType}`),
+            'capitalize'
+          ),
+          t,
+        }) as string
+      )
+    } else {
+      setShowFormErrorsAndWarnings(true)
+    }
   }
 
   return (
@@ -195,9 +203,6 @@ export function StepFormToolbox(props: StepFormToolboxProps): JSX.Element {
                     }
                   : handleSaveClick
               }
-              disabled={
-                isMultiStepToolbox && toolboxStep === 0 ? false : !canSave
-              }
               width="100%"
             >
               {isMultiStepToolbox && toolboxStep === 0
@@ -215,7 +220,9 @@ export function StepFormToolbox(props: StepFormToolboxProps): JSX.Element {
           </Flex>
         }
       >
-        <FormAlerts focusedField={focusedField} dirtyFields={dirtyFields} />
+        {showFormErrorsAndWarnings ? (
+          <FormAlerts focusedField={focusedField} dirtyFields={dirtyFields} />
+        ) : null}
         <ToolsComponent
           {...{
             formData,

diff --git a/...-designer/src/pages/Designer/ProtocolSteps/StepForm/StepTools/HeaterShakerTools/index.tsx b/...-designer/src/pages/Designer/ProtocolSteps/StepForm/StepTools/HeaterShakerTools/index.tsx
@@ -120,6 +120,7 @@ export function HeaterShakerTools(props: StepFormProps): JSX.Element {
           fieldTitle={t('form:step_edit_form.field.heaterShaker.duration')}
           isSelected={formData.heaterShakerSetTimer === true}
           units={t('application:units.time')}
+          toggleElement="checkbox"
         />
       </Flex>
     </Flex>

diff --git a/...c/pages/Designer/ProtocolSteps/StepForm/StepTools/ThermocyclerTools/ThermocyclerState.tsx b/...c/pages/Designer/ProtocolSteps/StepForm/StepTools/ThermocyclerTools/ThermocyclerState.tsx
@@ -77,11 +77,9 @@ export function ThermocyclerState(props: ThermocyclerStateProps): JSX.Element {
         fieldTitle={i18n.format(t('stepType.temperature'), 'capitalize')}
         units={t('units.degrees')}
         isSelected={formData[lidFieldActive] === true}
-        onLabel={t(
-          'form:step_edit_form.field.thermocyclerState.lidPosition.toggleOn'
-        )}
+        onLabel={t('form:step_edit_form.field.thermocyclerState.lid.toggleOn')}
         offLabel={t(
-          'form:step_edit_form.field.thermocyclerState.lidPosition.toggleOff'
+          'form:step_edit_form.field.thermocyclerState.lid.toggleOff'
         )}
       />
       <ToggleStepFormField

diff --git a/protocol-designer/src/pages/Designer/ProtocolSteps/index.tsx b/protocol-designer/src/pages/Designer/ProtocolSteps/index.tsx
@@ -37,6 +37,8 @@ import { BatchEditToolbox } from './BatchEditToolbox'
 import { getDesignerTab } from '../../../file-data/selectors'
 import { TimelineAlerts } from '../../../organisms'
 
+const CONTENT_MAX_WIDTH = '46.9375rem'
+
 export function ProtocolSteps(): JSX.Element {
   const { i18n, t } = useTranslation('starting_deck_state')
   const formData = useSelector(getUnsavedForm)
@@ -79,16 +81,13 @@ export function ProtocolSteps(): JSX.Element {
         width="100%"
         justifyContent={JUSTIFY_FLEX_START}
       >
-        <Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing16}>
+        <Flex
+          flexDirection={DIRECTION_COLUMN}
+          gridGap={SPACING.spacing16}
+          maxWidth={CONTENT_MAX_WIDTH}
+        >
           {tab === 'protocolSteps' ? (
-            <Flex
-              justifyContent={JUSTIFY_CENTER}
-              width="100%"
-              paddingTop="2.34375rem"
-              paddingBottom="1.34375rem"
-            >
-              <TimelineAlerts />
-            </Flex>
+            <TimelineAlerts justifyContent={JUSTIFY_CENTER} width="100%" />
           ) : null}
           <Flex
             justifyContent={
@@ -112,11 +111,7 @@ export function ProtocolSteps(): JSX.Element {
               }}
             />
           </Flex>
-          <Flex
-            flexDirection={DIRECTION_COLUMN}
-            gridGap={SPACING.spacing16}
-            maxWidth="46.9375rem"
-          >
+          <Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing16}>
             {deckView === leftString ? (
               <DeckSetupContainer tab="protocolSteps" />
             ) : (

diff --git a/protocol-designer/src/steplist/fieldLevel/errors.ts b/protocol-designer/src/steplist/fieldLevel/errors.ts
@@ -58,20 +58,20 @@ export const minimumWellCount = (minimum: number): ErrorChecker => (
 export const minFieldValue = (minimum: number): ErrorChecker => (
   value: unknown
 ): string | null =>
-  value === null || Number(value) >= minimum
+  !value || Number(value) >= minimum
     ? null
     : `${FIELD_ERRORS.UNDER_RANGE_MINIMUM} ${minimum}`
 export const maxFieldValue = (maximum: number): ErrorChecker => (
   value: unknown
 ): string | null =>
-  value === null || Number(value) <= maximum
+  !value || Number(value) <= maximum
     ? null
     : `${FIELD_ERRORS.OVER_RANGE_MAXIMUM} ${maximum}`
 export const temperatureRangeFieldValue = (
   minimum: number,
   maximum: number
 ): ErrorChecker => (value: unknown): string | null =>
-  value === null || (Number(value) <= maximum && Number(value) >= minimum)
+  !value || (Number(value) <= maximum && Number(value) >= minimum)
     ? null
     : `${FIELD_ERRORS.OUTSIDE_OF_RANGE} ${minimum} and ${maximum} °C`
 export const realNumber: ErrorChecker = (value: unknown) =>