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

feat(app): prompt to open TC lid before labware calibration #3853

Merged
merged 7 commits into from
Aug 9, 2019

Conversation

b-cooper
Copy link
Contributor

@b-cooper b-cooper commented Aug 9, 2019

Overview

Provide a mechanism to open the lid of the Thermocycler Module during pre-run calibration, so that the contained labware can be calibrated.

Closes #3066

Review Requests

  • with a closed thermocycler on deck, upload a thermocycler protocol to the robot and proceed to labware calibration
  • after you've confirmed the module is connected, you should be prompted to open the lid before proceeding
  • try again with the lid already open, you shouldn't see the blocking modal at all.

NOTE

The firmware/hardware combination will not report 'closed' because of latch complications, until new units are in, the behavior of the "Open Lid" button will be either behave as if it's in progress. In order to allow testing, we keep the button enabled for now, despite the loading state. There is a TODO

Provide a mechanism to open the lid of the Thermocycler Module during pre-run calibration, so that
the contained labware can be calibrated.

Closes #3066
@b-cooper b-cooper added feature Ticket is a feature request / PR introduces a feature app Affects the `app` project ready for review labels Aug 9, 2019
@b-cooper b-cooper requested review from mcous and Kadee80 August 9, 2019 17:14
@codecov
Copy link

codecov bot commented Aug 9, 2019

Codecov Report

Merging #3853 into edge will decrease coverage by 0.07%.
The diff coverage is 11.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge    #3853      +/-   ##
==========================================
- Coverage   34.51%   34.44%   -0.08%     
==========================================
  Files         742      743       +1     
  Lines       11770    11801      +31     
==========================================
+ Hits         4063     4065       +2     
- Misses       7707     7736      +29
Impacted Files Coverage Δ
app/src/components/ModuleControls/index.js 0% <0%> (ø) ⬆️
app/src/components/PrepareModules/index.js 0% <0%> (ø)
app/src/pages/Calibrate/Labware.js 0% <0%> (ø) ⬆️
app/src/components/ModuleLiveStatusCards/index.js 0% <0%> (ø) ⬆️
app/src/robot-api/resources/modules.js 41.66% <26.31%> (-14.59%) ⬇️

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 72952ba...87993a5. Read the comment docs.

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.

Code looks good, UI behaves as expected with TC attached 🍵

)
}

function mapDispatchToProps(dispatch: Dispatch, ownProps: OP): DP {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a change request, but since this is a new component that only uses mapDispatchToProps, useDispatch could make this component more compact and (in theory) more readable

@b-cooper b-cooper merged commit 2b7efbc into edge Aug 9, 2019
@mcous mcous deleted the app_open-tc-calibrate branch August 21, 2019 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app Affects the `app` project feature Ticket is a feature request / PR introduces a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Thermocycler: Run App: Open Lid Before Calibration
2 participants