feat(protocol designer): remove mix tip position fields (#6590)

shlokamin · web-flow · commit eb058a283eed · 2020-09-22T15:35:52.000-04:00
* feat(protocol designer): remove mix tip position fields remove mix tip position fields from UI, mixFormToArgs, types, and relevant utils #6589
diff --git a/protocol-designer/benchmarks/timelineGeneration.js b/protocol-designer/benchmarks/timelineGeneration.js
@@ -39,8 +39,8 @@ bench(`commandCreatorsTimeline: mix ${times} times`, b => {
       dispenseOffsetFromBottomMm: 2,
       aspirateFlowRateUlSec: 5,
       dispenseFlowRateUlSec: 6,
-      aspirateDelay: null,
-      dispenseDelay: null,
+      aspirateDelaySeconds: null,
+      dispenseDelaySeconds: null,
     }),
   ]
 
diff --git a/protocol-designer/src/components/StepEditForm/fields/DelayFields.js b/protocol-designer/src/components/StepEditForm/fields/DelayFields.js
@@ -16,7 +16,7 @@ import type {
 type DelayFieldProps = {
   checkboxFieldName: DelayCheckboxFields,
   secondsFieldName: DelaySecondFields,
-  tipPositionFieldName: TipOffsetFields,
+  tipPositionFieldName?: TipOffsetFields,
   focusHandlers: FocusHandlers,
 }
 
@@ -42,7 +42,9 @@ export const DelayFields = (props: DelayFieldProps): React.Node => {
         className={styles.small_field}
         {...focusHandlers}
       />
-      <TipPositionField fieldName={tipPositionFieldName} />
+      {tipPositionFieldName && (
+        <TipPositionField fieldName={tipPositionFieldName} />
+      )}
     </CheckboxRowField>
   )
 }
diff --git a/protocol-designer/src/components/StepEditForm/fields/TipPositionField/index.js b/protocol-designer/src/components/StepEditForm/fields/TipPositionField/index.js
@@ -36,8 +36,6 @@ function getLabwareFieldForPositioningField(
     dispense_delay_mmFromBottom: 'dispense_labware',
     mix_mmFromBottom: 'labware',
     mix_touchTip_mmFromBottom: 'labware',
-    mix_aspirate_delay_mmFromBottom: 'labware',
-    mix_dispense_delay_mmFromBottom: 'labware',
   }
   return fieldMap[fieldName]
 }
diff --git a/protocol-designer/src/components/StepEditForm/fields/__tests__/DelayFields.test.js b/protocol-designer/src/components/StepEditForm/fields/__tests__/DelayFields.test.js
@@ -16,7 +16,6 @@ describe('DelayFields', () => {
       props = {
         checkboxFieldName: 'aspirate_delay_checkbox',
         secondsFieldName: 'aspirate_delay_seconds',
-        tipPositionFieldName: 'aspirate_mmFromBottom',
         focusHandlers: {
           focusedField: '',
           dirtyFields: [],
@@ -26,9 +25,10 @@ describe('DelayFields', () => {
       }
     })
 
-    it('should render an aspirate delay field', () => {
-      const wrapper = render(props)
+    it('should render an aspirate delay field with a tip position field', () => {
+      props = { ...props, tipPositionFieldName: 'aspirate_mmFromBottom' }
 
+      const wrapper = render(props)
       const checkboxField = wrapper.find(CheckboxRowField)
       expect(checkboxField.prop('name')).toBe(props.checkboxFieldName)
       expect(checkboxField.prop('label')).toBe('delay')
@@ -44,14 +44,29 @@ describe('DelayFields', () => {
       expect(tipPosField.is(TipPositionField)).toBe(true)
       expect(tipPosField.prop('fieldName')).toBe(props.tipPositionFieldName)
     })
+    it('should render an aspirate delay field WITHOUT a tip position field', () => {
+      const wrapper = render(props)
+
+      const checkboxField = wrapper.find(CheckboxRowField)
+      expect(checkboxField.prop('name')).toBe(props.checkboxFieldName)
+      expect(checkboxField.prop('label')).toBe('delay')
+      expect(checkboxField.prop('tooltipComponent')).toBe(
+        FormTooltipText.step_fields.defaults.aspirate_delay_checkbox
+      )
+      const secondsField = checkboxField.childAt(0)
+      expect(secondsField.is(TextField)).toBe(true)
+      expect(secondsField.prop('name')).toBe(props.secondsFieldName)
+      expect(secondsField.prop('units')).toBe(ApplicationText.units.seconds)
+
+      expect(wrapper.find(TipPositionField).length).toBe(0)
+    })
   })
   describe('Dispense Delay', () => {
     let props
     beforeEach(() => {
       props = {
         checkboxFieldName: 'dispense_delay_checkbox',
         secondsFieldName: 'dispense_delay_seconds',
-        tipPositionFieldName: 'dispense_delay_mmFromBottom',
         focusHandlers: {
           focusedField: '',
           dirtyFields: [],
@@ -61,7 +76,9 @@ describe('DelayFields', () => {
       }
     })
 
-    it('should render an dispense delay field', () => {
+    it('should render an dispense delay field with a tip position field', () => {
+      props = { ...props, tipPositionFieldName: 'dispense_delay_mmFromBottom' }
+
       const wrapper = render(props)
 
       const checkboxField = wrapper.find(CheckboxRowField)
@@ -79,5 +96,22 @@ describe('DelayFields', () => {
       expect(tipPosField.is(TipPositionField)).toBe(true)
       expect(tipPosField.prop('fieldName')).toBe(props.tipPositionFieldName)
     })
+
+    it('should render an dispense delay field WITHOUT a tip position field', () => {
+      const wrapper = render(props)
+
+      const checkboxField = wrapper.find(CheckboxRowField)
+      expect(checkboxField.prop('name')).toBe(props.checkboxFieldName)
+      expect(checkboxField.prop('label')).toBe('delay')
+      expect(checkboxField.prop('tooltipComponent')).toBe(
+        FormTooltipText.step_fields.defaults.dispense_delay_checkbox
+      )
+      const secondsField = checkboxField.childAt(0)
+      expect(secondsField.is(TextField)).toBe(true)
+      expect(secondsField.prop('name')).toBe(props.secondsFieldName)
+      expect(secondsField.prop('units')).toBe(ApplicationText.units.seconds)
+
+      expect(wrapper.find(TipPositionField).length).toBe(0)
+    })
   })
 })
diff --git a/protocol-designer/src/components/StepEditForm/forms/MixForm.js b/protocol-designer/src/components/StepEditForm/forms/MixForm.js
@@ -117,7 +117,6 @@ export class MixForm extends React.Component<Props, State> {
               <DelayFields
                 checkboxFieldName={'aspirate_delay_checkbox'}
                 secondsFieldName={'aspirate_delay_seconds'}
-                tipPositionFieldName={'mix_aspirate_delay_mmFromBottom'}
                 focusHandlers={focusHandlers}
               />
             </div>
@@ -134,7 +133,6 @@ export class MixForm extends React.Component<Props, State> {
                 <DelayFields
                   checkboxFieldName={'dispense_delay_checkbox'}
                   secondsFieldName={'dispense_delay_seconds'}
-                  tipPositionFieldName={'mix_dispense_delay_mmFromBottom'}
                   focusHandlers={focusHandlers}
                 />
                 <CheckboxRowField
diff --git a/protocol-designer/src/components/StepEditForm/forms/__tests__/MixForm.test.js b/protocol-designer/src/components/StepEditForm/forms/__tests__/MixForm.test.js
@@ -44,9 +44,8 @@ describe('MixForm', () => {
       expect(aspirateDelayFields.prop('secondsFieldName')).toBe(
         'aspirate_delay_seconds'
       )
-      expect(aspirateDelayFields.prop('tipPositionFieldName')).toBe(
-        'mix_aspirate_delay_mmFromBottom'
-      )
+      // no tip position field
+      expect(aspirateDelayFields.prop('tipPositionFieldName')).toBe(undefined)
     })
     it('should render the dispense delay fields', () => {
       const wrapper = render(props)
@@ -59,9 +58,8 @@ describe('MixForm', () => {
       expect(aspirateDelayFields.prop('secondsFieldName')).toBe(
         'dispense_delay_seconds'
       )
-      expect(aspirateDelayFields.prop('tipPositionFieldName')).toBe(
-        'mix_dispense_delay_mmFromBottom'
-      )
+      // no tip position field
+      expect(aspirateDelayFields.prop('tipPositionFieldName')).toBe(undefined)
     })
   })
 })
diff --git a/protocol-designer/src/form-types.js b/protocol-designer/src/form-types.js
@@ -46,8 +46,6 @@ export type StepFieldName = any
 // | 'labwareLocationUpdate'
 // | 'mix_mmFromBottom'
 // | 'mix_touchTip_mmFromBottom'
-// | 'mix_aspirate_delay_mmFromBottom
-// | 'mix_dispense_delay_mmFromBottom
 // | 'path'
 // | 'pauseAction'
 // | 'pauseHour'
@@ -298,8 +296,6 @@ export type TipOffsetFields =
   | 'aspirate_delay_mmFromBottom'
   | 'dispense_delay_mmFromBottom'
   | 'mix_touchTip_mmFromBottom'
-  | 'mix_aspirate_delay_mmFromBottom'
-  | 'mix_dispense_delay_mmFromBottom'
 
 export type DelayCheckboxFields =
   | 'aspirate_delay_checkbox'
@@ -314,8 +310,6 @@ export function getIsTouchTipField(fieldName: string): boolean {
     'aspirate_touchTip_mmFromBottom',
     'dispense_touchTip_mmFromBottom',
     'mix_touchTip_mmFromBottom',
-    'mix_aspirate_delay_mmFromBottom',
-    'mix_dispense_delay_mmFromBottom',
   ]
   return touchTipFields.includes(fieldName)
 }
diff --git a/protocol-designer/src/localization/en/modal.json b/protocol-designer/src/localization/en/modal.json
@@ -26,9 +26,7 @@
       "dispense_touchTip_mmFromBottom": "Change from where in the well the robot performs touch tip",
       "mix_touchTip_mmFromBottom": "Change from where in the well the robot performs touch tip",
       "aspirate_delay_mmFromBottom": "Change from where in the well the robot delays after aspirating",
-      "dispense_delay_mmFromBottom": "Change from where in the well the robot delays after dispensing",
-      "mix_aspirate_delay_mmFromBottom": "Change from where in the well the robot delays after aspirating",
-      "mix_dispense_delay_mmFromBottom": "Change from where in the well the robot delays after dispensing"
+      "dispense_delay_mmFromBottom": "Change from where in the well the robot delays after dispensing"
     },
     "field_label": "Distance from bottom of well"
   },
diff --git a/protocol-designer/src/step-generation/__tests__/mix.test.js b/protocol-designer/src/step-generation/__tests__/mix.test.js
@@ -41,8 +41,8 @@ beforeEach(() => {
     blowoutLocation: null,
     touchTip: false,
 
-    aspirateDelay: null,
-    dispenseDelay: null,
+    aspirateDelaySeconds: null,
+    dispenseDelaySeconds: null,
   }
 
   invariantContext = makeContext()
diff --git a/protocol-designer/src/step-generation/types.js b/protocol-designer/src/step-generation/types.js
@@ -167,8 +167,8 @@ export type MixArgs = {|
   aspirateFlowRateUlSec: number,
   dispenseFlowRateUlSec: number,
   /** delays */
-  aspirateDelay: ?InnerDelayArgs,
-  dispenseDelay: ?InnerDelayArgs,
+  aspirateDelaySeconds: ?number,
+  dispenseDelaySeconds: ?number,
 |}
 
 export type PauseArgs = {|
diff --git a/protocol-designer/src/steplist/formLevel/stepFormToArgs/getDelayData.js b/protocol-designer/src/steplist/formLevel/stepFormToArgs/getDelayData.js
@@ -5,15 +5,13 @@ import type {
   DelaySecondFields,
 } from '../../../form-types'
 
-export function getDelayData<F: any>(
+export function getMoveLiquidDelayData<F: any>(
   hydratedFormData: F,
   checkboxField: DelayCheckboxFields,
   secondsField: DelaySecondFields,
   mmFromBottomField:
     | 'aspirate_delay_mmFromBottom'
     | 'dispense_delay_mmFromBottom'
-    | 'mix_aspirate_delay_mmFromBottom'
-    | 'mix_dispense_delay_mmFromBottom'
 ): InnerDelayArgs | null {
   const checkbox = hydratedFormData[checkboxField]
   const seconds = hydratedFormData[secondsField]
@@ -28,3 +26,17 @@ export function getDelayData<F: any>(
   }
   return null
 }
+
+export function getMixDelayData<F: any>(
+  hydratedFormData: F,
+  checkboxField: DelayCheckboxFields,
+  secondsField: DelaySecondFields
+): number | null {
+  const checkbox = hydratedFormData[checkboxField]
+  const seconds = hydratedFormData[secondsField]
+
+  if (checkbox && (typeof seconds === 'number' && seconds > 0)) {
+    return seconds
+  }
+  return null
+}
diff --git a/protocol-designer/src/steplist/formLevel/stepFormToArgs/mixFormToArgs.js b/protocol-designer/src/steplist/formLevel/stepFormToArgs/mixFormToArgs.js
@@ -9,7 +9,7 @@ import {
   DEFAULT_MM_TOUCH_TIP_OFFSET_FROM_TOP,
 } from '../../../constants'
 import { getOrderedWells } from '../../utils'
-import { getDelayData } from './getDelayData'
+import { getMixDelayData } from './getDelayData'
 import type { HydratedMixFormDataLegacy } from '../../../form-types'
 import type { MixArgs } from '../../../step-generation'
 
@@ -73,18 +73,16 @@ export const mixFormToArgs = (
     : 0
 
   // Delay settings
-  const aspirateDelay = getDelayData<HydratedMixFormDataLegacy>(
+  const aspirateDelaySeconds = getMixDelayData<HydratedMixFormDataLegacy>(
     hydratedFormData,
     'aspirate_delay_checkbox',
-    'aspirate_delay_seconds',
-    'mix_aspirate_delay_mmFromBottom'
+    'aspirate_delay_seconds'
   )
 
-  const dispenseDelay = getDelayData<HydratedMixFormDataLegacy>(
+  const dispenseDelaySeconds = getMixDelayData<HydratedMixFormDataLegacy>(
     hydratedFormData,
     'dispense_delay_checkbox',
-    'dispense_delay_seconds',
-    'mix_dispense_delay_mmFromBottom'
+    'dispense_delay_seconds'
   )
 
   return {
@@ -106,7 +104,7 @@ export const mixFormToArgs = (
     aspirateOffsetFromBottomMm,
     dispenseOffsetFromBottomMm,
     blowoutOffsetFromTopMm,
-    aspirateDelay,
-    dispenseDelay,
+    aspirateDelaySeconds,
+    dispenseDelaySeconds,
   }
 }
diff --git a/protocol-designer/src/steplist/formLevel/stepFormToArgs/moveLiquidFormToArgs.js b/protocol-designer/src/steplist/formLevel/stepFormToArgs/moveLiquidFormToArgs.js
@@ -12,7 +12,7 @@ import {
   DEFAULT_MM_TOUCH_TIP_OFFSET_FROM_TOP,
 } from '../../../constants'
 import { getOrderedWells } from '../../utils'
-import { getDelayData } from './getDelayData'
+import { getMoveLiquidDelayData } from './getDelayData'
 
 import type { HydratedMoveLiquidFormData } from '../../../form-types'
 import type {
@@ -157,14 +157,14 @@ export const moveLiquidFormToArgs = (
     'dispense_mix_volume',
     'dispense_mix_times'
   )
-  const aspirateDelay = getDelayData<MoveLiquidFields>(
+  const aspirateDelay = getMoveLiquidDelayData<MoveLiquidFields>(
     fields,
     'aspirate_delay_checkbox',
     'aspirate_delay_seconds',
     'aspirate_delay_mmFromBottom'
   )
 
-  const dispenseDelay = getDelayData<MoveLiquidFields>(
+  const dispenseDelay = getMoveLiquidDelayData<MoveLiquidFields>(
     fields,
     'dispense_delay_checkbox',
     'dispense_delay_seconds',
diff --git a/protocol-designer/src/steplist/formLevel/stepFormToArgs/test/getDelayData.test.js b/protocol-designer/src/steplist/formLevel/stepFormToArgs/test/getDelayData.test.js
diff --git a/protocol-designer/src/steplist/formLevel/stepFormToArgs/test/mixFormToArgs.test.js b/protocol-designer/src/steplist/formLevel/stepFormToArgs/test/mixFormToArgs.test.js
diff --git a/protocol-designer/src/timelineMiddleware/__tests__/generateRobotStateTimeline.test.js b/protocol-designer/src/timelineMiddleware/__tests__/generateRobotStateTimeline.test.js

Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@ import type {`
`16`	`16`	`type DelayFieldProps = {`
`17`	`17`	`checkboxFieldName: DelayCheckboxFields,`
`18`	`18`	`secondsFieldName: DelaySecondFields,`
`19`		`- tipPositionFieldName: TipOffsetFields,`
	`19`	`+ tipPositionFieldName?: TipOffsetFields,`
`20`	`20`	`focusHandlers: FocusHandlers,`
`21`	`21`	`}`
`22`	`22`
`@@ -42,7 +42,9 @@ export const DelayFields = (props: DelayFieldProps): React.Node => {`
`42`	`42`	`className={styles.small_field}`
`43`	`43`	`{...focusHandlers}`
`44`	`44`	`/>`
`45`		`- <TipPositionField fieldName={tipPositionFieldName} />`
	`45`	`+ {tipPositionFieldName && (`
	`46`	`+ <TipPositionField fieldName={tipPositionFieldName} />`
	`47`	`+ )}`
`46`	`48`	`</CheckboxRowField>`
`47`	`49`	`)`
`48`	`50`	`}`
Original file line number	Diff line number	Diff line change
`@@ -36,8 +36,6 @@ function getLabwareFieldForPositioningField(`
`36`	`36`	`dispense_delay_mmFromBottom: 'dispense_labware',`
`37`	`37`	`mix_mmFromBottom: 'labware',`
`38`	`38`	`mix_touchTip_mmFromBottom: 'labware',`
`39`		`- mix_aspirate_delay_mmFromBottom: 'labware',`
`40`		`- mix_dispense_delay_mmFromBottom: 'labware',`
`41`	`39`	`}`
`42`	`40`	`return fieldMap[fieldName]`
`43`	`41`	`}`
Original file line number	Diff line number	Diff line change
`@@ -41,8 +41,8 @@ beforeEach(() => {`
`41`	`41`	`blowoutLocation: null,`
`42`	`42`	`touchTip: false,`
`43`	`43`
`44`		`- aspirateDelay: null,`
`45`		`- dispenseDelay: null,`
	`44`	`+ aspirateDelaySeconds: null,`
	`45`	`+ dispenseDelaySeconds: null,`
`46`	`46`	`}`
`47`	`47`
`48`	`48`	`invariantContext = makeContext()`