Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(protocol-designer,-shared-data): add liquid class scaffolding to PD #17126

Merged
merged 7 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,9 @@
"OT_PD_ENABLE_REACT_SCAN": {
"title": "Enable React Scan",
"description": "Enable React Scan support for components rendering check"
},
"OT_PD_ENABLE_LIQUID_CLASSES": {
"title": "Enable liquid classes",
"description": "Enable liquid classes support"
}
}
4 changes: 4 additions & 0 deletions protocol-designer/src/assets/localization/en/liquids.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
"display_color": "Color",
"liquid_volume": "Liquid volume by well",
"liquid": "Liquid",
"liquid_class": {
"title": "Liquid class",
"tooltip": "Applies predefined pipetting settings to transfer and mix steps using this liquid"
},
"liquids_added": "Liquids added",
"liquids": "Liquids",
"microliters": "µL",
Expand Down
2 changes: 2 additions & 0 deletions protocol-designer/src/feature-flags/reducers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ const initialFlags: Flags = {
OT_PD_ENABLE_HOT_KEYS_DISPLAY:
process.env.OT_PD_ENABLE_HOT_KEYS_DISPLAY === '1' || true,
OT_PD_ENABLE_REACT_SCAN: process.env.OT_PD_ENABLE_REACT_SCAN === '1' || false,
OT_PD_ENABLE_LIQUID_CLASSES:
process.env.OT_PD_ENABLE_REACT_SCAN === '1' || false,
}
// @ts-expect-error(sa, 2021-6-10): cannot use string literals as action type
// TODO IMMEDIATELY: refactor this to the old fashioned way if we cannot have type safety: https://github.com/redux-utilities/redux-actions/issues/282#issuecomment-595163081
Expand Down
4 changes: 4 additions & 0 deletions protocol-designer/src/feature-flags/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,7 @@ export const getEnableReactScan: Selector<boolean> = createSelector(
getFeatureFlagData,
flags => flags.OT_PD_ENABLE_REACT_SCAN ?? false
)
export const getEnableLiquidClasses: Selector<boolean> = createSelector(
getFeatureFlagData,
flags => flags.OT_PD_ENABLE_LIQUID_CLASSES ?? false
)
2 changes: 2 additions & 0 deletions protocol-designer/src/feature-flags/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export type FlagTypes =
| 'OT_PD_ENABLE_RETURN_TIP'
| 'OT_PD_ENABLE_HOT_KEYS_DISPLAY'
| 'OT_PD_ENABLE_REACT_SCAN'
| 'OT_PD_ENABLE_LIQUID_CLASSES'
// flags that are not in this list only show in prerelease mode
export const userFacingFlags: FlagTypes[] = [
'OT_PD_DISABLE_MODULE_RESTRICTIONS',
Expand All @@ -49,5 +50,6 @@ export const allFlags: FlagTypes[] = [
'OT_PD_ENABLE_COMMENT',
'OT_PD_ENABLE_RETURN_TIP',
'OT_PD_ENABLE_REACT_SCAN',
'OT_PD_ENABLE_LIQUID_CLASSES',
]
export type Flags = Partial<Record<FlagTypes, boolean | null | undefined>>
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ describe('DUPLICATE_LABWARE action', () => {
wellDetailsByLocation: null,
concentration: '50 mol/ng',
description: '',
liquidClass: null,
displayColor: '#b925ff',
serialize: false,
},
Expand All @@ -27,6 +28,7 @@ describe('DUPLICATE_LABWARE action', () => {
wellDetailsByLocation: null,
concentration: '100%',
description: '',
liquidClass: null,
displayColor: '#ffd600',
serialize: false,
},
Expand Down
1 change: 1 addition & 0 deletions protocol-designer/src/labware-ingred/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ const allIngredientNamesIds: Selector<
ingredientId: ingredId,
name: ingreds[ingredId].name,
displayColor: ingreds[ingredId].displayColor,
liquidClass: ingreds[ingredId].liquidClass,
}))
})
const getLabwareSelectionMode: Selector<RootSlice, boolean> = createSelector(
Expand Down
10 changes: 4 additions & 6 deletions protocol-designer/src/labware-ingred/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,21 @@ export interface WellContents {
selected?: boolean
maxVolume?: number
}
export type ContentsByWell = {
[wellName: string]: WellContents
} | null
export interface WellContentsByLabware {
[labwareId: string]: ContentsByWell
}
export type ContentsByWell = Record<string, WellContents> | null
export type WellContentsByLabware = Record<string, ContentsByWell>
// ==== INGREDIENTS ====
export type OrderedLiquids = Array<{
ingredientId: string
name: string | null | undefined
displayColor: string | null | undefined
liquidClass: string | null | undefined
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, the other fields can be refactored as well.

Suggested change
liquidClass: string | null | undefined
liquidClass?: string | null

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this undefined here to avoid needing to add this to a migration?

Copy link
Collaborator Author

@ncdiehl11 ncdiehl11 Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these are unnecessarily allowed to be undefined or null in some cases. The modal disallows saving without a liquid name, and displayColor is always a valid hex string. I think description can be set to null for easier use downstream as well. Thoughts on this interface for the selector allIngredientGroupFields:

export interface LiquidGroup {
  name: string
  description: string | null
  displayColor: string
  liquidClass: string | null
  serialize: boolean
}

Copy link
Collaborator Author

@ncdiehl11 ncdiehl11 Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and since we're dealing with nullish initial values in DefineLiquidsModal, we can update that interface to

interface LiquidEditFormValues {
  name: string
  displayColor: string
  description: string
  liquidClass: string
  serialize: boolean
  [key: string]: unknown
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see, that makes sense to me.

i think we would need liquidClass to be undefined in OrderedLiquids at least because its included in the JSON protocols and if it is always null or a string, we'd need to make a migration for it. i'm all for adding this to the migration but not right now since there's no way to put it behind a FF. Do you mind adding a TODO or something above the type to remind us to migrate it later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For sure! I will keep it as an optional property for now and we can remove the optionality and add a migration later

}>
// TODO: Ian 2018-10-15 audit & rename these confusing types
export interface LiquidGroup {
name: string | null | undefined
description: string | null | undefined
displayColor: string
liquidClass: string | null
ncdiehl11 marked this conversation as resolved.
Show resolved Hide resolved
serialize: boolean
}
export type IngredInputs = LiquidGroup & {
Expand Down
11 changes: 11 additions & 0 deletions protocol-designer/src/liquid-defs/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { getAllLiquidClassDefs } from '@opentrons/shared-data'

const liquidClassDefs = getAllLiquidClassDefs()
export const getLiquidClassDisplayName = (
liquidClass: string | null
): string | null => {
if (liquidClass == null) {
return null
}
return liquidClassDefs[liquidClass]?.displayName ?? null
ncdiehl11 marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
} from '@opentrons/components'

import { LINE_CLAMP_TEXT_STYLE } from '../../atoms'
import { getEnableLiquidClasses } from '../../feature-flags/selectors'
import { removeWellsContents } from '../../labware-ingred/actions'
import { selectors as labwareIngredSelectors } from '../../labware-ingred/selectors'
import { getLabwareEntities } from '../../step-forms/selectors'
Expand All @@ -34,7 +35,7 @@ interface LiquidCardProps {

export function LiquidCard(props: LiquidCardProps): JSX.Element {
const { info } = props
const { name, color, liquidIndex } = info
const { name, color, liquidClassDisplayName, liquidIndex } = info
const { t } = useTranslation('liquids')
const dispatch = useDispatch()
const [isExpanded, setIsExpanded] = useState<boolean>(false)
Expand All @@ -49,6 +50,7 @@ export function LiquidCard(props: LiquidCardProps): JSX.Element {
const allWellContentsForActiveItem = useSelector(
wellContentsSelectors.getAllWellContentsForActiveItem
)
const enableLiquidClasses = useSelector(getEnableLiquidClasses)
const wellContents =
allWellContentsForActiveItem != null && labwareId != null
? allWellContentsForActiveItem[labwareId]
Expand Down Expand Up @@ -104,13 +106,24 @@ export function LiquidCard(props: LiquidCardProps): JSX.Element {
>
<Flex alignItems={ALIGN_CENTER} gridGap={SPACING.spacing16}>
<LiquidIcon color={color} size="medium" />
<Flex flexDirection={DIRECTION_COLUMN} width="12.375rem">
<Flex
flexDirection={DIRECTION_COLUMN}
width="12.375rem"
gridGap={SPACING.spacing4}
>
<StyledText
desktopStyle="bodyDefaultSemiBold"
css={LINE_CLAMP_TEXT_STYLE(3)}
>
{name}
</StyledText>
{liquidClassDisplayName != null && enableLiquidClasses ? (
<Tag
text={liquidClassDisplayName}
type="default"
shrinkToContent
/>
) : null}
<StyledText
desktopStyle="bodyDefaultRegular"
css={LINE_CLAMP_TEXT_STYLE(3)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { selectors as labwareIngredSelectors } from '../../labware-ingred/select
import * as wellContentsSelectors from '../../top-selectors/well-contents'
import * as fieldProcessors from '../../steplist/fieldLevel/processing'
import * as labwareIngredActions from '../../labware-ingred/actions'
import { getLiquidClassDisplayName } from '../../liquid-defs/utils'
import { getSelectedWells } from '../../well-selection/selectors'
import { getLabwareNicknamesById } from '../../ui/labware/selectors'
import {
Expand All @@ -38,6 +39,7 @@ export interface LiquidInfo {
name: string
color: string
liquidIndex: string
liquidClassDisplayName: string | null
}

interface ValidFormValues {
Expand Down Expand Up @@ -214,6 +216,9 @@ export function LiquidToolbox(props: LiquidToolboxProps): JSX.Element {
liquidIndex: liquid,
name: foundLiquid?.name ?? '',
color: foundLiquid?.displayColor ?? '',
liquidClassDisplayName: getLiquidClassDisplayName(
foundLiquid?.liquidClass ?? null
),
}
})
.filter(Boolean)
Expand Down
58 changes: 55 additions & 3 deletions protocol-designer/src/organisms/DefineLiquidsModal/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,16 @@ import { yupResolver } from '@hookform/resolvers/yup'
import * as Yup from 'yup'
import { Controller, useForm } from 'react-hook-form'
import styled from 'styled-components'
import { DEFAULT_LIQUID_COLORS } from '@opentrons/shared-data'
import {
DEFAULT_LIQUID_COLORS,
getAllLiquidClassDefs,
} from '@opentrons/shared-data'
import {
BORDERS,
Btn,
COLORS,
DIRECTION_COLUMN,
DropdownMenu,
Flex,
InputField,
JUSTIFY_END,
Expand All @@ -30,6 +34,7 @@ import * as labwareIngredActions from '../../labware-ingred/actions'
import { selectors as labwareIngredSelectors } from '../../labware-ingred/selectors'
import { HandleEnter } from '../../atoms/HandleEnter'
import { LINE_CLAMP_TEXT_STYLE } from '../../atoms'
import { getEnableLiquidClasses } from '../../feature-flags/selectors'
import { swatchColors } from './swatchColors'

import type { ColorResult, RGBColor } from 'react-color'
Expand All @@ -41,6 +46,7 @@ interface LiquidEditFormValues {
name: string
displayColor: string
description?: string | null
liquidClass: string | null
serialize?: boolean
[key: string]: unknown
}
Expand All @@ -49,6 +55,7 @@ const liquidEditFormSchema: any = Yup.object().shape({
name: Yup.string().required('liquid name is required'),
displayColor: Yup.string(),
description: Yup.string(),
liquidClass: Yup.string(),
serialize: Yup.boolean(),
})

Expand Down Expand Up @@ -77,6 +84,9 @@ export function DefineLiquidsModal(
const allIngredientGroupFields = useSelector(
labwareIngredSelectors.allIngredientGroupFields
)
const enableLiquidClasses = useSelector(getEnableLiquidClasses)
const liquidClassDefs = getAllLiquidClassDefs()

const liquidGroupId = selectedLiquidGroupState.liquidGroupId
const deleteLiquidGroup = (): void => {
if (liquidGroupId != null) {
Expand Down Expand Up @@ -107,13 +117,14 @@ export function DefineLiquidsModal(
const initialValues: LiquidEditFormValues = {
name: selectedIngredFields?.name ?? '',
displayColor: selectedIngredFields?.displayColor ?? swatchColors(liquidId),
liquidClass: selectedIngredFields?.liquidClass ?? '',
description: selectedIngredFields?.description ?? '',
serialize: selectedIngredFields?.serialize ?? false,
}

const {
handleSubmit,
formState: { errors, touchedFields },
formState,
control,
watch,
setValue,
Expand All @@ -125,11 +136,14 @@ export function DefineLiquidsModal(
})
const name = watch('name')
const color = watch('displayColor')
const liquidClass = watch('liquidClass')
const { errors, touchedFields } = formState

const handleLiquidEdits = (values: LiquidEditFormValues): void => {
saveForm({
name: values.name,
displayColor: values.displayColor,
liquidClass: values.liquidClass ? values.liquidClass : null,
description: values.description ?? null,
serialize: values.serialize ?? false,
})
Expand All @@ -142,6 +156,15 @@ export function DefineLiquidsModal(
return `#${toHex(r)}${toHex(g)}${toHex(b)}${toHex(alpha)}`
}

const liquidClassOptions = [
{ name: 'Choose an option', value: '' },
...Object.entries(liquidClassDefs).map(
([liquidClassDefName, { displayName }]) => {
return { name: displayName, value: liquidClassDefName }
}
),
]

return (
<HandleEnter
onEnter={() => {
Expand Down Expand Up @@ -202,7 +225,10 @@ export function DefineLiquidsModal(
) : null}

<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing32}>
<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing8}>
<Flex
flexDirection={DIRECTION_COLUMN}
gridGap={SPACING.spacing12}
>
<Flex
flexDirection={DIRECTION_COLUMN}
color={COLORS.grey60}
Expand Down Expand Up @@ -239,6 +265,32 @@ export function DefineLiquidsModal(
</StyledText>
<DescriptionField {...register('description')} />
</Flex>
{enableLiquidClasses ? (
<Flex flexDirection={DIRECTION_COLUMN} color={COLORS.grey60}>
<Controller
control={control}
name="liquidClass"
render={({ field }) => (
<DropdownMenu
title={t('liquid_class.title')}
tooltipText={t('liquid_class.tooltip')}
dropdownType="neutral"
width="100%"
filterOptions={liquidClassOptions}
currentOption={
liquidClassOptions.find(
({ value }) => value === liquidClass
) ?? liquidClassOptions[0]
}
onClick={value => {
field.onChange(value)
setValue('liquidClass', value)
}}
/>
)}
/>
</Flex>
) : null}
<Flex
flexDirection={DIRECTION_COLUMN}
color={COLORS.grey60}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ describe('MaterialsListModal', () => {
ingredientId: mockId,
name: 'mockName',
displayColor: 'mockDisplayColor',
liquidClass: null,
},
],
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ describe('SlotOverflowMenu', () => {
displayColor: 'mockColor',
name: 'mockname',
ingredientId: '0',
liquidClass: null,
},
])
})
Expand Down
Loading
Loading