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

refactor(protocol-designer): change tiprack default slot and copy upd… #14028

Merged
merged 1 commit into from
Nov 20, 2023

Conversation

jerader
Copy link
Collaborator

@jerader jerader commented Nov 20, 2023

…ates

closes RAUT-875

Overview

This PR does a few things as discussed in a slack thread. They are all quality of life improvements from Anurag:

  1. updates the copy in the pipette type tiles to specify "left" and "right" instead of "first" and "second"
  2. adds a default slot for the generated pipettes. instead of being generated in the next available slot, they are generated in the center column for both the Ot-2 and Flex. This means that you can modify the modules without having to worry about the slot being occupied by the default tiprack
  3. updates the copy for an occupied slot warning when editing the modules to something more descriptive

Test Plan

Create a Flex protocol and notice that the pipette type tiles include which mount you are selecting a pipetting for. Make sure you add a pipette to both mounts. Once you've created the protocol, navigate to the design tab and see that the tipracks are generated in slots B2 and C2. Move one of the tipracks into slot A2. Then add a magnetic block from the file tab and see that the error says that the slot is occupied.

Now create a new protocol for the OT-2. Add both pipettes. Go to the design tab and see that the tipracks are generated in slots 2 and 5.

Changelog

  • add a slot for the generated tips
  • change the copy for the alert text and the modal tile headers

Review requests

see test plan

Risk assessment

low

@jerader jerader requested a review from a team November 20, 2023 19:10
Comment on lines +267 to +268
const ot2Slots = index === 0 ? '2' : '5'
const flexSlots = index === 0 ? 'C2' : 'B2'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

any suggestions for a more elegant way to accomplish this? It works but idk if it could be cleaner.

Copy link
Contributor

@mjhuff mjhuff Nov 20, 2023

Choose a reason for hiding this comment

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

Off the top of my head, I can't think of anything clearer than this.

The only thing slightly confusing IMO is variable assignment above the dispatch and then property assignment within the dispatch. Maybe something like this? Again, just my opinion - this is definitely nit territory.

const ot2Slots = index === 0 ? '2' : '5'
        const flexSlots = index === 0 ? 'C2' : 'B2'
        const slot =
          values.fields.robotType === FLEX_ROBOT_TYPE ? flexSlots : ot2Slots
        const adapterUnderLabwareDefURI =
          values.pipettesByMount.left.pipetteName === 'p1000_96'
            ? adapter96ChannelDefUri
            : undefined
    dispatch(
      labwareIngredActions.createContainer({
        slot,
        labwareDefURI: tiprackDefURI,
        adapterUnderLabwareDefURI,
      })
    )`

Copy link
Contributor

Choose a reason for hiding this comment

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

I think L267-268 is clear enough.
I think Jamey's suggestion makes the code more readable and easier to debug.

Copy link

codecov bot commented Nov 20, 2023

Codecov Report

Merging #14028 (d199c93) into edge (d526537) will increase coverage by 0.00%.
Report is 1 commits behind head on edge.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             edge   #14028   +/-   ##
=======================================
  Coverage   70.66%   70.66%           
=======================================
  Files        1628     1628           
  Lines       54273    54276    +3     
  Branches     3872     3875    +3     
=======================================
+ Hits        38351    38354    +3     
  Misses      15247    15247           
  Partials      675      675           
Flag Coverage Δ
protocol-designer 45.62% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...onents/modals/CreateFileWizard/PipetteTypeTile.tsx 84.84% <ø> (ø)
...r/src/components/modals/CreateFileWizard/index.tsx 78.84% <100.00%> (+0.62%) ⬆️

Copy link
Contributor

@mjhuff mjhuff left a comment

Choose a reason for hiding this comment

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

🚀

@jerader jerader merged commit e348535 into edge Nov 20, 2023
22 checks passed
@jerader jerader deleted the pd_misc-fixes-RAUT-875 branch November 20, 2023 20:02
Copy link
Contributor

@koji koji left a comment

Choose a reason for hiding this comment

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

The changes look good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants