feat(ODD): HiFi designs and features for the protocol details page #12330
Conversation
Codecov Report
@@ Coverage Diff @@
## edge #12330 +/- ##
==========================================
+ Coverage 73.42% 73.77% +0.35%
==========================================
Files 1487 2219 +732
Lines 49037 61356 +12319
Branches 2983 6326 +3343
==========================================
+ Hits 36005 45266 +9261
- Misses 12578 14616 +2038
- Partials 454 1474 +1020
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Screen.Recording.2023-03-21.at.11.28.22.AM.mov |
ewagoner
force-pushed
the
RCORE-684-protocol-details-hifi
branch
2 times, most recently
from
0ab3038
to
f39fc6c
Compare
| lineHeight={TYPOGRAPHY.lineHeight28} | ||
| paddingLeft={SPACING.spacing5} | ||
| > | ||
| {getHardwareLocation(hardware, t)} |
There was a problem hiding this comment.
not sure if there is a separate ticket to address this but the UI for the hardware locations don't match what is in figma. Slots should only say the number with the border around them and mounts should be capitalized.
There was a problem hiding this comment.
I'm not sure if there is a separate ticket or not, but this is the same thing I asked about a couple of weeks ago re: the Labware Setup screen. I was going to make a function to map from the old "slots 1-12" format to the "a1, a2, ... etc" format but I believe the decision was to hold off on that until we just get the expected format from the backend. As of right now both the module setup and labware setup screens are also displaying locations as "slot #"
There was a problem hiding this comment.
Not a separate ticket yet (that I know of) but it is one of the TODO items in the PR description that I plan on spinning off into new tickets when this gets merged.
There was a problem hiding this comment.
Got it, thanks yall! The change will need to be added to the Liquids setup screen as well.
| lineHeight={TYPOGRAPHY.lineHeight28} | ||
| paddingLeft={SPACING.spacing5} | ||
| > | ||
| {getHardwareLocation(hardware, t)} |
There was a problem hiding this comment.
For mount, the design uses Right Mount so probably we would need to add textTransform to StyledText.
textTransform={TYPOGRAPHY. textTransformCapitalize}
There was a problem hiding this comment.
It's not just that -- the design shows iconography and other sugar, so I left the contents of this table as a TODO item on this PR.
| 'protocol_details:creation_method' | ||
| )}: ${getCreationMethod(protocolType)}`}</StyledText> | ||
| <Flex alignItems={ALIGN_CENTER} maxHeight="3.75rem" minWidth="15.6875rem"> | ||
| <NewPrimaryBtn |
There was a problem hiding this comment.
Can we use small button?
now the button link to small button.
There was a problem hiding this comment.
Maybe. Yesterday, Figma called this a "custom CTA" so that's what I made. I'll see if I can use the common component instead.
There was a problem hiding this comment.
Jethary is addressing feedback from Mel so you can leave this now, but please leave the comment.
app/src/pages/OnDeviceDisplay/ProtocolDetails/__tests__/ProtocolDetails.test.tsx
Show resolved
Hide resolved
|
TabbedButton focus still shows outline. Do you plan to update TabbedButton in a following PR? |
I saw that conversation in slack today. I'll fix both TabbedButton and MediumButton as a separate commit to this PR, since I'm here already using them both. Edit: I didn't have the errant box shadow in the MediumButton, so I just fixed the TabbedButton. |
ewagoner
force-pushed
the
RCORE-684-protocol-details-hifi
branch
2 times, most recently
from
898ab2a
to
6879b6e
Compare
|
I've addressed all the feedback on this PR. I do want to try to refactor the MediumButton to make it in line with this comment on the SmallButton PR: #12340 (review) |
|
I think this is finally good to go, again, for real this time. Re-requesting reviews! |
ewagoner
force-pushed
the
RCORE-684-protocol-details-hifi
branch
from
22468d9
to
23ab256
Compare
Overview
This PR implements the designs and features found in Figma at https://www.figma.com/file/AoTLAYuWawlaWItB1umOjr/Opentrons-Flex-Touchscreen-Designs?node-id=1-212&t=Y7Cv06vknVeA2a4h-0
TODO Items:
Closes RCORE-684
Test Plan
Unit tests have been added that cover the bulk of the page functionality. Storybook Stories have been created where appropriate, and verified visually. I also verified the page in both desktop mode and on the dev kit ODD.
Changelog
Review requests
No special requests.
Risk assessment
There is no risk to current users, as this work is confined to the ODD.