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): final copy and animations for golden tip LPC probe screens #13941

Merged
merged 10 commits into from
Nov 9, 2023

Conversation

smb2268
Copy link
Contributor

@smb2268 smb2268 commented Nov 7, 2023

fix RAUT-810, RAUT-811

Overview

Bring golden tip LPC wizard in line with designs and functionality requirements:

Screen Shot 2023-11-07 at 6 01 36 PM Screen Shot 2023-11-07 at 5 43 59 PM Screen Shot 2023-11-07 at 5 41 54 PM Screen Shot 2023-11-07 at 5 33 00 PM Screen Shot 2023-11-07 at 5 32 49 PM

Test Plan

Run through LPC on both OT2 and Flex and ensure copy and assets are correct for golden tip and old LPC

Changelog

  1. Bring copy and assets in line with designs for golden tip LPC
  2. Add move to maintenance position command to beginning of AttachProbe and DetachProbe, ensure probe is attached before proceeding with LPC

Review requests

Sanity check code

Risk assessment

Low

Screen Shot 2023-11-07 at 5 32 35 PM

@smb2268 smb2268 self-assigned this Nov 7, 2023
@smb2268 smb2268 requested a review from a team as a code owner November 7, 2023 22:18
Copy link

codecov bot commented Nov 7, 2023

Codecov Report

Merging #13941 (cce0a07) into edge (bcb4d9b) will decrease coverage by 0.07%.
Report is 5 commits behind head on edge.
The diff coverage is 8.57%.

Additional details and impacted files

Impacted file tree graph

@@            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     
Flag Coverage Δ
app 67.82% <8.57%> (-0.26%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...p/src/organisms/LabwarePositionCheck/PickUpTip.tsx 61.19% <ø> (ø)
...p/src/organisms/PipetteWizardFlows/AttachProbe.tsx 75.51% <0.00%> (+4.08%) ⬆️
...p/src/organisms/LabwarePositionCheck/CheckItem.tsx 71.60% <0.00%> (-1.82%) ⬇️
...rganisms/LabwarePositionCheck/ExitConfirmation.tsx 71.42% <60.00%> (-8.58%) ⬇️
...arePositionCheck/LabwarePositionCheckComponent.tsx 2.88% <0.00%> (ø)
...ganisms/LabwarePositionCheck/IntroScreen/index.tsx 14.28% <0.00%> (-1.72%) ⬇️
...p/src/organisms/LabwarePositionCheck/JogToWell.tsx 68.57% <0.00%> (-4.16%) ⬇️
...src/organisms/LabwarePositionCheck/DetachProbe.tsx 7.69% <0.00%> (-3.42%) ⬇️
.../organisms/PipetteWizardFlows/ProbeNotAttached.tsx 21.42% <21.42%> (ø)
...src/organisms/LabwarePositionCheck/AttachProbe.tsx 4.76% <0.00%> (-5.24%) ⬇️

... and 13 files with indirect coverage changes

Copy link
Contributor

@mjhuff mjhuff 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 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 = ''
Copy link
Contributor

@mjhuff mjhuff Nov 8, 2023

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?

Copy link
Contributor Author

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.

@smb2268
Copy link
Contributor Author

smb2268 commented Nov 8, 2023

Code looks great! Just one nit. Assuming no one else does, I'll test it out tomorrow AM so we can get it merged 👍

@mjhuff thank you! I tested this out on a Flex but haven't yet tested on OT2

@smb2268 smb2268 force-pushed the app_golden-lpc-updates branch from b2c9ee8 to 43b06ad Compare November 8, 2023 23:04
Copy link
Contributor

@mjhuff mjhuff left a 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!

@smb2268 smb2268 merged commit 4d56c06 into edge Nov 9, 2023
@smb2268 smb2268 deleted the app_golden-lpc-updates branch November 9, 2023 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants