-
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): add GEN2 images to change pipette #3734
Conversation
For a number of FE use cases, it is helpful to be able to distinguish "categories" of pipette names. This adds a 'displayCategory' key and value to the pipetteNameSpecs and schema.
Add images for GEN2 single and multi pipettes when changing or viewing attached pipettes. Closes #3630
For a number of FE use cases, it is helpful to be able to distinguish "categories" of pipette names. This adds a 'displayCategory' key and value to the pipetteNameSpecs and schema.
Codecov Report
@@ Coverage Diff @@
## edge #3734 +/- ##
==========================================
+ Coverage 57.14% 57.15% +<.01%
==========================================
Files 815 815
Lines 22489 22490 +1
==========================================
+ Hits 12852 12854 +2
+ Misses 9637 9636 -1
Continue to review full report at Codecov.
|
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.
Updating to use PipetteModel/NameSpecs is a real good call for this component 🛴
|
||
// TODO(mc, 2018-04-06): there must be a better way... |
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.
❓🌮 🌮
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.
🤕
|
||
.flipped_image { | ||
transform: scaleX(-1); | ||
filter: FlipH; |
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.
Looking at caniuse I think transform: scaleX(-1)
should be enough to get us coverage all the way back to IE9 (with our autoprefixing that's already in place). We should get together and propose an actual browser support spec for our projects
|
||
type Props = { | ||
channels?: number, | ||
pipetteSpecs?: ?PipetteNameSpecs | ?PipetteModelSpecs, |
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.
I think in this case we'll be good, but we should consider making PipetteNameSpecs
and PipetteModelSpecs
exact objects to make sure unions behave a little better
imgSrc = channels === 1 ? singleGEN2Src : multiGEN2Src | ||
break | ||
} | ||
default: |
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's the purpose of the default
above case 'OG'
here? Some sort of flow exhaustiveness check?
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.
Yup, I was reasoning that it should fall back to OG images, but I can see an argument for displaying nothing if the displayCategory is missing
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.
💥
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.
LGTM, was not able to test on robot, just mocked out pipette object in code
Overview
Add images for GEN2 single and multi pipettes when changing or viewing attached pipettes.
Closes #3630
Changelog
InstrumentDiagram
in CL to allow for image differentiation on 3 factors (mount, displayCategory, channels)InstructionStep
in RA to allow for image differentiation on 5 factors (mount, displayCategory, channels, direction, diagram)require()
passing a template string as a param, add flow config to ignore this case