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): add desktop and ODD droptip wizard loading/confirmation/errors on app #13815

Merged
merged 13 commits into from
Oct 25, 2023

Conversation

ncdiehl11
Copy link
Collaborator

@ncdiehl11 ncdiehl11 commented Oct 19, 2023

closes RAUT-796

Overview

Adds functionality to Droptip Wizard, both on desktop and ODD. Handles loading, error, and confirmation states for robot gantry movement, blowout action, and tip drop action. Adds video assets for initial blowout/tip drop selection.

Test Plan

  • Verify selections proceed properly for blowout/tip drop selecitons
  • Verify deck slot selection and jog controls work properly
  • Verify loading states correspond to action that is being performed
  • Verify confirmation tile renders after jogging to ensure the intended position is reached before blowout/tip drop
  • Note that ODD testing is contingent on this branch being merged to expose ODD entrypoint from instrument detail page

Review requests

@mjhuff (collaborator)

Risk assessment

medium

@ncdiehl11 ncdiehl11 requested a review from mjhuff October 19, 2023 16:02
@ncdiehl11 ncdiehl11 requested a review from a team as a code owner October 19, 2023 16:02
@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Merging #13815 (a503baf) into edge (60aaf7d) will decrease coverage by 0.25%.
Report is 91 commits behind head on edge.
The diff coverage is 5.49%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #13815      +/-   ##
==========================================
- Coverage   70.75%   70.51%   -0.25%     
==========================================
  Files        2465     2474       +9     
  Lines       69346    71605    +2259     
  Branches     8338     9220     +882     
==========================================
+ Hits        49069    50495    +1426     
- Misses      18317    18991     +674     
- Partials     1960     2119     +159     
Flag Coverage Δ
app 67.40% <5.49%> (-0.72%) ⬇️

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

Files Coverage Δ
...p/src/organisms/DropTipWizard/ExitConfirmation.tsx 0.00% <0.00%> (ø)
app/src/organisms/DropTipWizard/Success.tsx 20.00% <0.00%> (+3.33%) ⬆️
app/src/organisms/DropTipWizard/ChooseLocation.tsx 8.00% <0.00%> (-6.29%) ⬇️
...pp/src/organisms/DropTipWizard/BeforeBeginning.tsx 25.00% <6.66%> (-12.50%) ⬇️
app/src/organisms/DropTipWizard/JogToPosition.tsx 11.53% <8.33%> (-8.47%) ⬇️
app/src/organisms/DropTipWizard/index.tsx 3.27% <5.26%> (+0.55%) ⬆️

... and 91 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.

Having some difficulty with the flows:

  • On Desktop - I'm stuck on the choose drop-tip location step. Clicking the confirm position doesn't cause the flow to proceed.
  • On ODD - Choosing drop tip location doesn't move the gantry, and the jog controls don't appear to move the robot. I can click through all the way to "is the pipette positioned where the tips should be dropped?", but clicking drop tips doesn't do anything.

I'll look at the code once we get the functionality hammered out. Happy to pair some more if you want!

@ncdiehl11 ncdiehl11 marked this pull request as draft October 19, 2023 19:29
@ncdiehl11 ncdiehl11 marked this pull request as ready for review October 20, 2023 16:03
@b-cooper b-cooper requested a review from a team as a code owner October 20, 2023 16:54
@b-cooper b-cooper force-pushed the feat_droptip_wizard_loading_confirmation_errors branch from 88e4dc1 to 83f41a8 Compare October 20, 2023 16:55
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.

Looks great, Nick! I added some cleanup/comments and considerations, but it works 🔥

Per Slack convo, outside of the gantry moving after blowout, I don't think we need to consider any functional changes (outside of the one padding issue).

Thanks for being patient with me trying to test this out!

app/src/assets/localization/en/drop_tip_wizard.json Outdated Show resolved Hide resolved
app/src/organisms/DropTipWizard/BeforeBeginning.tsx Outdated Show resolved Hide resolved
app/src/organisms/DropTipWizard/BeforeBeginning.tsx Outdated Show resolved Hide resolved
const handleProceed = (): void => {
handleCreateAndSetup(flowType === 'liquid_and_tips')
}

return (
return isOnDevice ? (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason we aren't making use of GenericWizardTile here and in the other steps? I notice that these wizard steps have way more styling than step components in other flows (pipette, gripper, etc.). Maybe I'm missing something?

If this is something that actually needs refactoring but is time intensive, we can clean it up later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I completely overlooked the GenericWizardTile and was styling from scratch. I will refactor to make use of GenericWizardTile

app/src/organisms/DropTipWizard/index.tsx Outdated Show resolved Hide resolved
app/src/organisms/DropTipWizard/index.tsx Outdated Show resolved Hide resolved
app/src/organisms/DropTipWizard/index.tsx Outdated Show resolved Hide resolved
app/src/organisms/DropTipWizard/index.tsx Outdated Show resolved Hide resolved
app/src/organisms/DropTipWizard/index.tsx Outdated Show resolved Hide resolved
@ncdiehl11 ncdiehl11 requested a review from mjhuff October 24, 2023 19:51
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.

LGTM! 🚀

@ncdiehl11 ncdiehl11 merged commit 8c7c5f6 into edge Oct 25, 2023
21 of 22 checks passed
@ncdiehl11 ncdiehl11 deleted the feat_droptip_wizard_loading_confirmation_errors branch October 25, 2023 13:49
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