Skip to content

fix(protocol-designer): allow moving labware on top of adapters #18541

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jun 11, 2025
Merged

fix(protocol-designer): allow moving labware on top of adapters #18541

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e28935c
fix(protocol-designer): allow moving labware on top of adapters
jerader Jun 5, 2025
0ad1938
fix test
jerader Jun 5, 2025
8b5c0c0
fix ListButton story
jerader Jun 6, 2025
995457a
address comment
jerader Jun 10, 2025
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
5 changes: 5 additions & 0 deletions components/src/atoms/ListButton/ListButton.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ const Template = (args: ListButtonComponentProps): JSX.Element => {
<ListButtonAccordionContainer id="mainAccordionContainer">
<>
<CustomizeExpandButton
enableStackingFF={false}
key="buttonNested"
allowInputField={false}
loadName="mockloadname0"
Expand All @@ -80,6 +81,7 @@ const Template = (args: ListButtonComponentProps): JSX.Element => {
>
<>
<CustomizeExpandButton
enableStackingFF={false}
allowInputField={false}
loadName="mockloadname1"
isSelected={nestedButtonValue === 'radio button1'}
Expand All @@ -96,6 +98,7 @@ const Template = (args: ListButtonComponentProps): JSX.Element => {
) : null}
</>
<CustomizeExpandButton
enableStackingFF={false}
allowInputField={false}
loadName="mockloadname2"
isSelected={nestedButtonValue === 'radio button2'}
Expand All @@ -106,6 +109,7 @@ const Template = (args: ListButtonComponentProps): JSX.Element => {
}}
/>
<CustomizeExpandButton
enableStackingFF={false}
allowInputField={false}
loadName="mockloadname3"
isSelected={nestedButtonValue === 'radio button3'}
Expand All @@ -121,6 +125,7 @@ const Template = (args: ListButtonComponentProps): JSX.Element => {
</>
<>
<CustomizeExpandButton
enableStackingFF={false}
key="buttonNonNested"
allowInputField={false}
loadName="mockloadname4"
Expand Down
1 change: 1 addition & 0 deletions components/src/molecules/CustomizeExpandButton/__tests__/CustomizeExpandButton.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ describe('CustomizeExpandButton', () => {

beforeEach(() => {
props = {
enableStackingFF: true,
buttonText: 'mock text',
buttonValue: 'mockValue',
onChange: vi.fn(),
Expand Down
4 changes: 3 additions & 1 deletion components/src/molecules/CustomizeExpandButton/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export interface StackingProps {
}

interface CustomizeExpandButtonProps extends StyleProps {
enableStackingFF: boolean
allowInputField: boolean
loadName: string
buttonText: string
Expand All @@ -50,6 +51,7 @@ export function CustomizeExpandButton(
stackingProps,
allowInputField,
loadName,
enableStackingFF,
} = props
const isLid =
stackingProps != null &&
Expand Down Expand Up @@ -85,7 +87,7 @@ export function CustomizeExpandButton(
<StyledText desktopStyle="bodyDefaultRegular">
{buttonText}
</StyledText>
{stackingProps != null && isSelected ? (
{stackingProps != null && enableStackingFF && isSelected ? (
<Flex
flexDirection={DIRECTION_COLUMN}
backgroundColor={COLORS.blue10}
Expand Down
32 changes: 32 additions & 0 deletions protocol-designer/src/components/organisms/SelectLabwareModal/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@
{filteredLabwareByCategory[CUSTOM_CATEGORY].map(
({ uri }, index) => (
<CustomizeExpandButton
enableStackingFF={enableStacking}

Check warning on line 435 in protocol-designer/src/components/organisms/SelectLabwareModal/index.tsx

View check run for this annotation

Codecov / codecov/patch

protocol-designer/src/components/organisms/SelectLabwareModal/index.tsx#L435 

Added line #L435 was not covered by tests
loadName={customLabwareDefs[uri].parameters.loadName}
allowInputField={onFlexStacker}
key={`${index}_${uri}`}
Expand Down Expand Up @@ -537,6 +538,7 @@
key={`${index}_${category}_${loadName}`}
>
<CustomizeExpandButton
enableStackingFF={enableStacking}
loadName={loadName}
allowInputField={
onFlexStacker ||
Expand Down Expand Up @@ -611,6 +613,9 @@
defs[tiprackDefUri]
return (
<CustomizeExpandButton
enableStackingFF={
enableStacking

Check warning on line 617 in protocol-designer/src/components/organisms/SelectLabwareModal/index.tsx

View check run for this annotation

Codecov / codecov/patch

protocol-designer/src/components/organisms/SelectLabwareModal/index.tsx#L616-L617 

Added lines #L616 - L617 were not covered by tests
}
loadName={loadName}
allowInputField={false}
key={`${index}_${category}_${loadName}_${tiprackDefUri}`}
Expand Down Expand Up @@ -665,6 +670,30 @@
null && slot !== 'offDeck'
? {
...stackingPropsShared,
checkboxCaption: t(
'with_lid',
{
name:
defs[
stackingLabwareDefUri
].metadata
.displayName,
}
),
checked:
selectedLidLabware !=
null,
onCheckboxChange: () => {
dispatch(
selectLid({
labwareDefURI:
selectedLidLabware ===
stackingLabwareDefUri
? null
: stackingLabwareDefUri,
})
)
},

Check warning on line 696 in protocol-designer/src/components/organisms/SelectLabwareModal/index.tsx

View check run for this annotation

Codecov / codecov/patch

protocol-designer/src/components/organisms/SelectLabwareModal/index.tsx#L673-L696 

Added lines #L673 - L696 were not covered by tests
inputCaption: t(
'valid_range',
{
Expand Down Expand Up @@ -700,6 +729,9 @@

return (
<CustomizeExpandButton
enableStackingFF={
enableStacking
}
loadName={
nestedDef.parameters
.loadName
Expand Down
22 changes: 16 additions & 6 deletions protocol-designer/src/top-selectors/labware-locations/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
} from '@opentrons/shared-data'
import {
COLUMN_4_SLOTS,
getSlotInLocationStack,
getTopLocationInStack,
} from '@opentrons/step-generation'

Expand Down Expand Up @@ -185,11 +186,19 @@

const unoccupiedAdapterOptions = Object.entries(labware).reduce<Option[]>(
(acc, [labwareId, labwareOnDeck]) => {
const labwareOnAdapter = Object.values(labware).find(
temporalProperties =>
getTopLocationInStack(temporalProperties.stack) === labwareId
const hasLabwareAboveAdapter = Object.values(labware).reduce(
(acc, temporalProperties) => {
if (temporalProperties.stack.includes(labwareId)) {
const topId = getTopLocationInStack(temporalProperties.stack)
if (topId !== labwareId) {
return true
}
}
return acc
},
false

Check warning on line 199 in protocol-designer/src/top-selectors/labware-locations/index.ts

View check run for this annotation

Codecov / codecov/patch

protocol-designer/src/top-selectors/labware-locations/index.ts#L189-L199 

Added lines #L189 - L199 were not covered by tests
)
const adapterSlot = getTopLocationInStack(labwareOnDeck.stack)
const adapterSlot = getSlotInLocationStack(labwareOnDeck.stack)

Check warning on line 201 in protocol-designer/src/top-selectors/labware-locations/index.ts

View check run for this annotation

Codecov / codecov/patch

protocol-designer/src/top-selectors/labware-locations/index.ts#L201 

Added line #L201 was not covered by tests
const modIdWithAdapter = Object.keys(modules).find(modId =>
labwareOnDeck.stack.includes(modId)
)
Expand All @@ -204,7 +213,8 @@
: 'unknown module'
const moduleSlotInfo = modSlot ?? 'unknown slot'
const adapterSlotInfo = adapterSlot ?? 'unknown adapter'
return labwareOnAdapter == null && isAdapter

return isAdapter && !hasLabwareAboveAdapter

Check warning on line 217 in protocol-designer/src/top-selectors/labware-locations/index.ts

View check run for this annotation

Codecov / codecov/patch

protocol-designer/src/top-selectors/labware-locations/index.ts#L217 

Added line #L217 was not covered by tests
? [
...acc,
{
Expand All @@ -223,7 +233,7 @@
const unoccupiedModuleOptions = Object.entries(modules).reduce<Option[]>(
(acc, [modId, modOnDeck]) => {
const moduleHasLabware = Object.entries(labware).some(
([_, lwOnDeck]) => lwOnDeck.stack[0] === modId
([_, lwOnDeck]) => lwOnDeck.stack[lwOnDeck.stack.length - 2] === modId

Check warning on line 236 in protocol-designer/src/top-selectors/labware-locations/index.ts

View check run for this annotation

Codecov / codecov/patch

protocol-designer/src/top-selectors/labware-locations/index.ts#L236 

Added line #L236 was not covered by tests
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is this logic?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the module id in the stack will be the 2nd item in the stack array from the end of the array. So if the stack includes the module id, then we know the module has labware.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh I see. I worry slightly about hardcoding in that index as our stacking capabilities theoretically expand. For example— what if you have a lid on top of a labware on top of an adapter on top of a module?

While it might be overkill for now, I recommend a utility for like getModuleInStack that returns the module ID of a stack if one exists.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The module id will always be in the same index of the array though: ['labwareId', 'adapterId', 'moduleId', 'slotId'] But i agree, if the module is on an adapter some day, this would break. is it ok if i fix this in edge though for TC lid stuff?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes that works!

)
const type = moduleEntities[modId].type
const slot = modOnDeck.slot
Expand Down
Loading