From a9b219e395c75c014680e161236775560991215a Mon Sep 17 00:00:00 2001 From: koji Date: Tue, 8 Oct 2024 09:22:04 -0400 Subject: [PATCH] fix(protocol-designer): fix issues in select pipette screen (#16404) * fix(protocol-designer): fix issues in select pipette screen --- .../localization/en/create_new_protocol.json | 2 +- .../localization/en/protocol_steps.json | 2 +- .../organisms/EditInstrumentsModal/index.tsx | 2 - .../__tests__/PipetteInfoItem.test.tsx | 19 +- .../src/organisms/PipetteInfoItem/index.tsx | 44 ++-- .../CreateNewProtocolWizard/SelectGripper.tsx | 4 + .../SelectPipettes.tsx | 194 +++++++++++------- .../__tests__/SelectGripper.test.tsx | 11 + .../__tests__/SelectPipettes.test.tsx | 10 + 9 files changed, 175 insertions(+), 113 deletions(-) diff --git a/protocol-designer/src/assets/localization/en/create_new_protocol.json b/protocol-designer/src/assets/localization/en/create_new_protocol.json index a10a1653685..52c03ca7b50 100644 --- a/protocol-designer/src/assets/localization/en/create_new_protocol.json +++ b/protocol-designer/src/assets/localization/en/create_new_protocol.json @@ -32,7 +32,7 @@ "show_tips": "Show incompatible tips", "slots_limit_reached": "Slots limit reached", "stagingArea": "Staging area", - "swap": "Swap pipettes", + "swap_pipettes": "Swap pipettes", "tell_us": "Tell us about your protocol", "trash_required": "A trash entity is required", "trashBin": "Trash Bin", diff --git a/protocol-designer/src/assets/localization/en/protocol_steps.json b/protocol-designer/src/assets/localization/en/protocol_steps.json index 3ab8b590dcc..3b00d2f7d5c 100644 --- a/protocol-designer/src/assets/localization/en/protocol_steps.json +++ b/protocol-designer/src/assets/localization/en/protocol_steps.json @@ -6,8 +6,8 @@ "delete": "Delete step", "dispensed": "Dispensed", "duplicate": "Duplicate step", - "engage_height": "Engage height", "edit_step": "Edit step", + "engage_height": "Engage height", "final_deck_state": "Final deck state", "from": "from", "heater_shaker_settings": "Heater-shaker settings", diff --git a/protocol-designer/src/organisms/EditInstrumentsModal/index.tsx b/protocol-designer/src/organisms/EditInstrumentsModal/index.tsx index c01e4ee859f..c11a62a678c 100644 --- a/protocol-designer/src/organisms/EditInstrumentsModal/index.tsx +++ b/protocol-designer/src/organisms/EditInstrumentsModal/index.tsx @@ -242,7 +242,6 @@ export function EditInstrumentsModal( leftInfo != null ? ( { @@ -277,7 +276,6 @@ export function EditInstrumentsModal( rightInfo != null ? ( { diff --git a/protocol-designer/src/organisms/PipetteInfoItem/__tests__/PipetteInfoItem.test.tsx b/protocol-designer/src/organisms/PipetteInfoItem/__tests__/PipetteInfoItem.test.tsx index 8a35eb66e87..5b6f51e414c 100644 --- a/protocol-designer/src/organisms/PipetteInfoItem/__tests__/PipetteInfoItem.test.tsx +++ b/protocol-designer/src/organisms/PipetteInfoItem/__tests__/PipetteInfoItem.test.tsx @@ -25,10 +25,6 @@ describe('PipetteInfoItem', () => { tiprackDefURIs: ['mockDefUri'], pipetteName: 'p1000_single', mount: 'left', - formPipettesByMount: { - left: { pipetteName: 'p1000_single' }, - right: { pipetteName: 'p50_single' }, - }, } vi.mocked(getLabwareDefsByURI).mockReturnValue({ @@ -45,4 +41,19 @@ describe('PipetteInfoItem', () => { fireEvent.click(screen.getByText('Remove')) expect(props.cleanForm).toHaveBeenCalled() }) + + it('renders pipette with edit and remove buttons right pipette', () => { + props = { + ...props, + mount: 'right', + } + render(props) + screen.getByText('P1000 Single-Channel GEN1') + screen.getByText('Right pipette') + screen.getByText('mock display name') + fireEvent.click(screen.getByText('Edit')) + expect(props.editClick).toHaveBeenCalled() + fireEvent.click(screen.getByText('Remove')) + expect(props.cleanForm).toHaveBeenCalled() + }) }) diff --git a/protocol-designer/src/organisms/PipetteInfoItem/index.tsx b/protocol-designer/src/organisms/PipetteInfoItem/index.tsx index 25c9855ea95..220b08cb823 100644 --- a/protocol-designer/src/organisms/PipetteInfoItem/index.tsx +++ b/protocol-designer/src/organisms/PipetteInfoItem/index.tsx @@ -15,7 +15,6 @@ import { getPipetteSpecsV2 } from '@opentrons/shared-data' import { BUTTON_LINK_STYLE } from '../../atoms' import { getLabwareDefsByURI } from '../../labware-defs/selectors' import type { PipetteMount, PipetteName } from '@opentrons/shared-data' -import type { FormPipettesByMount, PipetteOnDeck } from '../../step-forms' interface PipetteInfoItemProps { mount: PipetteMount @@ -23,22 +22,11 @@ interface PipetteInfoItemProps { tiprackDefURIs: string[] editClick: () => void cleanForm: () => void - formPipettesByMount?: FormPipettesByMount - pipetteOnDeck?: PipetteOnDeck[] } export function PipetteInfoItem(props: PipetteInfoItemProps): JSX.Element { - const { - mount, - pipetteName, - tiprackDefURIs, - editClick, - cleanForm, - formPipettesByMount, - pipetteOnDeck, - } = props + const { mount, pipetteName, tiprackDefURIs, editClick, cleanForm } = props const { t, i18n } = useTranslation('create_new_protocol') - const oppositeMount = mount === 'left' ? 'right' : 'left' const allLabware = useSelector(getLabwareDefsByURI) const is96Channel = pipetteName === 'p1000_96' return ( @@ -73,33 +61,31 @@ export function PipetteInfoItem(props: PipetteInfoItemProps): JSX.Element { {t('edit')} - {(formPipettesByMount != null && - formPipettesByMount[oppositeMount].pipetteName == null) || - (pipetteOnDeck != null && pipetteOnDeck.length === 1) ? null : ( - { - cleanForm() - }} - textDecoration={TYPOGRAPHY.textDecorationUnderline} - css={BUTTON_LINK_STYLE} - > - - {t('remove')} - - - )} + { + cleanForm() + }} + textDecoration={TYPOGRAPHY.textDecorationUnderline} + css={BUTTON_LINK_STYLE} + padding={SPACING.spacing4} + > + + {t('remove')} + + diff --git a/protocol-designer/src/pages/CreateNewProtocolWizard/SelectGripper.tsx b/protocol-designer/src/pages/CreateNewProtocolWizard/SelectGripper.tsx index f4067689fa4..88dc6ab031d 100644 --- a/protocol-designer/src/pages/CreateNewProtocolWizard/SelectGripper.tsx +++ b/protocol-designer/src/pages/CreateNewProtocolWizard/SelectGripper.tsx @@ -1,6 +1,8 @@ import { useState } from 'react' import { useTranslation } from 'react-i18next' import without from 'lodash/without' +import { useLocation } from 'react-router-dom' + import { Flex, SPACING, @@ -16,6 +18,7 @@ import type { WizardTileProps } from './types' export function SelectGripper(props: WizardTileProps): JSX.Element | null { const { goBack, setValue, proceed, watch } = props const { t } = useTranslation(['create_new_protocol', 'shared']) + const location = useLocation() const [gripperStatus, setGripperStatus] = useState<'yes' | 'no' | null>(null) const additionalEquipment = watch('additionalEquipment') @@ -44,6 +47,7 @@ export function SelectGripper(props: WizardTileProps): JSX.Element | null { header={t('add_gripper')} disabled={gripperStatus == null} goBack={() => { + location.state = 'gripper' goBack(1) }} proceed={handleProceed} diff --git a/protocol-designer/src/pages/CreateNewProtocolWizard/SelectPipettes.tsx b/protocol-designer/src/pages/CreateNewProtocolWizard/SelectPipettes.tsx index 7602b091c8f..b9f16f5e23c 100644 --- a/protocol-designer/src/pages/CreateNewProtocolWizard/SelectPipettes.tsx +++ b/protocol-designer/src/pages/CreateNewProtocolWizard/SelectPipettes.tsx @@ -2,6 +2,7 @@ import { useState, useEffect } from 'react' import { useTranslation } from 'react-i18next' import styled, { css } from 'styled-components' import { useDispatch, useSelector } from 'react-redux' +import { useLocation } from 'react-router-dom' import { FLEX_ROBOT_TYPE, getAllPipetteNames, @@ -57,6 +58,7 @@ const MAX_TIPRACKS_ALLOWED = 3 export function SelectPipettes(props: WizardTileProps): JSX.Element | null { const { goBack, proceed, watch, setValue } = props const { t } = useTranslation(['create_new_protocol', 'shared']) + const location = useLocation() const pipettesByMount = watch('pipettesByMount') const fields = watch('fields') const { makeSnackbar } = useKitchen() @@ -101,8 +103,21 @@ export function SelectPipettes(props: WizardTileProps): JSX.Element | null { } }, [pipetteType, pipetteGen, pipetteVolume, selectedPipetteName]) + const noPipette = + (pipettesByMount.left.pipetteName == null || + pipettesByMount.left.tiprackDefURI == null) && + (pipettesByMount.right.pipetteName == null || + pipettesByMount.right.tiprackDefURI == null) + const isDisabled = - page === 'add' && pipettesByMount[defaultMount].tiprackDefURI == null + (page === 'add' && pipettesByMount[defaultMount].tiprackDefURI == null) || + noPipette + + const targetPipetteMount = + pipettesByMount.left.pipetteName == null || + pipettesByMount.left.tiprackDefURI == null + ? 'left' + : 'right' const handleProceed = (): void => { if (!isDisabled) { @@ -113,6 +128,34 @@ export function SelectPipettes(props: WizardTileProps): JSX.Element | null { } } } + + const handleGoBack = (): void => { + if (page === 'add') { + resetFields() + setValue(`pipettesByMount.${defaultMount}.pipetteName`, undefined) + setValue(`pipettesByMount.${defaultMount}.tiprackDefURI`, undefined) + if ( + pipettesByMount.left.pipetteName != null || + pipettesByMount.left.tiprackDefURI != null || + pipettesByMount.right.pipetteName != null || + pipettesByMount.right.tiprackDefURI != null + ) { + setPage('overview') + } else { + goBack(1) + } + } + if (page === 'overview') { + setPage('add') + } + } + + useEffect(() => { + if (location.state === 'gripper') { + setPage('overview') + } + }, [location]) + return ( <> {showIncompatibleTip ? ( @@ -129,17 +172,7 @@ export function SelectPipettes(props: WizardTileProps): JSX.Element | null { subHeader={page === 'add' ? t('which_pipette') : undefined} proceed={handleProceed} goBack={() => { - if (page === 'add') { - resetFields() - setValue(`pipettesByMount.${defaultMount}.pipetteName`, undefined) - setValue( - `pipettesByMount.${defaultMount}.tiprackDefURI`, - undefined - ) - goBack(1) - } else { - setPage('add') - } + handleGoBack() }} disabled={isDisabled} > @@ -375,7 +408,11 @@ export function SelectPipettes(props: WizardTileProps): JSX.Element | null { {t('your_pipettes')} - {has96Channel ? null : ( + {has96Channel || + (pipettesByMount.left.pipetteName == null && + pipettesByMount.right.pipetteName == null) || + (pipettesByMount.left.tiprackDefURI == null && + pipettesByMount.right.tiprackDefURI == null) ? null : ( { @@ -411,76 +448,81 @@ export function SelectPipettes(props: WizardTileProps): JSX.Element | null { transform="rotate(90deg)" /> - {t('swap')} + {t('swap_pipettes')} )} - {pipettesByMount.left.pipetteName != null && - pipettesByMount.left.tiprackDefURI != null ? ( - { - setPage('add') - setMount('left') - }} - cleanForm={() => { - setValue(`pipettesByMount.left.pipetteName`, undefined) - setValue(`pipettesByMount.left.tiprackDefURI`, undefined) + + {pipettesByMount.left.pipetteName != null && + pipettesByMount.left.tiprackDefURI != null ? ( + { + setPage('add') + setMount('left') + }} + cleanForm={() => { + setValue(`pipettesByMount.left.pipetteName`, undefined) + setValue( + `pipettesByMount.left.tiprackDefURI`, + undefined + ) - resetFields() - }} - /> - ) : ( - { - setPage('add') - setMount('left') - resetFields() - }} - text={t('add_pipette')} - textAlignment="left" - iconName="plus" - /> - )} - {pipettesByMount.right.pipetteName != null && - pipettesByMount.right.tiprackDefURI != null ? ( - { - setPage('add') - setMount('right') - }} - cleanForm={() => { - setValue(`pipettesByMount.right.pipetteName`, undefined) - setValue(`pipettesByMount.right.tiprackDefURI`, undefined) - resetFields() - }} - /> - ) : has96Channel ? null : ( - { - setPage('add') - setMount('right') - resetFields() - }} - text={t('add_pipette')} - textAlignment="left" - iconName="plus" - /> - )} + resetFields() + }} + /> + ) : null} + {pipettesByMount.right.pipetteName != null && + pipettesByMount.right.tiprackDefURI != null ? ( + { + setPage('add') + setMount('right') + }} + cleanForm={() => { + setValue(`pipettesByMount.right.pipetteName`, undefined) + setValue( + `pipettesByMount.right.tiprackDefURI`, + undefined + ) + resetFields() + }} + /> + ) : null} + + <> + {has96Channel || + (pipettesByMount.left.pipetteName != null && + pipettesByMount.right.pipetteName != null && + pipettesByMount.left.tiprackDefURI != null && + pipettesByMount.right.tiprackDefURI != null) ? null : ( + { + setPage('add') + setMount(targetPipetteMount) + resetFields() + }} + text={t('add_pipette')} + textAlignment="left" + iconName="plus" + /> + )} + )} diff --git a/protocol-designer/src/pages/CreateNewProtocolWizard/__tests__/SelectGripper.test.tsx b/protocol-designer/src/pages/CreateNewProtocolWizard/__tests__/SelectGripper.test.tsx index d6c25eef7d1..87ab1bb07e3 100644 --- a/protocol-designer/src/pages/CreateNewProtocolWizard/__tests__/SelectGripper.test.tsx +++ b/protocol-designer/src/pages/CreateNewProtocolWizard/__tests__/SelectGripper.test.tsx @@ -7,8 +7,19 @@ import { i18n } from '../../../assets/localization' import { renderWithProviders } from '../../../__testing-utils__' import { SelectGripper } from '../SelectGripper' +import type { NavigateFunction } from 'react-router-dom' import type { WizardFormState, WizardTileProps } from '../types' +const mockLocation = vi.fn() + +vi.mock('react-router-dom', async importOriginal => { + const actual = await importOriginal() + return { + ...actual, + useLocation: () => mockLocation, + } +}) + const render = (props: React.ComponentProps) => { return renderWithProviders(, { i18nInstance: i18n, diff --git a/protocol-designer/src/pages/CreateNewProtocolWizard/__tests__/SelectPipettes.test.tsx b/protocol-designer/src/pages/CreateNewProtocolWizard/__tests__/SelectPipettes.test.tsx index 6426b9ee083..922c48a0959 100644 --- a/protocol-designer/src/pages/CreateNewProtocolWizard/__tests__/SelectPipettes.test.tsx +++ b/protocol-designer/src/pages/CreateNewProtocolWizard/__tests__/SelectPipettes.test.tsx @@ -12,6 +12,7 @@ import { createCustomTiprackDef } from '../../../labware-defs/actions' import { SelectPipettes } from '../SelectPipettes' import { getTiprackOptions } from '../utils' +import type { NavigateFunction } from 'react-router-dom' import type { WizardFormState, WizardTileProps } from '../types' vi.mock('../../../labware-defs/selectors') @@ -19,6 +20,15 @@ vi.mock('../../../feature-flags/selectors') vi.mock('../../../organisms') vi.mock('../../../labware-defs/actions') vi.mock('../utils') +const mockLocation = vi.fn() + +vi.mock('react-router-dom', async importOriginal => { + const actual = await importOriginal() + return { + ...actual, + useLocation: () => mockLocation, + } +}) const render = (props: React.ComponentProps) => { return renderWithProviders(, {