-
Notifications
You must be signed in to change notification settings - Fork 179
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): final copy and animations for golden tip LPC probe screens #13941
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## edge #13941 +/- ##
==========================================
- Coverage 70.71% 70.64% -0.07%
==========================================
Files 2504 2506 +2
Lines 70674 70896 +222
Branches 8666 8745 +79
==========================================
+ Hits 49976 50085 +109
- Misses 18590 18675 +85
- Partials 2108 2136 +28
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
Code looks great! Just one nit. Assuming no one else does, I'll test it out tomorrow AM so we can get it merged 👍
const pipetteMount = pipette?.mount | ||
if (pipetteName == null || pipetteMount == null) return null | ||
let probeVideoSrc = attachProbe1 | ||
let probeLocation = '' |
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.
Maybe we should include a meaningful default value for probeLocation?
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.
We actually want this to be an empty string for a single channel - the i18n string is "Push the pipette ejector up and press the probe firmly onto the {{location}} pipette nozzle as far as it can go." and if the pipette is a single channel we don't specify a location since there is only one nozzle.
@mjhuff thank you! I tested this out on a Flex but haven't yet tested on OT2 |
b2c9ee8
to
43b06ad
Compare
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.
Definitely not blocking and probably nothing you can do about it, but the cal probe on step 5/7 with the align to the top of labware looks pretty low res... I don't know if we have anything better, but we should swap that out at some point.
Looks good though! 🚀
EDIT: Talked to Sue, the new asset should be up soon!
fix RAUT-810, RAUT-811
Overview
Bring golden tip LPC wizard in line with designs and functionality requirements:
Test Plan
Run through LPC on both OT2 and Flex and ensure copy and assets are correct for golden tip and old LPC
Changelog
Review requests
Sanity check code
Risk assessment
Low