Skip to content

Commit

Permalink
fix(app): fix module calibration overflowmenu bug (#13469)
Browse files Browse the repository at this point in the history
* fix(app): fix module calibration overflowmenu bug and disabled each menu button instead of overflowmenu itself
  • Loading branch information
koji authored Sep 7, 2023
1 parent 4822f86 commit fdf6e22
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,18 @@ import { formatLastCalibrated } from './utils'
import { ModuleCalibrationOverflowMenu } from './ModuleCalibrationOverflowMenu'

import type { AttachedModule } from '@opentrons/api-client'
import type { FormattedPipetteOffsetCalibration } from '..'

interface ModuleCalibrationItemsProps {
attachedModules: AttachedModule[]
updateRobotStatus: (isRobotBusy: boolean) => void
formattedPipetteOffsetCalibrations: FormattedPipetteOffsetCalibration[]
}

export function ModuleCalibrationItems({
attachedModules,
updateRobotStatus,
formattedPipetteOffsetCalibrations,
}: ModuleCalibrationItemsProps): JSX.Element {
const { t } = useTranslation('device_settings')

Expand Down Expand Up @@ -58,6 +61,9 @@ export function ModuleCalibrationItems({
}
attachedModule={attachedModule}
updateRobotStatus={updateRobotStatus}
formattedPipetteOffsetCalibrations={
formattedPipetteOffsetCalibrations
}
/>
</StyledTableCell>
</StyledTableRow>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,23 @@ import { Divider } from '../../../atoms/structure'
import { OverflowBtn } from '../../../atoms/MenuList/OverflowBtn'
import { MenuItem } from '../../../atoms/MenuList/MenuItem'
import { useMenuHandleClickOutside } from '../../../atoms/MenuList/hooks'
import {
useRunStatuses,
useAttachedPipetteCalibrations,
useAttachedPipettes,
} from '../../Devices/hooks'
import { useRunStatuses } from '../../Devices/hooks'
import { ModuleWizardFlows } from '../../ModuleWizardFlows'

import type { AttachedModule } from '../../../redux/modules/types'

import type { FormattedPipetteOffsetCalibration } from '../'
interface ModuleCalibrationOverflowMenuProps {
isCalibrated: boolean
attachedModule: AttachedModule
updateRobotStatus: (isRobotBusy: boolean) => void
formattedPipetteOffsetCalibrations: FormattedPipetteOffsetCalibration[]
}

export function ModuleCalibrationOverflowMenu({
isCalibrated,
attachedModule,
updateRobotStatus,
formattedPipetteOffsetCalibrations,
}: ModuleCalibrationOverflowMenuProps): JSX.Element {
const { t } = useTranslation(['device_settings', 'robot_calibration'])

Expand All @@ -52,15 +50,10 @@ export function ModuleCalibrationOverflowMenu({
onClickOutside: () => setShowOverflowMenu(false),
})

const attachedPipettes = useAttachedPipettes()
const missingPipettes =
attachedPipettes.left == null && attachedPipettes.right == null
const attachedPipetteCalibrations = useAttachedPipetteCalibrations()
const missingPipetteCalibrations =
attachedPipetteCalibrations?.left?.offset?.lastModified == null ||
attachedPipetteCalibrations?.right?.offset?.lastModified == null
const requiredAttachOrCalibratePipette =
missingPipettes || missingPipetteCalibrations
formattedPipetteOffsetCalibrations.length === 0 ||
(formattedPipetteOffsetCalibrations[0].lastCalibrated == null &&
formattedPipetteOffsetCalibrations[1].lastCalibrated == null)

const handleCalibration = (): void => {
setShowOverflowMenu(false)
Expand All @@ -84,7 +77,6 @@ export function ModuleCalibrationOverflowMenu({
alignSelf={ALIGN_FLEX_END}
aria-label="ModuleCalibrationOverflowMenu"
onClick={handleOverflowClick}
disabled={isRunning || requiredAttachOrCalibratePipette}
/>
{showModuleWizard ? (
<ModuleWizardFlows
Expand All @@ -107,7 +99,10 @@ export function ModuleCalibrationOverflowMenu({
right="0"
flexDirection={DIRECTION_COLUMN}
>
<MenuItem onClick={handleCalibration}>
<MenuItem
onClick={handleCalibration}
disabled={isRunning || requiredAttachOrCalibratePipette}
>
{isCalibrated ? t('recalibrate_module') : t('calibrate_module')}
</MenuItem>
{isCalibrated ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ describe('ModuleCalibrationItems', () => {
props = {
attachedModules: mockFetchModulesSuccessActionPayloadModules,
updateRobotStatus: jest.fn(),
formattedPipetteOffsetCalibrations: [],
}
mockModuleCalibrationOverflowMenu.mockReturnValue(
<div>mock ModuleCalibrationOverflowMenu</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,44 +5,39 @@ import { renderWithProviders } from '@opentrons/components'
import { i18n } from '../../../../i18n'
import { ModuleWizardFlows } from '../../../ModuleWizardFlows'
import { mockMagneticModule } from '../../../../redux/modules/__fixtures__'
import {
useRunStatuses,
useAttachedPipettes,
useAttachedPipetteCalibrations,
} from '../../../Devices/hooks'
import { mockLeftProtoPipette } from '../../../../redux/pipettes/__fixtures__'
import {
mockPipetteOffsetCalibration1,
mockPipetteOffsetCalibration2,
} from '../../../../redux/calibration/pipette-offset/__fixtures__'
import { useRunStatuses } from '../../../Devices/hooks'
import { ModuleCalibrationOverflowMenu } from '../ModuleCalibrationOverflowMenu'

import type { PipetteCalibrationsByMount } from '../../../../redux/pipettes/types'
import type { Mount } from '@opentrons/components'

jest.mock('../../../ModuleWizardFlows')
jest.mock('../../../Devices/hooks')

const mockAttachedPipetteCalibrations: PipetteCalibrationsByMount = {
left: {
offset: mockPipetteOffsetCalibration1,
const mockPipetteOffsetCalibrations = [
{
modelName: 'mockPipetteModelLeft',
serialNumber: '1234567',
mount: 'left' as Mount,
tiprack: 'mockTiprackLeft',
lastCalibrated: '2022-11-10T18:14:01',
markedBad: false,
},
right: {
offset: mockPipetteOffsetCalibration2,
{
modelName: 'mockPipetteModelRight',
serialNumber: '01234567',
mount: 'right' as Mount,
tiprack: 'mockTiprackRight',
lastCalibrated: '2022-11-10T18:15:02',
markedBad: false,
},
} as any
]

const mockModuleWizardFlows = ModuleWizardFlows as jest.MockedFunction<
typeof ModuleWizardFlows
>
const mockUseRunStatuses = useRunStatuses as jest.MockedFunction<
typeof useRunStatuses
>
const mockUseAttachedPipettes = useAttachedPipettes as jest.MockedFunction<
typeof useAttachedPipettes
>
const mockUseAttachedPipetteCalibrations = useAttachedPipetteCalibrations as jest.MockedFunction<
typeof useAttachedPipetteCalibrations
>

const render = (
props: React.ComponentProps<typeof ModuleCalibrationOverflowMenu>
Expand All @@ -60,21 +55,19 @@ describe('ModuleCalibrationOverflowMenu', () => {
isCalibrated: false,
attachedModule: mockMagneticModule,
updateRobotStatus: jest.fn(),
formattedPipetteOffsetCalibrations: mockPipetteOffsetCalibrations,
}
mockModuleWizardFlows.mockReturnValue(<div>module wizard flows</div>)
mockUseAttachedPipettes.mockReturnValue({
left: mockLeftProtoPipette,
right: null,
})
mockUseRunStatuses.mockReturnValue({
isRunRunning: false,
isRunStill: false,
isRunIdle: false,
isRunTerminal: false,
})
mockUseAttachedPipetteCalibrations.mockReturnValue(
mockAttachedPipetteCalibrations
)
})

afterEach(() => {
jest.clearAllMocks()
})

it('should render overflow menu buttons - not calibrated', () => {
Expand All @@ -100,21 +93,18 @@ describe('ModuleCalibrationOverflowMenu', () => {
})

it('should be disabled when not calibrated module and pipette is not attached', () => {
mockUseAttachedPipettes.mockReturnValue({
left: null,
right: null,
})
const [{ getByLabelText }] = render(props)
expect(getByLabelText('ModuleCalibrationOverflowMenu')).toBeDisabled()
props.formattedPipetteOffsetCalibrations = [] as any
const [{ getByText, getByLabelText }] = render(props)
getByLabelText('ModuleCalibrationOverflowMenu').click()
expect(getByText('Calibrate module')).toBeDisabled()
})

it('should be disabled when not calibrated module and pipette is not calibrated', () => {
mockUseAttachedPipetteCalibrations.mockReturnValue({
left: null,
right: null,
} as any)
const [{ getByLabelText }] = render(props)
expect(getByLabelText('ModuleCalibrationOverflowMenu')).toBeDisabled()
props.formattedPipetteOffsetCalibrations[0].lastCalibrated = undefined
props.formattedPipetteOffsetCalibrations[1].lastCalibrated = undefined
const [{ getByText, getByLabelText }] = render(props)
getByLabelText('ModuleCalibrationOverflowMenu').click()
expect(getByText('Calibrate module')).toBeDisabled()
})

it('should be disabled when running', () => {
Expand All @@ -124,7 +114,8 @@ describe('ModuleCalibrationOverflowMenu', () => {
isRunIdle: false,
isRunTerminal: false,
})
const [{ getByLabelText }] = render(props)
expect(getByLabelText('ModuleCalibrationOverflowMenu')).toBeDisabled()
const [{ getByText, getByLabelText }] = render(props)
getByLabelText('ModuleCalibrationOverflowMenu').click()
expect(getByText('Calibrate module')).toBeDisabled()
})
})
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react'
import { when, resetAllWhenMocks } from 'jest-when'

import { renderWithProviders, Mount } from '@opentrons/components'
import { renderWithProviders } from '@opentrons/components'

import { i18n } from '../../../../i18n'
import {
Expand All @@ -17,6 +17,7 @@ import { PipetteOffsetCalibrationItems } from '../PipetteOffsetCalibrationItems'
import { OverflowMenu } from '../OverflowMenu'
import { formatLastCalibrated } from '../utils'

import type { Mount } from '@opentrons/components'
import type { AttachedPipettesByMount } from '../../../../redux/pipettes/types'

const render = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,18 @@ import { StyledText } from '../../atoms/text'
import { ModuleCalibrationItems } from './CalibrationDetails/ModuleCalibrationItems'

import type { AttachedModule } from '@opentrons/api-client'
import type { FormattedPipetteOffsetCalibration } from '.'

interface RobotSettingsModuleCalibrationProps {
attachedModules: AttachedModule[]
updateRobotStatus: (isRobotBusy: boolean) => void
formattedPipetteOffsetCalibrations: FormattedPipetteOffsetCalibration[]
}

export function RobotSettingsModuleCalibration({
attachedModules,
updateRobotStatus,
formattedPipetteOffsetCalibrations,
}: RobotSettingsModuleCalibrationProps): JSX.Element {
const { t } = useTranslation('device_settings')

Expand All @@ -38,6 +41,9 @@ export function RobotSettingsModuleCalibration({
<ModuleCalibrationItems
attachedModules={attachedModules}
updateRobotStatus={updateRobotStatus}
formattedPipetteOffsetCalibrations={
formattedPipetteOffsetCalibrations
}
/>
) : (
<StyledText as="label" marginTop={SPACING.spacing8}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ describe('RobotSettingsModuleCalibration', () => {
props = {
attachedModules: mockFetchModulesSuccessActionPayloadModules,
updateRobotStatus: jest.fn(),
formattedPipetteOffsetCalibrations: [],
}
mockModuleCalibrationItems.mockReturnValue(
<div>mock ModuleCalibrationItems</div>
Expand Down
6 changes: 6 additions & 0 deletions app/src/organisms/RobotSettingsCalibration/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ export function RobotSettingsCalibration({
: null
)

console.log('pipetteOffsetCalibrations', pipetteOffsetCalibrations)
// console.log('attachedInstruments', attachedInstruments)

const createStatus = createRequest?.status

const isJogging =
Expand Down Expand Up @@ -329,6 +332,9 @@ export function RobotSettingsCalibration({
<RobotSettingsModuleCalibration
attachedModules={attachedModules}
updateRobotStatus={updateRobotStatus}
formattedPipetteOffsetCalibrations={
formattedPipetteOffsetCalibrations
}
/>
</>
) : (
Expand Down

0 comments on commit fdf6e22

Please sign in to comment.