Skip to content

Commit

Permalink
fix(protocol-designer): fix well selection and uncessary mixes detail… (
Browse files Browse the repository at this point in the history
#16850)

…s with partial-tip 96 channel

fix RQA-3564, RQA-3619, RQA-3620

<!--
Thanks for taking the time to open a Pull Request (PR)! Please make sure
you've read the "Opening Pull Requests" section of our Contributing
Guide:


https://github.com/Opentrons/opentrons/blob/edge/CONTRIBUTING.md#opening-pull-requests

GitHub provides robust markdown to format your PR. Links, diagrams,
pictures, and videos along with text formatting make it possible to
create a rich and informative PR. For more information on GitHub
markdown, see:


https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax

To ensure your code is reviewed quickly and thoroughly, please fill out
the sections below to the best of your ability!
-->

# Overview

<!--
Describe your PR at a high level. State acceptance criteria and how this
PR fits into other work. Link issues, PRs, and other relevant resources.
-->

In this PR, I’ve fixed the unnecessary mixes in the transfer details
when choosing nozzles as a column, cleared out the selected wells when
switching nozzles, and ensured the liquid is correctly shown as mixed
when aspirating and dispensing into the same well of the same labware.

## Test Plan and Hands on Testing

<!--
Describe your testing of the PR. Emphasize testing not reflected in the
code. Attach protocols, logs, screenshots and any other assets that
support your testing.
-->

## Changelog

<!--
List changes introduced by this PR considering future developers and the
end user. Give careful thought and clear documentation to breaking
changes.
-->

- Used `useEffect()` to reset `primaryWellCount` and `selectedWells`
when `nozzleType` changes
- Added optional `isSameLabware` prop to check whether its source and
dest labware are the same
- Added a filter to exclude substep details with undefined source wells
(unselected wells when using column nozzles)

## Review requests

<!--
- What do you need from reviewers to feel confident this PR is ready to
merge?
- Ask questions.
-->

## Risk assessment

<!--
- Indicate the level of attention this PR needs.
- Provide context to guide reviewers.
- Discuss trade-offs, coupling, and side effects.
- Look for the possibility, even if you think it's small, that your
change may affect some other part of the system.
- For instance, changing return tip behavior may also change the
behavior of labware calibration.
- How do your unit tests and on hands on testing mitigate this PR's
risks and the risk of future regressions?
- Especially in high risk PRs, explain how you know your testing is
enough.
-->

---------

Co-authored-by: shiyaochen <[email protected]>
Co-authored-by: shiyaochen <[email protected]>
  • Loading branch information
3 people authored Nov 19, 2024
1 parent 44a80fb commit 1c4385c
Show file tree
Hide file tree
Showing 7 changed files with 144 additions and 63 deletions.
25 changes: 16 additions & 9 deletions protocol-designer/src/organisms/SelectWellsModal/index.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as React from 'react'
import { useEffect, useState } from 'react'
import { useSelector } from 'react-redux'
import { useTranslation } from 'react-i18next'
import omit from 'lodash/omit'
Expand Down Expand Up @@ -65,15 +65,22 @@ export const SelectWellsModal = (
(labwareId != null ? labwareEntities[labwareId]?.def : null) ?? null
const pipette = pipetteId != null ? pipetteEntities[pipetteId] : null

const initialSelectedPrimaryWells = Array.isArray(wellFieldData)
? arrayToWellGroup(wellFieldData as string[])
: {}
const [selectedPrimaryWells, setSelectedPrimaryWells] = useState<WellGroup>(
Array.isArray(wellFieldData)
? arrayToWellGroup(wellFieldData as string[])
: {}
)
useEffect(() => {
if (Array.isArray(wellFieldData)) {
setSelectedPrimaryWells(
wellFieldData.length === 0
? {}
: arrayToWellGroup(wellFieldData as string[])
)
}
}, [wellFieldData])

const [
selectedPrimaryWells,
setSelectedPrimaryWells,
] = React.useState<WellGroup>(initialSelectedPrimaryWells)
const [highlightedWells, setHighlightedWells] = React.useState<WellGroup>({})
const [highlightedWells, setHighlightedWells] = useState<WellGroup>({})

if (!isOpen) return null

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { useEffect, useRef, useState } from 'react'
import { createPortal } from 'react-dom'
import { useDispatch, useSelector } from 'react-redux'
import { useTranslation } from 'react-i18next'
Expand Down Expand Up @@ -58,12 +59,28 @@ export const WellSelectionField = (
const stepId = useSelector(getSelectedStepId)
const pipetteEntities = useSelector(stepFormSelectors.getPipetteEntities)
const wellSelectionLabwareKey = useSelector(getWellSelectionLabwareKey)
const primaryWellCount =

const calculateWellCount =
Array.isArray(selectedWells) && selectedWells.length > 0
? selectedWells.length.toString()
: null

const [primaryWellCount, setPrimaryWellCount] = useState(calculateWellCount)
const pipette = pipetteId != null ? pipetteEntities[pipetteId] : null
const nozzleType = getNozzleType(pipette, nozzles)
const previousNozzleType = useRef(nozzleType)

useEffect(() => {
if (previousNozzleType.current !== nozzleType) {
setPrimaryWellCount(null)
updateValue([])
previousNozzleType.current = nozzleType
}
}, [nozzleType, updateValue])

useEffect(() => {
setPrimaryWellCount(calculateWellCount)
}, [selectedWells])

const getModalKey = (): string => {
return `${String(stepId)}${name}${pipetteId || 'noPipette'}${
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ interface MultichannelSubstepProps {
substepIndex: number
selectSubstep: (substepIdentifier: SubstepIdentifier) => void
highlighted?: boolean
isSameLabware?: boolean
}

export function MultichannelSubstep(
Expand All @@ -40,6 +41,7 @@ export function MultichannelSubstep(
substepIndex,
ingredNames,
trashName,
isSameLabware,
} = props
const { t } = useTranslation('application')
const [collapsed, setCollapsed] = useState<Boolean>(true)
Expand Down Expand Up @@ -110,6 +112,7 @@ export function MultichannelSubstep(
dest={row.dest}
stepId={stepId}
substepIndex={substepIndex}
isSameLabware={isSameLabware}
/>
)
})}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,33 @@ export function PipettingSubsteps(props: PipettingSubstepsProps): JSX.Element {
? additionalEquipment[destLocationId]?.name
: null

const isSameLabware = formData.aspirate_labware === formData.dispense_labware

const renderSubsteps = substeps.multichannel
? substeps.multiRows.map((rowGroup, groupKey) => (
<MultichannelSubstep
trashName={trashName}
key={groupKey}
highlighted={
!!hoveredSubstep &&
hoveredSubstep.stepId === substeps.parentStepId &&
hoveredSubstep.substepIndex === groupKey
}
rowGroup={rowGroup}
stepId={substeps.parentStepId}
substepIndex={groupKey}
selectSubstep={selectSubstep}
ingredNames={ingredNames}
/>
))
? substeps.multiRows.map((rowGroup, groupKey) => {
const filteredRowGroup = rowGroup.filter(
item => item.source !== undefined
)
if (filteredRowGroup.length === 0) return null

return (
<MultichannelSubstep
trashName={trashName}
key={groupKey}
highlighted={
!!hoveredSubstep &&
hoveredSubstep.stepId === substeps.parentStepId &&
hoveredSubstep.substepIndex === groupKey
}
rowGroup={filteredRowGroup}
stepId={substeps.parentStepId}
substepIndex={groupKey}
selectSubstep={selectSubstep}
ingredNames={ingredNames}
isSameLabware={isSameLabware}
/>
)
})
: substeps.rows.map((row, substepIndex) => (
<Substep
trashName={trashName}
Expand All @@ -58,6 +68,7 @@ export function PipettingSubsteps(props: PipettingSubstepsProps): JSX.Element {
volume={row.volume}
source={row.source}
dest={row.dest}
isSameLabware={isSameLabware}
/>
))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ interface SubstepProps {
source?: SubstepWellData
dest?: SubstepWellData
selectSubstep?: (substepIdentifier: SubstepIdentifier) => void
isSameLabware?: boolean
}

function SubstepComponent(props: SubstepProps): JSX.Element {
Expand All @@ -51,6 +52,7 @@ function SubstepComponent(props: SubstepProps): JSX.Element {
dest,
trashName,
selectSubstep: propSelectSubstep,
isSameLabware,
} = props
const { t } = useTranslation(['application', 'protocol_steps', 'shared'])
const compactedSourcePreIngreds = source
Expand All @@ -77,7 +79,7 @@ function SubstepComponent(props: SubstepProps): JSX.Element {
/>
)

const isMix = source?.well === dest?.well
const isMix = source?.well === dest?.well && isSameLabware

return (
<Flex
Expand Down Expand Up @@ -166,43 +168,45 @@ function SubstepComponent(props: SubstepProps): JSX.Element {
</Flex>
</ListItem>
<ListItem type="noActive">
<Flex
gridGap={SPACING.spacing4}
padding={SPACING.spacing12}
justifyContent={JUSTIFY_SPACE_BETWEEN}
width="100%"
alignItems={ALIGN_CENTER}
>
{ingredIds.length > 0 ? (
<Flex gridGap={SPACING.spacing4} alignItems={ALIGN_CENTER}>
<LiquidIcon color={color} size="medium" />
<StyledText desktopStyle="bodyDefaultRegular">
{ingredIds.map(groupId => ingredNames[groupId]).join(',')}
</StyledText>
</Flex>
) : null}
{dest != null || trashName != null ? (
<Flex gridGap={SPACING.spacing4} alignItems={ALIGN_CENTER}>
<StyledText desktopStyle="bodyDefaultRegular">
{t('protocol_steps:dispensed')}
</StyledText>
{volumeTag}
<StyledText desktopStyle="bodyDefaultRegular">
{t('protocol_steps:into')}
</StyledText>
{dest !== undefined ? (
<Flex
gridGap={SPACING.spacing4}
padding={SPACING.spacing12}
justifyContent={JUSTIFY_SPACE_BETWEEN}
width="100%"
alignItems={ALIGN_CENTER}
>
{ingredIds.length > 0 ? (
<Flex gridGap={SPACING.spacing4} alignItems={ALIGN_CENTER}>
<LiquidIcon color={color} size="medium" />
<StyledText desktopStyle="bodyDefaultRegular">
{ingredIds.map(groupId => ingredNames[groupId]).join(',')}
</StyledText>
</Flex>
) : null}
{dest != null || trashName != null ? (
<Flex gridGap={SPACING.spacing4} alignItems={ALIGN_CENTER}>
<StyledText desktopStyle="bodyDefaultRegular">
{t('protocol_steps:dispensed')}
</StyledText>
{volumeTag}
<StyledText desktopStyle="bodyDefaultRegular">
{t('protocol_steps:into')}
</StyledText>

<DeckInfoLabel
deckLabel={
dest?.well != null
? t('protocol_steps:well_name', {
wellName: dest.well,
})
: t(`shared:${trashName}`)
}
/>
</Flex>
) : null}
</Flex>
<DeckInfoLabel
deckLabel={
dest?.well != null
? t('protocol_steps:well_name', {
wellName: dest.well,
})
: t(`shared:${trashName}`)
}
/>
</Flex>
) : null}
</Flex>
) : null}
</ListItem>
</>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,23 @@ const updatePatchOnTiprackChange = (
return patch
}

const updatePatchOnNozzleChange = (
patch: FormPatch,
rawForm: FormData,
pipetteEntities: PipetteEntities
): FormPatch => {
if (
Object.values(pipetteEntities).find(pip => pip.spec.channels === 96) &&
fieldHasChanged(rawForm, patch, 'nozzles')
) {
return {
...patch,
...getDefaultFields('wells'),
}
}
return patch
}

export function dependentFieldsUpdateMix(
originalPatch: FormPatch,
rawForm: FormData, // raw = NOT hydrated
Expand All @@ -188,5 +205,7 @@ export function dependentFieldsUpdateMix(
),
chainPatch => updatePatchOnPipetteChange(chainPatch, rawForm),
chainPatch => updatePatchOnTiprackChange(chainPatch, rawForm),
chainPatch =>
updatePatchOnNozzleChange(chainPatch, rawForm, pipetteEntities),
])
}
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,24 @@ export function updatePatchBlowoutFields(

return patch
}

const updatePatchOnNozzleChange = (
patch: FormPatch,
rawForm: FormData,
pipetteEntities: PipetteEntities
): FormPatch => {
if (
Object.values(pipetteEntities).find(pip => pip.spec.channels === 96) &&
fieldHasChanged(rawForm, patch, 'nozzles')
) {
return {
...patch,
...getDefaultFields('aspirate_wells', 'dispense_wells'),
}
}
return patch
}

export function dependentFieldsUpdateMoveLiquid(
originalPatch: FormPatch,
rawForm: FormData, // raw = NOT hydrated
Expand Down Expand Up @@ -706,5 +724,7 @@ export function dependentFieldsUpdateMoveLiquid(
clampDispenseAirGapVolume(chainPatch, rawForm, pipetteEntities),
chainPatch =>
updatePatchOnTiprackChange(chainPatch, rawForm, pipetteEntities),
chainPatch =>
updatePatchOnNozzleChange(chainPatch, rawForm, pipetteEntities),
])
}

0 comments on commit 1c4385c

Please sign in to comment.