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): pipette shell calibration flow #11618

Merged
merged 3 commits into from
Oct 26, 2022
Merged

Conversation

jerader
Copy link
Collaborator

@jerader jerader commented Oct 25, 2022

Closes RLIQ-219 RLIQ-126

Co-authored with @smb2268

Overview

This creates the shell for the pipette calibrate flow and wires it up to the pipette overflow menu and pipette card

Changelog

  • create PipetteWizardFlows component - to be used as the parent component for all the ot-3 pipette flows
  • create getPIpetteWizardSteps which grabs the steps needed for the calibration flow right now but will be used to grab the steps for the other flows later on
  • create the types and constants for the PipetteWizardFlows work
  • creates 4 components for each step in the calibration flow: BeforeBeginning, AttachStem, DetachStem, Result
  • PipetteWizardFlows is wired up to PipetteOverflowMenu and PipetteCard.
  • each component has a test file created
  • create pipette_wizard_flows json file for i18n keys associated with the flows

Review requests

  • in test.json in robot-server, change a pipette to a gen3 pipette. In the app, go to the protocol card and click on Calibrate Pipette Offset, click through the 4 pages in the flow by pressing on the next button. You shold be able to click through all pages and close the flow

Risk assessment

low

@jerader jerader requested a review from a team as a code owner October 25, 2022 19:09
@jerader jerader requested review from b-cooper, smb2268 and a team and removed request for a team October 25, 2022 19:09
@codecov
Copy link

codecov bot commented Oct 25, 2022

Codecov Report

Merging #11618 (9b50c33) into edge (7825a89) will decrease coverage by 0.03%.
The diff coverage is 23.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #11618      +/-   ##
==========================================
- Coverage   75.01%   74.98%   -0.04%     
==========================================
  Files        2059     2066       +7     
  Lines       57481    57522      +41     
  Branches     5999     6007       +8     
==========================================
+ Hits        43122    43131       +9     
- Misses      12950    12981      +31     
- Partials     1409     1410       +1     
Flag Coverage Δ
app 75.16% <23.25%> (-0.18%) ⬇️

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

Impacted Files Coverage Δ
app/src/assets/localization/en/index.ts 100.00% <ø> (ø)
...anisms/Devices/PipetteCard/PipetteOverflowMenu.tsx 100.00% <ø> (ø)
app/src/organisms/PipetteWizardFlows/index.tsx 5.00% <5.00%> (ø)
...anisms/PipetteWizardFlows/getPipetteWizardSteps.ts 25.00% <25.00%> (ø)
...pp/src/organisms/PipetteWizardFlows/AttachStem.tsx 33.33% <33.33%> (ø)
...c/organisms/PipetteWizardFlows/BeforeBeginning.tsx 33.33% <33.33%> (ø)
...pp/src/organisms/PipetteWizardFlows/DetachStem.tsx 33.33% <33.33%> (ø)
app/src/organisms/PipetteWizardFlows/Results.tsx 33.33% <33.33%> (ø)
app/src/organisms/Devices/PipetteCard/index.tsx 60.00% <40.00%> (-0.61%) ⬇️
app/src/organisms/PipetteWizardFlows/constants.ts 100.00% <100.00%> (ø)

export interface PipetteWizardStepProps {
flowType: PipetteWizardFlow
mount: PipetteMount
nextStep: () => void
Copy link
Contributor

Choose a reason for hiding this comment

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

nextStep reads as a noun here, so based on our naming conventions, I would expect it to be an object that contains the details of the next step.

We generally name functions with active verbs like proceed or proceedToNextStep, either of which seem clearer than nextStep

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That makes sense, thanks Brian. I'll rename it to proceed. I will do so in this pr to avoid a bunch of merge conflicts since I made that branch off of this branch.

Copy link
Contributor

@b-cooper b-cooper left a comment

Choose a reason for hiding this comment

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

This is awesome ground work! It's shaping up to be a powerful abstraction 🔋

@jerader jerader merged commit 2dd6aac into edge Oct 26, 2022
@jerader jerader deleted the app_shell-calibration-flow branch October 26, 2022 22:19
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.

3 participants