-
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
fix(app): Home pipettes when skipping Drop Tip wizard #15947
Conversation
2e5b348
to
9f8606c
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.
I was under the impression that the EXEC-631 designs (not EXEC-661) had the final copy for this. Some of the current phrasing, like "Exit before completing drop tips?", has a real crash blossom quality to it. We can really tie ourselves in linguistic knots any time we use "drop tip" as a noun — I strongly prefer "dropping a tip" or "tip drop".
I believe part of the issue is that the exit confirmation screen never existed in Figma, however it has existed in code since the wizard's launch (and this was the copy). I do not have the context for why this disparity existed, but design was made aware of this screen yesterday. Would copy along the lines of "Exit before completing tip drop?" work? |
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.
The functionality looks good to me, defer to others on the design and phrasing.
Per convo with @ecormany and Sue, there are copy updates to the confirmation screen. |
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.
New copy looks good, thanks!
Overview
Drop tip wizard CTAs now home the pipette if skipping the flow and close the current run context in both the "skip" and "remove tips" cases. Copy is updated throughout all drop tip flow entry points as well.
This is the first time the app directly POSTs a one-off command via a maintenance run. We do POST one-off commands elsewhere in the app, but we do so using older, less-supported endpoints. If these one-off commands become more common, we should consider creating a dedicated API for doing so (more hooks!). To solve the immediate problem now, I refactored
useDropTipMaintenanceRun
.The real complexity of this PR is managing all the interactions with closing run context and ensuring other clients not engaging with the drop tip flows see/don't see the drop tip CTAs when appropriate. It's probably easiest to follow this PR commit-by-commit.
NOTE: If a user completes a run with tips attached on both pipettes intentionally (the protocol itself has tips attached for both pipettes at the end of the run), the wizard won't display for both pipettes, only the left pipette. The run context closes as soon as the home happens, so there's no second CTA. The skip command won't home the plungers, so that's safe, but current behavior is the user will have to do drop tips via the Instruments page. I imagine this is quite the edge case, but it's worth noting. I can re-address this when I update the tip detection logic shortly.
Here's a complete breakdown of all the changes:
Drop Tip Wizard: Error Recovery, Desktop and ODD
Drop Tip Wizard: Post-Run, Desktop and ODD
Drop Tip Wizard: Instruments Page, Desktop and ODD
Note that the drop-tip banner will no longer be present on the desktop, post-run.
Copy Changes
Note these changes are 1:1 on the ODD when applicable, but only the desktop versions are shown.
Confirm exit before completion, non-error recovery drop tip wizard
![Screenshot 2024-08-08 at 7 22 16 PM](https://private-user-images.githubusercontent.com/64858653/356659727-f2467e53-bfc5-400e-a79a-5c1bd5dd34c9.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkxMDgxODMsIm5iZiI6MTczOTEwNzg4MywicGF0aCI6Ii82NDg1ODY1My8zNTY2NTk3MjctZjI0NjdlNTMtYmZjNS00MDBlLWE3OWEtNWMxYmQ1ZGQzNGM5LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMDklMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjA5VDEzMzEyM1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTdjZTJiYWM0NGY3OWE4MDIxMWRhMWRlZGY2MjE1NWNhZjExZjgyMzRmZDQ3MDU2Nzk2MjdiNzc4ZmZkZmExZDgmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.RnKDZyPJ8qhQDLUINxl8AGywKBXjDmYwsrJLQVHmHdQ)
Post run
Test Plan and Hands on Testing
Changelog
Review requests
Risk assessment
medium - Closing the run context explicitly after DT is new, and doing so has the potential to lead to interesting client-side behavior.