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
Changes from 6 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
@@ -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
@@ -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",
2 changes: 2 additions & 0 deletions protocol-designer/src/feature-flags/reducers.ts
Original file line number Diff line number Diff line change
@@ -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
4 changes: 4 additions & 0 deletions protocol-designer/src/feature-flags/selectors.ts
Original file line number Diff line number Diff line change
@@ -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
@@ -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',
@@ -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
@@ -19,6 +19,7 @@ describe('DUPLICATE_LABWARE action', () => {
wellDetailsByLocation: null,
concentration: '50 mol/ng',
description: '',
liquidClass: null,
displayColor: '#b925ff',
serialize: false,
},
@@ -27,6 +28,7 @@ describe('DUPLICATE_LABWARE action', () => {
wellDetailsByLocation: null,
concentration: '100%',
description: '',
liquidClass: null,
displayColor: '#ffd600',
serialize: false,
},
1 change: 1 addition & 0 deletions protocol-designer/src/labware-ingred/selectors.ts
Original file line number Diff line number Diff line change
@@ -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(
14 changes: 6 additions & 8 deletions protocol-designer/src/labware-ingred/types.ts
Original file line number Diff line number Diff line change
@@ -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
name: string | null
description: string | null
displayColor: string
liquidClass: string | null
ncdiehl11 marked this conversation as resolved.
Show resolved Hide resolved
serialize: boolean
}
export type IngredInputs = LiquidGroup & {
15 changes: 15 additions & 0 deletions protocol-designer/src/liquid-defs/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { getAllLiquidClassDefs } from '@opentrons/shared-data'

const liquidClassDefs = getAllLiquidClassDefs()
export const getLiquidClassDisplayName = (
liquidClass: string | null
): string | null => {
if (liquidClass == null) {
return null
}
if (!(liquidClass in liquidClassDefs)) {
console.warn(`Liquid class ${liquidClass} not found`)
return null
}
return liquidClassDefs[liquidClass].displayName
}
Original file line number Diff line number Diff line change
@@ -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'
@@ -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)
@@ -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]
@@ -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)}
Original file line number Diff line number Diff line change
@@ -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 {
@@ -38,6 +39,7 @@ export interface LiquidInfo {
name: string
color: string
liquidIndex: string
liquidClassDisplayName: string | null
}

interface ValidFormValues {
@@ -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)
65 changes: 59 additions & 6 deletions protocol-designer/src/organisms/DefineLiquidsModal/index.tsx
Original file line number Diff line number Diff line change
@@ -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,
@@ -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'
@@ -40,15 +45,17 @@ import type { LiquidGroup } from '../../labware-ingred/types'
interface LiquidEditFormValues {
name: string
displayColor: string
description?: string | null
serialize?: boolean
description: string
liquidClass: string
serialize: boolean
[key: string]: unknown
}

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(),
})

@@ -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) {
@@ -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,
@@ -125,12 +136,15 @@ 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,
description: values.description ?? null,
liquidClass: values.liquidClass ? values.liquidClass : null,
description: values.description ? values.description : null,
serialize: values.serialize ?? false,
})
}
@@ -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={() => {
@@ -182,6 +205,7 @@ export function DefineLiquidsModal(
left="4.375rem"
top="4.6875rem"
ref={chooseColorWrapperRef}
zIndex={2}
>
<Controller
name="displayColor"
@@ -202,7 +226,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}
@@ -239,6 +266,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}
Original file line number Diff line number Diff line change
@@ -162,6 +162,7 @@ describe('MaterialsListModal', () => {
ingredientId: mockId,
name: 'mockName',
displayColor: 'mockDisplayColor',
liquidClass: null,
},
],
}
Original file line number Diff line number Diff line change
@@ -42,6 +42,7 @@ describe('SlotOverflowMenu', () => {
displayColor: 'mockColor',
name: 'mockname',
ingredientId: '0',
liquidClass: null,
},
])
})
Loading

Unchanged files with check annotations Beta

units?: string
type?: string
}
interface PipetteQuirksField {

Check warning on line 62 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature

Check warning on line 62 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature
[quirkId: string]: boolean
}
interface QuirksField {
quirks?: PipetteQuirksField
}
export type PipetteSettingsFieldsMap = QuirksField & {

Check warning on line 69 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature

Check warning on line 69 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature
[fieldId: string]: PipetteSettingsField
}
export interface IndividualPipetteSettings {
fields: PipetteSettingsFieldsMap
}
type PipetteSettingsById = Partial<{ [id: string]: IndividualPipetteSettings }>

Check warning on line 77 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature

Check warning on line 77 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature
export type PipetteSettings = PipetteSettingsById
export interface PipetteSettingsUpdateFieldsMap {

Check warning on line 81 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature

Check warning on line 81 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature
[fieldId: string]: PipetteSettingsUpdateField
}
} | null
export interface UpdatePipetteSettingsData {
fields: { [fieldId: string]: PipetteSettingsUpdateField }

Check warning on line 90 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature

Check warning on line 90 in api-client/src/pipettes/types.ts

GitHub Actions / js checks

A record is preferred over an index signature
}
export interface ResourceLink {
href: string
meta?: Partial<{ [key: string]: string | null | undefined }>

Check warning on line 14 in api-client/src/types.ts

GitHub Actions / js checks

A record is preferred over an index signature

Check warning on line 14 in api-client/src/types.ts

GitHub Actions / js checks

A record is preferred over an index signature
}
export type ResourceLinks = Record<
export const appRestart = (message: string): AppRestartAction => ({
type: APP_RESTART,
payload: {
message: message,

Check warning on line 360 in app-shell-odd/src/actions.ts

GitHub Actions / js checks

Expected property shorthand

Check warning on line 360 in app-shell-odd/src/actions.ts

GitHub Actions / js checks

Expected property shorthand
},
meta: { shell: true },
})
export const reloadUi = (message: string): ReloadUiAction => ({
type: RELOAD_UI,
payload: {
message: message,

Check warning on line 368 in app-shell-odd/src/actions.ts

GitHub Actions / js checks

Expected property shorthand

Check warning on line 368 in app-shell-odd/src/actions.ts

GitHub Actions / js checks

Expected property shorthand
},
meta: { shell: true },
})
export const sendLog = (message: string): SendLogAction => ({
type: SEND_LOG,
payload: {
message: message,

Check warning on line 376 in app-shell-odd/src/actions.ts

GitHub Actions / js checks

Expected property shorthand

Check warning on line 376 in app-shell-odd/src/actions.ts

GitHub Actions / js checks

Expected property shorthand
},
meta: { shell: true },
})
export const updateBrightness = (message: string): UpdateBrightnessAction => ({
type: UPDATE_BRIGHTNESS,
payload: {
message: message,

Check warning on line 384 in app-shell-odd/src/actions.ts

GitHub Actions / js checks

Expected property shorthand

Check warning on line 384 in app-shell-odd/src/actions.ts

GitHub Actions / js checks

Expected property shorthand
},
meta: { shell: true },
})