Skip to content

feat(app): Add Submerge screen to QT Aspirate and Dispense #18209

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

Merged
merged 8 commits into from
May 2, 2025

Conversation

koji
Copy link
Contributor

@koji koji commented Apr 29, 2025

Overview

add Submerge to QuickTransfer's Aspirate tab and Dispense Tab

close AUTH-913 and AUTH-914

Test Plan and Hands on Testing

Changelog

  • add Submerge and its test
  • update state type
  • update actions of useReducer

Review requests

Risk assessment

low

@koji koji requested review from ncdiehl11 and jerader April 29, 2025 20:17
@koji koji marked this pull request as ready for review April 29, 2025 20:21
@koji koji requested a review from a team as a code owner April 29, 2025 20:21
Comment on lines +191 to +219
<>
<Flex
width="30.5rem"
height="100%"
gridGap={SPACING.spacing24}
flexDirection={DIRECTION_COLUMN}
marginTop={SPACING.spacing68}
>
<StyledText oddStyle="level4HeaderRegular">
{t('submerge_aspirating_description')}
</StyledText>
<InputField type="number" value={speed} title={t('speed')} readOnly />
</Flex>
<Flex
paddingX={SPACING.spacing24}
height="21.25rem"
marginTop="7.75rem"
borderRadius="0"
>
<NumericalKeyboard
key={`${kind}_speed_keyboard`}
keyboardRef={keyboardRef}
initialValue={String(speed ?? '')}
onChange={e => {
setSpeed(Number(e))
}}
/>
</Flex>
</>
Copy link
Contributor Author

@koji koji Apr 29, 2025

Choose a reason for hiding this comment

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

Using this because of https://opentrons.slack.com/archives/C07JJA1AJFQ/p1745864136081549
once i sync with Sue, i think i would be able to make a common component for the main part (input field + numerical keyboard) and reduce redundancy of this part (including the following one)
For size, there is diff between Primary design and the currnet prod. this pr aligns with the currnet prod size. I will disscuss this with Sue.

@koji koji removed the request for review from a team April 29, 2025 20:30
onClickBack={handleClickBackOrExit}
onClickButton={handleClickSaveOrContinue}
top={SPACING.spacing8}
// buttonIsDisabled={buttonIsDisabled}
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, i forgot to update the disabled.
i will update this.

@@ -53,23 +53,23 @@ describe('SelectPipettePath', () => {
screen.getByText('Single transfers')
})

it('should render sinle transfer when transfer is 1:n', () => {
it('should render single transfer when transfer is 1:n', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice catch lol

@@ -95,7 +95,7 @@ export function Delay(props: DelayProps): JSX.Element {
trackEventWithRobotSerial({
name: ANALYTICS_QUICK_TRANSFER_SETTING_SAVED,
properties: {
settting: `Delay_${kind}`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice catch here too, i guess this has been affecting mixpanel

@@ -144,7 +144,7 @@ export function BlowOut(props: BlowOutProps): JSX.Element {
trackEventWithRobotSerial({
name: ANALYTICS_QUICK_TRANSFER_SETTING_SAVED,
properties: {
settting: `BlowOut`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

😄 nice

Copy link
Collaborator

@jerader jerader left a comment

Choose a reason for hiding this comment

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

lgtm, just left some minor comments

Copy link

codecov bot commented May 1, 2025

Codecov Report

Attention: Patch coverage is 75.09881% with 63 lines in your changes missing coverage. Please review.

Project coverage is 57.72%. Comparing base (9e81f8f) to head (f8314df).
Report is 46 commits behind head on edge.

Files with missing lines Patch % Lines
...ferFlow/QuickTransferAdvancedSettings/Submerge.tsx 83.88% 34 Missing ⚠️
...pp/src/organisms/ODD/QuickTransferFlow/reducers.ts 0.00% 12 Missing ⚠️
...erFlow/Aspirate/hooks/useAspirateSettingsConfig.ts 0.00% 11 Missing ⚠️
...erFlow/Dispense/hooks/useDispenseSettingsConfig.ts 33.33% 6 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #18209      +/-   ##
==========================================
+ Coverage   57.53%   57.72%   +0.18%     
==========================================
  Files        3039     3044       +5     
  Lines      254623   256455    +1832     
  Branches    30448    30890     +442     
==========================================
+ Hits       146507   148036    +1529     
- Misses     107930   108236     +306     
+ Partials      186      183       -3     
Flag Coverage Δ
app 47.87% <75.09%> (+0.23%) ⬆️

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

Files with missing lines Coverage Δ
...ickTransferFlow/Aspirate/AspirateSettingDetail.tsx 80.64% <100.00%> (+0.64%) ⬆️
...ickTransferFlow/Dispense/DispenseSettingDetail.tsx 81.25% <100.00%> (+0.60%) ⬆️
...sferFlow/QuickTransferAdvancedSettings/BlowOut.tsx 85.35% <100.00%> (ø)
...ansferFlow/QuickTransferAdvancedSettings/Delay.tsx 86.20% <100.00%> (ø)
...p/src/organisms/ODD/QuickTransferFlow/constants.ts 100.00% <100.00%> (ø)
app/src/organisms/ODD/QuickTransferFlow/types.ts 100.00% <ø> (ø)
...erFlow/Dispense/hooks/useDispenseSettingsConfig.ts 54.19% <33.33%> (-1.14%) ⬇️
...erFlow/Aspirate/hooks/useAspirateSettingsConfig.ts 1.40% <0.00%> (-0.08%) ⬇️
...pp/src/organisms/ODD/QuickTransferFlow/reducers.ts 1.23% <0.00%> (-0.07%) ⬇️
...ferFlow/QuickTransferAdvancedSettings/Submerge.tsx 83.88% <83.88%> (ø)

... and 56 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@koji koji merged commit 25a943a into edge May 2, 2025
32 checks passed
@koji koji deleted the feat_add-submerge-to-qt branch May 2, 2025 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants