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

fix: Opentrons ai client create protocol fixes #16802

Merged
merged 3 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"opentrons_ot2_label": "Opentrons OT-2",
"opentrons_ot2": "Opentrons OT-2",
"instruments_pipettes_title": "What pipettes would you like to use?",
"two_pipettes_label": "Two pipettes",
"two_pipettes_label": "1-Channel or 8-Channel pipettes",
"right_pipette_label": "Right mount",
"left_pipette_label": "Left mount",
"choose_pipette_placeholder": "Choose pipette",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,13 @@ export function ControlledAddTextAreaFields({
<Link
role="button"
onClick={() => {
field.onChange(values.filter((_, i) => i !== index))
const newValues = values
.filter((_, i) => i !== index)
.map(
(value, i) =>
`${t(name)} ${i + 1}: ${value.split(': ')[1]}`
)
field.onChange(newValues)
}}
css={css`
justify-content: flex-end;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ export function ExitConfirmModal(): JSX.Element {
return (
<Modal type="info" title={t('exit_confirmation_title')} marginLeft="0">
<Flex flexDirection={DIRECTION_COLUMN}>
<StyledText padding={`${SPACING.spacing24} 0`}>
<StyledText
paddingTop={`${SPACING.spacing8}`}
paddingBottom={`${SPACING.spacing24}`}
>
{t('exit_confirmation_body')}
</StyledText>
<Flex justifyContent={JUSTIFY_FLEX_END} gap={SPACING.spacing8}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ const TestFormProviderComponent = () => {
return (
<FormProvider {...methods}>
<ApplicationSection />

<p>{`form is ${methods.formState.isValid ? 'valid' : 'invalid'}`}</p>
</FormProvider>
)
}
Expand All @@ -33,7 +35,6 @@ describe('ApplicationSection', () => {
expect(
screen.getByText('Describe what you are trying to do')
).toBeInTheDocument()
expect(screen.getByText('Confirm')).toBeInTheDocument()
})

it('should not render other application dropdown if Other option is not selected', () => {
Expand All @@ -54,10 +55,10 @@ describe('ApplicationSection', () => {
expect(screen.getByText('Other application')).toBeInTheDocument()
})

it('should enable confirm button when all fields are filled', async () => {
it('should update the form state to valid when all fields are filled', async () => {
render()

expect(screen.getByRole('button', { name: 'Confirm' })).toBeDisabled()
expect(screen.getByText('form is invalid')).toBeInTheDocument()

const applicationDropdown = screen.getByText('Select an option')
fireEvent.click(applicationDropdown)
Expand All @@ -69,14 +70,13 @@ describe('ApplicationSection', () => {
fireEvent.change(describeInput, { target: { value: 'Test description' } })

await waitFor(() => {
expect(screen.getByRole('button', { name: 'Confirm' })).toBeEnabled()
expect(screen.getByText('form is valid')).toBeInTheDocument()
})
})

it('should disable confirm button when all fields are not filled', () => {
it('should update the form state to invalid when not all fields are filled', () => {
render()

const confirmButton = screen.getByRole('button')
expect(confirmButton).toBeDisabled()
expect(screen.getByText('form is invalid')).toBeInTheDocument()
})
})
42 changes: 2 additions & 40 deletions opentrons-ai-client/src/organisms/ApplicationSection/index.tsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,8 @@
import {
DIRECTION_COLUMN,
DISPLAY_FLEX,
Flex,
JUSTIFY_FLEX_END,
LargeButton,
SPACING,
} from '@opentrons/components'
import { DIRECTION_COLUMN, Flex, SPACING } from '@opentrons/components'
import { useFormContext } from 'react-hook-form'
import { useTranslation } from 'react-i18next'
import styled from 'styled-components'
import { ControlledDropdownMenu } from '../../atoms/ControlledDropdownMenu'
import { ControlledInputField } from '../../atoms/ControlledInputField'
import { useAtom } from 'jotai'
import { createProtocolAtom } from '../../resources/atoms'
import { APPLICATION_STEP } from '../ProtocolSectionsContainer'

export const BASIC_ALIQUOTING = 'basic_aliquoting'
export const PCR = 'pcr'
Expand All @@ -25,11 +14,7 @@ export const APPLICATION_DESCRIBE = 'application.description'

export function ApplicationSection(): JSX.Element | null {
const { t } = useTranslation('create_protocol')
const {
watch,
formState: { isValid },
} = useFormContext()
const [{ currentStep }, setCreateProtocolAtom] = useAtom(createProtocolAtom)
const { watch } = useFormContext()

const options = [
{ name: t(BASIC_ALIQUOTING), value: BASIC_ALIQUOTING },
Expand All @@ -39,16 +24,6 @@ export function ApplicationSection(): JSX.Element | null {

const isOtherSelected = watch(APPLICATION_SCIENTIFIC_APPLICATION) === OTHER

function handleConfirmButtonClick(): void {
const step =
currentStep > APPLICATION_STEP ? currentStep : APPLICATION_STEP + 1

setCreateProtocolAtom({
currentStep: step,
focusStep: step,
})
}

return (
<Flex
flexDirection={DIRECTION_COLUMN}
Expand Down Expand Up @@ -80,19 +55,6 @@ export function ApplicationSection(): JSX.Element | null {
caption={t('application_describe_caption')}
rules={{ required: true, minLength: 3 }}
/>

<ButtonContainer>
<LargeButton
onClick={handleConfirmButtonClick}
disabled={!isValid}
buttonText={t('section_confirm_button')}
></LargeButton>
</ButtonContainer>
</Flex>
)
}

const ButtonContainer = styled.div`
display: ${DISPLAY_FLEX};
justify-content: ${JUSTIFY_FLEX_END};
`
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ const TestFormProviderComponent = () => {
return (
<FormProvider {...methods}>
<InstrumentsSection />

<p>{`form is ${methods.formState.isValid ? 'valid' : 'invalid'}`}</p>
</FormProvider>
)
}
Expand All @@ -24,7 +26,7 @@ const render = (): ReturnType<typeof renderWithProviders> => {
}

describe('ApplicationSection', () => {
it('should render robot, pipette, flex gripper radios, mounts dropdowns, and confirm button', async () => {
it('should render robot, pipette, flex gripper radios and mounts dropdowns', async () => {
render()

expect(
Expand All @@ -40,7 +42,6 @@ describe('ApplicationSection', () => {
expect(
screen.getByText('Do you want to use the Flex Gripper?')
).toBeInTheDocument()
expect(screen.getByText('Confirm')).toBeInTheDocument()
})

it('should not render left and right mount dropdowns if 96-Channel 1000µL pipette radio is selected', () => {
Expand Down Expand Up @@ -80,7 +81,7 @@ describe('ApplicationSection', () => {
render()

await waitFor(() => {
expect(screen.getByRole('button', { name: 'Confirm' })).toBeDisabled()
expect(screen.getByText('form is invalid')).toBeInTheDocument()
})

const leftMount = screen.getAllByText('Choose pipette')[0]
Expand All @@ -96,14 +97,14 @@ describe('ApplicationSection', () => {
})
expect(screen.getByText('None')).toBeInTheDocument()

expect(screen.getByRole('button', { name: 'Confirm' })).toBeEnabled()
expect(screen.getByText('form is valid')).toBeInTheDocument()
})

it('should not be able to select two pipettes with none value', async () => {
render()

await waitFor(() => {
expect(screen.getByRole('button', { name: 'Confirm' })).toBeDisabled()
expect(screen.getByText('form is invalid')).toBeInTheDocument()
})

const leftMount = screen.getAllByText('Choose pipette')[0]
Expand All @@ -115,15 +116,15 @@ describe('ApplicationSection', () => {
fireEvent.click(screen.getAllByText('None')[1])

await waitFor(() => {
expect(screen.getByRole('button', { name: 'Confirm' })).toBeDisabled()
expect(screen.getByText('form is invalid')).toBeInTheDocument()
})
})

it('should enable confirm button when all fields are filled', async () => {
it('should update the form state to valid when all fields are filled', async () => {
render()

await waitFor(() => {
expect(screen.getByRole('button', { name: 'Confirm' })).toBeDisabled()
expect(screen.getByText('form is invalid')).toBeInTheDocument()
})

const leftMount = screen.getAllByText('Choose pipette')[0]
Expand All @@ -135,16 +136,7 @@ describe('ApplicationSection', () => {
fireEvent.click(screen.getByText('Flex 8-Channel 50 μL'))

await waitFor(() => {
expect(screen.getByRole('button', { name: 'Confirm' })).toBeEnabled()
})
})

it('should disable confirm button when all fields are not filled', async () => {
render()

const confirmButton = screen.getByRole('button')
await waitFor(() => {
expect(confirmButton).not.toBeEnabled()
expect(screen.getByText('form is valid')).toBeInTheDocument()
})
})
})
38 changes: 2 additions & 36 deletions opentrons-ai-client/src/organisms/InstrumentsSection/index.tsx
Original file line number Diff line number Diff line change
@@ -1,27 +1,20 @@
import {
COLORS,
DIRECTION_COLUMN,
DISPLAY_FLEX,
Flex,
JUSTIFY_FLEX_END,
LargeButton,
SPACING,
StyledText,
} from '@opentrons/components'
import { useFormContext } from 'react-hook-form'
import { useTranslation } from 'react-i18next'
import styled from 'styled-components'
import { useAtom } from 'jotai'
import { createProtocolAtom } from '../../resources/atoms'
import { INSTRUMENTS_STEP } from '../ProtocolSectionsContainer'
import { ControlledDropdownMenu } from '../../atoms/ControlledDropdownMenu'
import { ControlledRadioButtonGroup } from '../../molecules/ControlledRadioButtonGroup'
import { useMemo } from 'react'
import {
getAllPipetteNames,
getPipetteSpecsV2,
OT2_PIPETTES,
OT2_ROBOT_TYPE,
OT3_PIPETTES,
} from '@opentrons/shared-data'

Expand All @@ -40,11 +33,7 @@ export const NO_PIPETTES = 'none'

export function InstrumentsSection(): JSX.Element | null {
const { t } = useTranslation('create_protocol')
const {
formState: { isValid },
watch,
} = useFormContext()
const [{ currentStep }, setCreateProtocolAtom] = useAtom(createProtocolAtom)
const { watch } = useFormContext()
const robotType = watch(ROBOT_FIELD_NAME)
const isOtherPipettesSelected = watch(PIPETTES_FIELD_NAME) === TWO_PIPETTES
const isOpentronsOT2Selected = robotType === OPENTRONS_OT2
Expand Down Expand Up @@ -91,7 +80,7 @@ export function InstrumentsSection(): JSX.Element | null {
const pipetteOptions = useMemo(() => {
const allPipetteOptions = getAllPipetteNames('maxVolume', 'channels')
.filter(name =>
(robotType === OT2_ROBOT_TYPE ? OT2_PIPETTES : OT3_PIPETTES).includes(
(robotType === OPENTRONS_OT2 ? OT2_PIPETTES : OT3_PIPETTES).includes(
name
)
)
Expand All @@ -103,16 +92,6 @@ export function InstrumentsSection(): JSX.Element | null {
return [{ name: t('none'), value: NO_PIPETTES }, ...allPipetteOptions]
}, [robotType])

function handleConfirmButtonClick(): void {
const step =
currentStep > INSTRUMENTS_STEP ? currentStep : INSTRUMENTS_STEP + 1

setCreateProtocolAtom({
currentStep: step,
focusStep: step,
})
}

return (
<Flex
flexDirection={DIRECTION_COLUMN}
Expand Down Expand Up @@ -186,23 +165,10 @@ export function InstrumentsSection(): JSX.Element | null {
rules={{ required: true }}
/>
)}

<ButtonContainer>
<LargeButton
onClick={handleConfirmButtonClick}
disabled={!isValid}
buttonText={t('section_confirm_button')}
></LargeButton>
</ButtonContainer>
</Flex>
)
}

const ButtonContainer = styled.div`
display: ${DISPLAY_FLEX};
justify-content: ${JUSTIFY_FLEX_END};
`

const PipettesDropdown = styled.div<{ isOpentronsOT2Selected?: boolean }>`
display: flex;
flex-direction: column;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { fireEvent, screen } from '@testing-library/react'
import { fireEvent, screen, waitFor } from '@testing-library/react'
import { describe, it, expect } from 'vitest'
import { renderWithProviders } from '../../../__testing-utils__'
import { i18n } from '../../../i18n'
Expand All @@ -15,6 +15,8 @@ const TestFormProviderComponent = () => {
return (
<FormProvider {...methods}>
<LabwareLiquidsSection />

<p>{`form is ${methods.formState.isValid ? 'valid' : 'invalid'}`}</p>
</FormProvider>
)
}
Expand All @@ -31,7 +33,6 @@ describe('LabwareLiquidsSection', () => {

expect(screen.getByText('Add Opentrons labware')).toBeInTheDocument()
expect(screen.getByText('No labware added yet')).toBeInTheDocument()
expect(screen.getByText('Confirm')).toBeInTheDocument()
})

it('should not display the no labware added message if labwares have been added', async () => {
Expand All @@ -51,20 +52,27 @@ describe('LabwareLiquidsSection', () => {
expect(screen.queryByText('No labware added yet')).not.toBeInTheDocument()
})

// it('should enable the confirm button when labwares have been added', async () => {
// render()
it('should update form state to valid when labwares and liquids have been added', async () => {
render()

// expect(screen.getByText('Confirm')).toBeDisabled()
await waitFor(() => {
expect(screen.getByText('form is invalid')).toBeInTheDocument()
})
const addButton = screen.getByText('Add Opentrons labware')
fireEvent.click(addButton)

// const addButton = screen.getByText('Add Opentrons labware')
// fireEvent.click(addButton)
fireEvent.click(screen.getByText('Tip rack'))
fireEvent.click(
await screen.findByText('Opentrons Flex 96 Tip Rack 1000 µL')
)
fireEvent.click(screen.getByText('Save'))

// fireEvent.click(screen.getByText('Tip rack'))
// fireEvent.click(
// await screen.findByText('Opentrons Flex 96 Tip Rack 1000 µL')
// )
// fireEvent.click(screen.getByText('Save'))
fireEvent.change(screen.getByRole('textbox'), {
target: { value: 'test liquid' },
})

// expect(screen.getByText('Confirm')).toBeEnabled()
// })
await waitFor(() => {
expect(screen.getByText('form is valid')).toBeInTheDocument()
})
})
})
Loading
Loading