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

fix(app): ensure gantry not blocking pcr seal placement #4071

Merged
merged 2 commits into from
Sep 17, 2019

Conversation

b-cooper
Copy link
Contributor

Overview

The prompt to place the PCR seal in the app was occuring while the gantry was still hovering over
the TC's labware. This changes the order of the confirmation process, to allow the tip to be
returned before prompting about the pcr seal

Closes #4034

NOTE: This also fixes a small regression relating to getUnpreparedModule that was introduced by #3994

review requests

  • Please go through the calibration process for a simple TC protocol. At the end, you should only see the "Place PCR Seal" modal after you have pressed "Return Tip and Proceed to Run".
  • confirm the gantry is actually in a safe place away from the TC, and youy could actually place a seal on it.

The prompt to place the PCR seal in the app was occuring while the gantry was still hovering over
the TC's labware. This changes the order of the confirmation process, to allow the tip to be
returned before prompting about the pcr seal

Closes #4034
@b-cooper b-cooper added app Affects the `app` project ready for review fix PR fixes a bug labels Sep 17, 2019
@b-cooper b-cooper requested review from mcous and a team September 17, 2019 18:50
Copy link
Contributor

@mcous mcous left a comment

Choose a reason for hiding this comment

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

Behavior and code look good. Sorry about the regression, thanks for the catch

@codecov
Copy link

codecov bot commented Sep 17, 2019

Codecov Report

Merging #4071 into edge will decrease coverage by 19.19%.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff            @@
##            edge   #4071      +/-   ##
========================================
- Coverage   76.4%   57.2%   -19.2%     
========================================
  Files        110     855     +745     
  Lines      13225   23996   +10771     
========================================
+ Hits       10105   13728    +3623     
- Misses      3120   10268    +7148
Impacted Files Coverage Δ
...pp/src/components/CalibrateLabware/ProceedToRun.js 0% <0%> (ø)
app/src/robot-api/resources/modules.js 38.59% <0%> (ø)
...omponents/DeckSetup/LabwareOverlays/LabwareName.js 0% <0%> (ø)
...library/src/labware-creator/components/Dropdown.js 0% <0%> (ø)
protocol-designer/src/analytics/actions.js 0% <0%> (ø)
labware-library/src/localization/en.js 100% <0%> (ø)
...areUploadMessageModal/LabwareUploadMessageModal.js 0% <0%> (ø)
...etNextRobotStateAndWarnings/forAspirateDispense.js 100% <0%> (ø)
...designer/src/components/FileSidebar/FileSidebar.js 0% <0%> (ø)
...p/src/components/TipProbe/InstrumentMovingPanel.js 0% <0%> (ø)
... and 737 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0629b8...181e52a. Read the comment docs.

@b-cooper b-cooper merged commit 01d6858 into edge Sep 17, 2019
@b-cooper b-cooper deleted the app_move-gantry-pcr-seal branch September 17, 2019 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app Affects the `app` project fix PR fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PCR Seal Prompt Gantry Move
2 participants