Skip to content

Commit 32a5e92

Browse files
kojikoji
authored
fix(app): update delay screen to align with the design (#18878)
* fix(app): update delay screen to align with the design
1 parent 739a912 commit 32a5e92

File tree

7 files changed

+161
-35
lines changed

7 files changed

+161
-35
lines changed

‎app/src/assets/localization/en/quick_transfer.json

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
"air_gap_volume_µL": "Air gap volume (µL)",
1414
"air_gap": "Air gap",
1515
"all": "All labware",
16-
"always": "Before every aspirate",
16+
"always": "Always",
1717
"apply_predefined_settings": "Apply predefined settings for the type of liquid used in your transfer",
1818
"aspirate_flow_rate_µL": "Aspirate flow rate (µL/s)",
1919
"aspirate_flow_rate": "Aspirate flow rate",
@@ -55,6 +55,8 @@
5555
"default": "Default",
5656
"delay_after_aspirating": "Delay after aspirating",
5757
"delay_before_dispensing": "Delay before dispensing",
58+
"delay_description_aspirate": "Delay after each aspiration and air gap",
59+
"delay_description_dispense": "Delay after each dispense",
5860
"delay_duration_s": "Delay duration (seconds)",
5961
"delay_value": "{{delay}}s",
6062
"delay": "Delay",
@@ -101,15 +103,16 @@
101103
"mix_volume_µL": "Mix volume (µL)",
102104
"mix": "Mix",
103105
"name_your_transfer": "Name your quick transfer",
106+
"never": "Never",
104107
"none_to_show": "No quick transfers to show!",
105108
"number_wells_selected_error_learn_more": "Quick transfers with multiple source {{selectionUnits}} can either be one-to-one (select {{wellCount}} destination {{selectionUnits}} for this transfer) or consolidate (select 1 destination {{selectionUnit}}).",
106109
"number_wells_selected_error_message": "Select 1 or {{wellCount}} {{selectionUnits}} to make this transfer.",
107-
"once": "Once at the start of the transfer",
110+
"once": "Once",
108111
"option_disabled": "Disabled",
109112
"option_enabled": "Enabled",
110113
"overview": "Overview",
111-
"perDest": "Per destination well",
112-
"perSource": "Per source well",
114+
"perDest": "Per destination",
115+
"perSource": "Per source",
113116
"pin_transfer": "Pin quick transfer",
114117
"pinned_transfer": "Pinned quick transfer",
115118
"pinned_transfers": "Pinned Quick Transfers",

‎app/src/organisms/ODD/QuickTransferFlow/QuickTransferAdvancedSettings/Delay.tsx

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
POSITION_FIXED,
1212
RadioButton,
1313
SPACING,
14+
StyledText,
1415
} from '@opentrons/components'
1516

1617
import { getTopPortalEl } from '/app/App/portal'
@@ -120,7 +121,7 @@ export function Delay(props: DelayProps): JSX.Element {
120121
delayIsEnabled && currentStep < 2 ? t('shared:continue') : t('shared:save')
121122

122123
// allow a maximum of 10 digits for delay duration
123-
const durationRange = { min: 1, max: 9999999999 }
124+
const durationRange = { min: 0.1, max: 9999999999 }
124125
const durationError =
125126
delayDuration != null &&
126127
(delayDuration < durationRange.min || delayDuration > durationRange.max)
@@ -153,19 +154,26 @@ export function Delay(props: DelayProps): JSX.Element {
153154
marginTop={SPACING.spacing120}
154155
flexDirection={DIRECTION_COLUMN}
155156
padding={`${SPACING.spacing16} ${SPACING.spacing60} ${SPACING.spacing40} ${SPACING.spacing60}`}
156-
gridGap={SPACING.spacing4}
157+
gridGap={SPACING.spacing24}
157158
width="100%"
158159
>
159-
{delayEnabledDisplayItems.map(displayItem => (
160-
<RadioButton
161-
key={displayItem.description}
162-
isSelected={delayIsEnabled === displayItem.option}
163-
onChange={displayItem.onClick}
164-
buttonValue={displayItem.description}
165-
buttonLabel={displayItem.description}
166-
radioButtonType="large"
167-
/>
168-
))}
160+
<StyledText oddStyle="level4HeaderRegular">
161+
{kind === 'aspirate'
162+
? t('delay_description_aspirate')
163+
: t('delay_description_dispense')}
164+
</StyledText>
165+
<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing8}>
166+
{delayEnabledDisplayItems.map(displayItem => (
167+
<RadioButton
168+
key={displayItem.description}
169+
isSelected={delayIsEnabled === displayItem.option}
170+
onChange={displayItem.onClick}
171+
buttonValue={displayItem.description}
172+
buttonLabel={displayItem.description}
173+
radioButtonType="large"
174+
/>
175+
))}
176+
</Flex>
169177
</Flex>
170178
) : null}
171179
{currentStep === 2 ? (
@@ -202,6 +210,7 @@ export function Delay(props: DelayProps): JSX.Element {
202210
<NumericalKeyboard
203211
keyboardRef={keyboardRef}
204212
initialValue={String(delayDuration ?? '')}
213+
isDecimal
205214
onChange={e => {
206215
setDelayDuration(Number(e))
207216
}}

‎app/src/organisms/ODD/QuickTransferFlow/QuickTransferAdvancedSettings/__tests__/Delay.test.tsx

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
import { fireEvent, screen } from '@testing-library/react'
2+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
3+
4+
import { renderWithProviders } from '/app/__testing-utils__'
5+
import { i18n } from '/app/i18n'
6+
import { useTrackEventWithRobotSerial } from '/app/redux-resources/analytics'
7+
8+
import QuickTransferState from '../__fixtures__/QuickTransferState.json'
9+
import { Delay } from '../Delay'
10+
11+
import type { ComponentProps } from 'react'
12+
13+
vi.mock('/app/redux-resources/analytics')
14+
15+
const render = (props: ComponentProps<typeof Delay>) => {
16+
return renderWithProviders(<Delay {...props} />, {
17+
i18nInstance: i18n,
18+
})
19+
}
20+
let mockTrackEventWithRobotSerial: any
21+
22+
describe('Delay', () => {
23+
let props: ComponentProps<typeof Delay>
24+
25+
beforeEach(() => {
26+
props = {
27+
onBack: vi.fn(),
28+
state: QuickTransferState as any,
29+
dispatch: vi.fn(),
30+
kind: 'aspirate',
31+
}
32+
mockTrackEventWithRobotSerial = vi.fn(
33+
() => new Promise(resolve => resolve({}))
34+
)
35+
vi.mocked(useTrackEventWithRobotSerial).mockReturnValue({
36+
trackEventWithRobotSerial: mockTrackEventWithRobotSerial,
37+
})
38+
})
39+
40+
afterEach(() => {
41+
vi.resetAllMocks()
42+
})
43+
44+
it('renders text, buttons for delay aspirate', () => {
45+
render(props)
46+
screen.getByText('Delay after aspirating')
47+
screen.getByText('Save')
48+
screen.getByText('Delay after each aspiration and air gap')
49+
screen.getByText('Enabled')
50+
screen.getByText('Disabled')
51+
fireEvent.click(screen.getByText('Enabled'))
52+
screen.getByText('Continue')
53+
})
54+
55+
it('renders text, buttons for delay dispense', () => {
56+
props.kind = 'dispense'
57+
render(props)
58+
screen.getByText('Delay before dispensing')
59+
screen.getByText('Save')
60+
screen.getByText('Delay after each dispense')
61+
screen.getByText('Enabled')
62+
screen.getByText('Disabled')
63+
fireEvent.click(screen.getByText('Enabled'))
64+
screen.getByText('Continue')
65+
})
66+
67+
it('renders text, buttons, input field, and keyboard for delay before aspirating - volume', () => {
68+
render(props)
69+
fireEvent.click(screen.getByText('Enabled'))
70+
fireEvent.click(screen.getByText('Continue'))
71+
screen.getByText('Delay duration (seconds)')
72+
screen.getByRole('button', { name: '1' })
73+
screen.getByRole('button', { name: '5' })
74+
screen.getByRole('button', { name: '9' })
75+
screen.getByRole('button', { name: '.' })
76+
screen.getByRole('button', { name: 'del' })
77+
fireEvent.click(screen.getByRole('button', { name: '0' }))
78+
screen.getByText('Value must be between 0.1 to 9999999999')
79+
screen.getByText('Save')
80+
})
81+
82+
it('should call dispatch when clicking save button', () => {
83+
render(props)
84+
fireEvent.click(screen.getByText('Enabled'))
85+
fireEvent.click(screen.getByText('Continue'))
86+
screen.getByText('Delay duration (seconds)')
87+
screen.getByRole('button', { name: '1' })
88+
screen.getByRole('button', { name: '5' })
89+
screen.getByRole('button', { name: '9' })
90+
screen.getByRole('button', { name: '.' })
91+
screen.getByRole('button', { name: 'del' })
92+
fireEvent.click(screen.getByRole('button', { name: '0' }))
93+
fireEvent.click(screen.getByRole('button', { name: '.' }))
94+
fireEvent.click(screen.getByRole('button', { name: '2' }))
95+
fireEvent.click(screen.getByText('Save'))
96+
expect(props.dispatch).toHaveBeenCalledWith({
97+
type: 'SET_DELAY_ASPIRATE',
98+
delaySettings: {
99+
delayDuration: 0.2,
100+
},
101+
})
102+
expect(mockTrackEventWithRobotSerial).toHaveBeenCalledWith({
103+
name: 'quickTransferSettingSaved',
104+
properties: {
105+
setting: 'Delay_aspirate',
106+
},
107+
})
108+
})
109+
it('should call mock function when clicking back button', () => {
110+
render(props)
111+
fireEvent.click(screen.getByTestId('ChildNavigation_Back_Button'))
112+
expect(props.onBack).toHaveBeenCalled()
113+
})
114+
})

‎app/src/organisms/ODD/QuickTransferFlow/__tests__/QuickTransferAdvancedSettings/Delay.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ describe('Delay', () => {
133133
expect(vi.mocked(InputField)).toHaveBeenCalledWith(
134134
{
135135
title: 'Delay duration (seconds)',
136-
error: 'Value must be between 1 to 9999999999',
136+
error: 'Value must be between 0.1 to 9999999999',
137137
readOnly: true,
138138
type: 'number',
139139
value: 0,

‎app/src/organisms/ODD/QuickTransferFlow/__tests__/SelectTipFrequency.test.tsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,22 +50,22 @@ describe('SelectTipFrequency', () => {
5050
screen.getByText('Select change tip frequency')
5151
screen.getByText('Exit')
5252
screen.getByText('Continue')
53-
screen.getByText('Once at the start of the transfer')
54-
screen.getByText('Per source well')
53+
screen.getByText('Once')
54+
screen.getByText('Per source')
5555
})
5656

5757
it('renders once at the start of the transfer option only', () => {
5858
props.state.sourceWells = []
5959
props.state.destinationWells = []
6060
props.state.path = 'single'
6161
render(props)
62-
screen.getByText('Once at the start of the transfer')
62+
screen.getByText('Once')
6363
})
6464

6565
it('renders once at the start of the transfer option only', () => {
6666
props.state.transferType = 'distribute'
6767
render(props)
68-
screen.getByText('Per destination well')
68+
screen.getByText('Per destination')
6969
})
7070

7171
it('should call mock function when tappin exit button', () => {
@@ -76,7 +76,7 @@ describe('SelectTipFrequency', () => {
7676

7777
it('should call mock function when tappin continue button', () => {
7878
render(props)
79-
fireEvent.click(screen.getByText('Once at the start of the transfer'))
79+
fireEvent.click(screen.getByText('Once'))
8080
fireEvent.click(screen.getByText('Continue'))
8181
expect(props.onNext).toHaveBeenCalled()
8282
expect(props.dispatch).toHaveBeenCalledWith({

‎app/src/organisms/ODD/QuickTransferFlow/__tests__/TipManagement/ChangeTip.test.tsx

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ describe('ChangeTip', () => {
5757
it('calls dispatch when you select a new option and save', () => {
5858
render(props)
5959
screen.getByText('Change tip')
60-
screen.getByText('Once at the start of the transfer')
61-
const perSource = screen.getByText('Before every aspirate')
60+
screen.getByText('Once')
61+
const perSource = screen.getByText('Always')
6262
fireEvent.click(perSource)
6363
const saveBtn = screen.getByText('Save')
6464
fireEvent.click(saveBtn)
@@ -71,23 +71,23 @@ describe('ChangeTip', () => {
7171
it('renders correct change tip options when single transfer of less than 96 wells', () => {
7272
render(props)
7373
screen.getByText('Change tip')
74-
screen.getByText('Once at the start of the transfer')
75-
screen.getByText('Before every aspirate')
76-
screen.getByText('Per source well')
74+
screen.getByText('Once')
75+
screen.getByText('Always')
76+
screen.getByText('Per source')
7777
})
7878
it('renders correct change tip options for consolidate with less than 96 wells', () => {
7979
render({ ...props, state: { ...props.state, transferType: 'consolidate' } })
8080
screen.getByText('Change tip')
81-
screen.getByText('Once at the start of the transfer')
82-
screen.getByText('Before every aspirate')
83-
screen.getByText('Per source well')
81+
screen.getByText('Once')
82+
screen.getByText('Always')
83+
screen.getByText('Per source')
8484
})
8585
it('renders correct change tip options for distribute with less than 96 wells', () => {
8686
render({ ...props, state: { ...props.state, transferType: 'distribute' } })
8787
screen.getByText('Change tip')
88-
screen.getByText('Once at the start of the transfer')
89-
screen.getByText('Before every aspirate')
90-
screen.getByText('Per destination well')
88+
screen.getByText('Once')
89+
screen.getByText('Always')
90+
screen.getByText('Per destination')
9191
})
9292
it('renders correct change tip options any transfer with more than 96 wells', () => {
9393
render({
@@ -211,6 +211,6 @@ describe('ChangeTip', () => {
211211
},
212212
})
213213
screen.getByText('Change tip')
214-
screen.getByText('Once at the start of the transfer')
214+
screen.getByText('Once')
215215
})
216216
})

‎app/src/organisms/ODD/QuickTransferFlow/__tests__/TipManagement/TipManagement.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ describe('TipManagement', () => {
4040
it('renders tip management options and their values', () => {
4141
render(props)
4242
screen.getByText('Change tip')
43-
screen.getByText('Once at the start of the transfer')
43+
screen.getByText('Once')
4444
screen.getByText('Tip drop location')
4545
screen.getByText('Trash bin')
4646
})

0 commit comments

Comments
 (0)