-
Notifications
You must be signed in to change notification settings - Fork 184
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.
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
base: chore_release-pd-8.5.0
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## chore_release-pd-8.5.0 #18541 +/- ##
==========================================================
- Coverage 23.91% 23.69% -0.23%
==========================================================
Files 3228 3228
Lines 278220 278251 +31
Branches 27384 27552 +168
==========================================================
- Hits 66530 65923 -607
- Misses 211663 212301 +638
Partials 27 27
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
@@ -223,7 +233,7 @@ export const getUnoccupiedLabwareLocationOptions: Selector< | |||
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this logic?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
closes RQA-4247
Overview
This PR fixes a bug where you couldn't move a labware onto an adapter and also removes the checkbox for adding a TC lid to a labware on an adapter in the select labware modal
Test Plan and Hands on Testing
Upload the attached protocol and try to move the tough plate to the adapter on the temperature module and see that it moves correctly.
Also, if you try to add the 96-well aluminum block on a temperature module with a 96 well plate during deck setup, you shouldn't see the checkbox option to add the TC lid when the stacking capabilities FF is turned off.
untitled (100).json
Changelog
Risk assessment
low