address comments

protocol-designer/src/components/StepEditForm/fields/TipPositionField/TipPositionAllViz.tsx

@@ -17,9 +17,7 @@ interface TipPositionAllVizProps {
   xWidthMm: number
 }
 
-export const TipPositionAllViz = (
-  props: TipPositionAllVizProps
-): JSX.Element => {
+export function TipPositionAllViz(props: TipPositionAllVizProps): JSX.Element {
   const { mmFromBottom, xPosition, wellDepthMm, xWidthMm } = props
   const fractionOfWellHeight = mmFromBottom / wellDepthMm
   const pixelsFromBottom =

protocol-designer/src/components/StepEditForm/fields/TipPositionField/TipPositionInput.module.css

@@ -70,7 +70,7 @@
   height: 1.5rem;
   width: 1.5rem;
   cursor: pointer;
-  color: #24313f;
+  color: #24313f; /* black80 */
 }
 
 .tip_position_icon:hover {

protocol-designer/src/components/StepEditForm/fields/TipPositionField/ZTipPositionModal.tsx

@@ -22,28 +22,35 @@ import type { StepFieldName } from '../../../../form-types'
 import modalStyles from '../../../modals/modal.module.css'
 import styles from './TipPositionInput.module.css'
 
-interface Props {
+interface ZTipPositionModalProps {
   closeModal: () => void
-  isIndeterminate?: boolean
   mmFromBottom: number | null
   name: StepFieldName
-  updateValue: (val: number | null | undefined) => unknown
+  updateValue: (val?: number | null) => unknown
   wellDepthMm: number
+  isIndeterminate?: boolean
 }
 
-export const ZTipPositionModal = (props: Props): JSX.Element => {
-  const { isIndeterminate, name, wellDepthMm } = props
+export function ZTipPositionModal(props: ZTipPositionModalProps): JSX.Element {
+  const {
+    isIndeterminate,
+    name,
+    wellDepthMm,
+    mmFromBottom,
+    closeModal,
+    updateValue,
+  } = props
   const { t } = useTranslation(['modal', 'button'])
   const defaultMmFromBottom = utils.getDefaultMmFromBottom({
     name,
     wellDepthMm,
   })
 
   const [value, setValue] = React.useState<string | null>(
-    props.mmFromBottom === null ? null : String(props.mmFromBottom)
+    mmFromBottom === null ? null : String(mmFromBottom)
   )
   const [isDefault, setIsDefault] = React.useState<boolean>(
-    !isIndeterminate && props.mmFromBottom === null
+    !isIndeterminate && mmFromBottom === null
   )
   // in this modal, pristinity hides the OUT_OF_BOUNDS error only.
   const [isPristine, setPristine] = React.useState<boolean>(true)
@@ -87,16 +94,16 @@ export const ZTipPositionModal = (props: Props): JSX.Element => {
 
     if (!hasErrors) {
       if (isDefault) {
-        props.updateValue(null)
+        updateValue(null)
       } else {
-        props.updateValue(value === null ? null : Number(value))
+        updateValue(value === null ? null : Number(value))
       }
-      props.closeModal()
+      closeModal()
     }
   }
 
   const handleCancel = (): void => {
-    props.closeModal()
+    closeModal()
   }
 
   const handleChange = (newValueRaw: string | number): void => {
@@ -219,7 +226,7 @@ export const ZTipPositionModal = (props: Props): JSX.Element => {
             </div>
 
             <div className={styles.viz_group}>
-              {!isDefault && (
+              {!isDefault ? (
                 <div className={styles.adjustment_buttons}>
                   <OutlineButton
                     id="Increment_tipPosition"
@@ -236,7 +243,7 @@ export const ZTipPositionModal = (props: Props): JSX.Element => {
                     <Icon name="minus" />
                   </OutlineButton>
                 </div>
-              )}
+              ) : null}
               <TipPositionZAxisViz
                 mmFromBottom={
                   value !== null ? Number(value) : defaultMmFromBottom

protocol-designer/src/components/StepEditForm/fields/TipPositionField/__tests__/TipPositionField.test.tsx

@@ -1,13 +1,14 @@
 import * as React from 'react'
 import { fireEvent, screen } from '@testing-library/react'
 import { describe, it, expect, vi, beforeEach } from 'vitest'
+import { fixture96Plate } from '@opentrons/shared-data'
 import { renderWithProviders } from '../../../../../__testing-utils__'
 import { i18n } from '../../../../../localization'
-import { TipPositionModal } from '../TipPositionModal'
-import { TipPositionField } from '../index'
 import { getLabwareEntities } from '../../../../../step-forms/selectors'
-import { LabwareDefinition2, fixture96Plate } from '@opentrons/shared-data'
 import { ZTipPositionModal } from '../ZTipPositionModal'
+import { TipPositionModal } from '../TipPositionModal'
+import { TipPositionField } from '../index'
+import type { LabwareDefinition2 } from '@opentrons/shared-data'
 
 vi.mock('../../../../../step-forms/selectors')
 vi.mock('../ZTipPositionModal')

protocol-designer/src/components/StepEditForm/fields/TipPositionField/index.tsx

@@ -11,17 +11,13 @@ import {
   useHoverTooltip,
   UseHoverTooltipTargetProps,
 } from '@opentrons/components'
-import {
-  getWellsDepth,
-  getWellXDimension,
-  getWellYDimension,
-} from '@opentrons/shared-data'
+import { getWellsDepth, getWellDimension } from '@opentrons/shared-data'
 import {
   getIsTouchTipField,
   getIsDelayPositionField,
 } from '../../../../form-types'
 import { selectors as stepFormSelectors } from '../../../../step-forms'
-import { PositionSpecs, TipPositionModal } from './TipPositionModal'
+import { TipPositionModal } from './TipPositionModal'
 import { getDefaultMmFromBottom } from './utils'
 import { ZTipPositionModal } from './ZTipPositionModal'
 import type {
@@ -30,6 +26,7 @@ import type {
   TipZOffsetFields,
 } from '../../../../form-types'
 import type { FieldPropsByName } from '../../types'
+import type { PositionSpecs } from './TipPositionModal'
 
 import stepFormStyles from '../../StepEditForm.module.css'
 import styles from './TipPositionInput.module.css'
@@ -71,8 +68,8 @@ export function TipPositionField(props: TipPositionFieldProps): JSX.Element {
     const firstWell = labwareDef.wells.A1
     if (firstWell) {
       wellDepthMm = getWellsDepth(labwareDef, ['A1'])
-      wellXWidthMm = getWellXDimension(labwareDef, ['A1'])
-      wellYWidthMm = getWellYDimension(labwareDef, ['A1'])
+      wellXWidthMm = getWellDimension(labwareDef, ['A1'], 'x')
+      wellYWidthMm = getWellDimension(labwareDef, ['A1'], 'y')
     }
   }
 
@@ -104,9 +101,7 @@ export function TipPositionField(props: TipPositionFieldProps): JSX.Element {
   if (wellDepthMm !== null) {
     // show default value for field in parens if no mmFromBottom value is selected
     zValue =
-      mmFromBottom !== null
-        ? mmFromBottom
-        : getDefaultMmFromBottom({ name: zName, wellDepthMm })
+      mmFromBottom ?? getDefaultMmFromBottom({ name: zName, wellDepthMm })
   }
 
   let modal = (
@@ -173,7 +168,7 @@ export function TipPositionField(props: TipPositionFieldProps): JSX.Element {
       >
         {yField != null && xField != null ? (
           <Flex
-            onClick={disabled != null ? () => handleOpen(true) : undefined}
+            onClick={disabled != null ? () => handleOpen(true) : () => {}}
             id={`TipPositionIcon_${zName}`}
             data-testid={`TipPositionIcon_${zName}`}
             width="5rem"

protocol-designer/src/components/StepEditForm/fields/TipPositionField/utils.ts

@@ -47,7 +47,7 @@ export function getDefaultMmFromBottom(args: {
 }
 
 export const roundValue = (value: number | string | null): number => {
-  return round(Number(value), DECIMALS_ALLOWED)
+  return value === null ? 0 : round(Number(value), DECIMALS_ALLOWED)
 }
 
 const OUT_OF_BOUNDS: 'OUT_OF_BOUNDS' = 'OUT_OF_BOUNDS'
@@ -80,19 +80,19 @@ export const getErrors = (args: {
   maxMm: number
   minMm: number
 }): Error[] => {
-  const { isDefault, value, maxMm, minMm } = args
+  const { isDefault, value: rawValue, maxMm, minMm } = args
   const errors: Error[] = []
   if (isDefault) return errors
 
-  const v = Number(value)
-  if (value === null || Number.isNaN(v)) {
+  const value = Number(rawValue)
+  if (rawValue === null || Number.isNaN(value)) {
     // blank or otherwise invalid should show this error as a fallback
     return [OUT_OF_BOUNDS]
   }
-  const correctDecimals = round(v, DECIMALS_ALLOWED) === v
-  const outOfBounds = v > maxMm || v < minMm
+  const incorrectDecimals = round(value, DECIMALS_ALLOWED) !== value
+  const outOfBounds = value > maxMm || value < minMm
 
-  if (!correctDecimals) {
+  if (incorrectDecimals) {
     errors.push(TOO_MANY_DECIMALS)
   }
   if (outOfBounds) {
@@ -108,7 +108,7 @@ interface MinMaxValues {
 
 export const getMinMaxWidth = (width: number): MinMaxValues => {
   return {
-    minValue: -width / 2,
-    maxValue: width / 2,
+    minValue: -width * 0.5,
+    maxValue: width * 0.5,
   }
 }

protocol-designer/src/localization/en/tooltip.json

@@ -26,7 +26,7 @@
       "aspirate_delay_mmFromBottom": "Distance from the bottom of the well",
       "aspirate_flowRate": "The speed at which the pipette aspirates",
       "aspirate_mix_checkbox": "Pipette up and down before aspirating",
-      "aspirate_mmFromBottom": "Adjust tip position",
+      "aspirate_mmFromBottom": "Adjust tip position for aspirate",
       "aspirate_touchTip_checkbox": "Touch tip to each side of well after aspirating",
       "aspirate_touchTip_mmFromBottom": "Distance from the bottom of the well",
       "blowout_checkbox": "Where to dispose of remaining volume in tip",
@@ -37,7 +37,7 @@
       "dispense_delay_mmFromBottom": "Distance from the bottom of the well",
       "dispense_flowRate": "The speed at which the pipette dispenses",
       "dispense_mix_checkbox": "Pipette up and down after dispensing",
-      "dispense_mmFromBottom": "Adjust tip position",
+      "dispense_mmFromBottom": "Adjust tip position for dispense",
       "dispense_touchTip_checkbox": "Touch tip to each side of well after dispensing",
       "dispense_touchTip_mmFromBottom": "Distance from the bottom of the well",
       "disposalVolume_checkbox": "Aspirate extra volume that is disposed of after a multi-dispense is complete. We recommend a disposal volume of at least the pipette's minimum.",

shared-data/js/helpers/index.ts

@@ -201,33 +201,18 @@ export const getWellsDepth = (
   return offsets[0]
 }
 
-export const getWellXDimension = (
+export const getWellDimension = (
   labwareDef: LabwareDefinition2,
-  wells: string[]
-): number => {
-  const offsets = wells.map(well => {
-    const labwareWell = labwareDef.wells[well]
-    const shape = labwareWell.shape
-    if (shape === 'circular') {
-      return labwareWell.diameter
-    } else {
-      return labwareWell.xDimension
-    }
-  })
-  return offsets[0]
-}
-
-export const getWellYDimension = (
-  labwareDef: LabwareDefinition2,
-  wells: string[]
+  wells: string[],
+  position: 'x' | 'y'
 ): number => {
   const offsets = wells.map(well => {
     const labwareWell = labwareDef.wells[well]
     const shape = labwareWell.shape
     if (shape === 'circular') {
       return labwareWell.diameter
     } else {
-      return labwareWell.yDimension
+      return position === 'x' ? labwareWell.xDimension : labwareWell.yDimension
     }
   })
   return offsets[0]