Skip to content

Commit

Permalink
feat(protocol-designer): add step names to protocol steps and disable…
Browse files Browse the repository at this point in the history
… overflow menu buttons (#16571)

This PR addresses two major tasks: 1) adding the step name above deck
map in ProtocolSteps component, and 2) appropriately disabling buttons
from step overflow menus when appropriate. Namely, when a step is in
editing, its overflow menu should not permit viewing details (for
transfer and thermocycler profile steps). When a step form has unsaved
changes, its overflow menu should not permit duplication. If batch edit
is open, neither steps's overflow menu should permit batch duplication.
Deletion is available in any scenario. Also, the opening of a step form
edit toolbox takes priority over step details toolbox.

In addition, I pass the `zIndex` prop for `Toolbox` only if it is of
position fixed.

Closes AUTH-878, Closes AUTH-880
  • Loading branch information
ncdiehl11 authored Oct 23, 2024
1 parent 37e81a0 commit 2f8a751
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 51 deletions.
2 changes: 1 addition & 1 deletion components/src/organisms/Toolbox/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,11 @@ export function Toolbox(props: ToolboxProps): JSX.Element {
...(side === 'left' && { left: '0' }),
...(horizontalSide === 'bottom' && { bottom: '0' }),
...(horizontalSide === 'top' && { top: '5rem' }),
zIndex: 10,
}
: {}
return (
<Flex
zIndex={10}
cursor="auto"
backgroundColor={COLORS.white}
boxShadow="0px 3px 6px rgba(0, 0, 0, 0.23)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ export const BatchEditToolbox = (): JSX.Element | null => {
return (
<Toolbox
position={POSITION_RELATIVE}
height="calc(100vh - 64px)"
title={
<StyledText desktopStyle="bodyLargeSemiBold">
{t('protocol_steps:batch_edit')}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,6 @@ export function StepFormToolbox(props: StepFormToolboxProps): JSX.Element {
</PrimaryButton>
</Flex>
}
height="calc(100vh - 64px)"
title={
<Flex gridGap={SPACING.spacing8} alignItems={ALIGN_CENTER}>
<Icon size="1rem" name={icon} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {
toggleViewSubstep,
} from '../../../../ui/steps/actions/actions'
import {
getBatchEditFormHasUnsavedChanges,
getCurrentFormHasUnsavedChanges,
getSavedStepForms,
getUnsavedForm,
} from '../../../../step-forms/selectors'
Expand Down Expand Up @@ -49,6 +51,12 @@ export function StepOverflowMenu(props: StepOverflowMenuProps): JSX.Element {
multiSelectItemIds,
} = props
const { t } = useTranslation('protocol_steps')
const singleEditFormHasUnsavedChanges = useSelector(
getCurrentFormHasUnsavedChanges
)
const batchEditFormHasUnstagedChanges = useSelector(
getBatchEditFormHasUnsavedChanges
)
const dispatch = useDispatch<ThunkDispatch<BaseState, any, any>>()
const formData = useSelector(getUnsavedForm)
const savedStepFormData = useSelector(getSavedStepForms)[stepId]
Expand Down Expand Up @@ -93,6 +101,7 @@ export function StepOverflowMenu(props: StepOverflowMenuProps): JSX.Element {
{multiSelectItemIds != null && multiSelectItemIds.length > 0 ? (
<>
<MenuButton
disabled={batchEditFormHasUnstagedChanges}
onClick={() => {
duplicateMultipleSteps()
setStepOverflowMenu(false)
Expand All @@ -117,6 +126,7 @@ export function StepOverflowMenu(props: StepOverflowMenuProps): JSX.Element {
)}
{isPipetteStep || isThermocyclerProfile ? (
<MenuButton
disabled={formData != null}
onClick={() => {
setStepOverflowMenu(false)
dispatch(hoverOnStep(stepId))
Expand All @@ -127,6 +137,7 @@ export function StepOverflowMenu(props: StepOverflowMenuProps): JSX.Element {
</MenuButton>
) : null}
<MenuButton
disabled={singleEditFormHasUnsavedChanges}
onClick={() => {
duplicateStep(stepId)
setStepOverflowMenu(false)
Expand Down Expand Up @@ -166,5 +177,8 @@ const MenuButton = styled.button`
&:disabled {
color: ${COLORS.grey40};
cursor: auto;
&:hover {
background-color: ${COLORS.transparent};
}
}
`
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { useDispatch, useSelector } from 'react-redux'
import {
FLEX_MAX_CONTENT,
Flex,
Icon,
PrimaryButton,
SPACING,
StyledText,
Expand Down Expand Up @@ -48,6 +49,11 @@ export function SubstepsToolbox(
return null
}

const handleClose = (): void => {
dispatch(toggleViewSubstep(null))
dispatch(hoverOnStep(null))
}

return ('commandCreatorFnName' in substeps &&
(substeps.commandCreatorFnName === 'transfer' ||
substeps.commandCreatorFnName === 'consolidate' ||
Expand All @@ -57,14 +63,10 @@ export function SubstepsToolbox(
<Toolbox
width={FLEX_MAX_CONTENT}
childrenPadding="0"
closeButton={<Icon size="2rem" name="close" />}
onCloseClick={handleClose}
confirmButton={
<PrimaryButton
onClick={() => {
dispatch(toggleViewSubstep(null))
dispatch(hoverOnStep(null))
}}
width="100%"
>
<PrimaryButton onClick={handleClose} width="100%">
{t('shared:done')}
</PrimaryButton>
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,30 @@
import { describe, it, vi, beforeEach, expect } from 'vitest'
import { describe, it, vi, beforeEach } from 'vitest'
import '@testing-library/jest-dom/vitest'
import { fireEvent, screen } from '@testing-library/react'
import { i18n } from '../../../../assets/localization'
import { renderWithProviders } from '../../../../__testing-utils__'
import { getUnsavedForm } from '../../../../step-forms/selectors'
import { getSelectedSubstep } from '../../../../ui/steps/selectors'
import {
getSavedStepForms,
getUnsavedForm,
} from '../../../../step-forms/selectors'
import {
getSelectedStepId,
getSelectedSubstep,
} from '../../../../ui/steps/selectors'
import { getEnableHotKeysDisplay } from '../../../../feature-flags/selectors'
import { DeckSetupContainer } from '../../DeckSetup'
import { OffDeck } from '../../Offdeck'
import { ProtocolSteps } from '..'
import { SubstepsToolbox, TimelineToolbox } from '../Timeline'
import type { SavedStepFormState } from '../../../../step-forms'

vi.mock('../../Offdeck')
vi.mock('../../../../step-forms/selectors')
vi.mock('../../../../ui/steps/selectors')
vi.mock('../../../../ui/labware/selectors')
vi.mock('../StepForm')
vi.mock('../../DeckSetup')
vi.mock('../StepSummary.tsx')
vi.mock('../Timeline')
vi.mock('../../../../feature-flags/selectors')

Expand All @@ -26,6 +34,23 @@ const render = () => {
})[0]
}

const MOCK_STEP_FORMS = {
'0522fde8-25a3-4840-b84a-af7282bd80d5': {
moduleId: '781599b2-1eff-4594-8c96-06fcd54f4faa:heaterShakerModuleType',
pauseAction: 'untilTime',
pauseHour: '22',
pauseMessage: 'sdfg',
pauseMinute: '22',
pauseSecond: '11',
pauseTemperature: null,
pauseTime: null,
id: '0522fde8-25a3-4840-b84a-af7282bd80d5',
stepType: 'pause',
stepName: 'custom pause',
stepDetails: '',
},
}

describe('ProtocolSteps', () => {
beforeEach(() => {
vi.mocked(TimelineToolbox).mockReturnValue(<div>mock TimelineToolbox</div>)
Expand All @@ -37,6 +62,12 @@ describe('ProtocolSteps', () => {
vi.mocked(getSelectedSubstep).mockReturnValue(null)
vi.mocked(SubstepsToolbox).mockReturnValue(<div>mock SubstepsToolbox</div>)
vi.mocked(getEnableHotKeysDisplay).mockReturnValue(true)
vi.mocked(getSavedStepForms).mockReturnValue(
MOCK_STEP_FORMS as SavedStepFormState
)
vi.mocked(getSelectedStepId).mockReturnValue(
'0522fde8-25a3-4840-b84a-af7282bd80d5'
)
})

it('renders each component in ProtocolSteps', () => {
Expand All @@ -52,15 +83,6 @@ describe('ProtocolSteps', () => {
screen.getByText('mock OffDeck')
})

it('renders no toggle when formData does not equal moveLabware type', () => {
vi.mocked(getUnsavedForm).mockReturnValue({
stepType: 'magnet',
id: 'mockId',
})
render()
expect(screen.queryByText('Off-deck')).not.toBeInTheDocument()
})

it('renders the substepToolbox when selectedSubstep is not null', () => {
vi.mocked(getSelectedSubstep).mockReturnValue('mockId')
render()
Expand All @@ -73,4 +95,9 @@ describe('ProtocolSteps', () => {
screen.getByText('Shift + Click to select all')
screen.getByText('Command + Click for multi-select')
})

it('renders the current step name', () => {
render()
screen.getByText('Custom pause')
})
})
57 changes: 29 additions & 28 deletions protocol-designer/src/pages/Designer/ProtocolSteps/index.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useState, useEffect } from 'react'
import { useState } from 'react'
import { useSelector } from 'react-redux'
import { useTranslation } from 'react-i18next'
import {
Expand All @@ -12,6 +12,7 @@ import {
JUSTIFY_SPACE_BETWEEN,
POSITION_FIXED,
SPACING,
StyledText,
Tag,
ToggleGroup,
} from '@opentrons/components'
Expand All @@ -34,7 +35,7 @@ import { StepSummary } from './StepSummary'
import { BatchEditToolbox } from './BatchEditToolbox'

export function ProtocolSteps(): JSX.Element {
const { t } = useTranslation('starting_deck_state')
const { i18n, t } = useTranslation('starting_deck_state')
const formData = useSelector(getUnsavedForm)
const isMultiSelectMode = useSelector(getIsMultiSelectMode)
const selectedSubstep = useSelector(getSelectedSubstep)
Expand All @@ -45,14 +46,6 @@ export function ProtocolSteps(): JSX.Element {
typeof leftString | typeof rightString
>(leftString)

const formType = formData?.stepType

useEffect(() => {
if (formData != null && formType !== 'moveLabware') {
setDeckView(leftString)
}
}, [formData, formType, deckView])

const currentHoveredStepId = useSelector(getHoveredStepId)
const currentSelectedStepId = useSelector(getSelectedStepId)
const currentstepIdForStepSummary =
Expand All @@ -76,29 +69,35 @@ export function ProtocolSteps(): JSX.Element {
<TimelineToolbox />
<Flex
alignItems={ALIGN_CENTER}
alignSelf={ALIGN_CENTER}
flexDirection={DIRECTION_COLUMN}
gridGap={SPACING.spacing16}
width="100%"
paddingBottom={enableHoyKeyDisplay ? '5rem' : '0'}
paddingTop={SPACING.spacing16}
justifyContent={JUSTIFY_FLEX_START}
>
<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing16}>
{formData == null || formType === 'moveLabware' ? (
<Flex justifyContent={JUSTIFY_FLEX_END}>
<ToggleGroup
selectedValue={deckView}
leftText={leftString}
rightText={rightString}
leftClick={() => {
setDeckView(leftString)
}}
rightClick={() => {
setDeckView(rightString)
}}
/>
</Flex>
) : null}
<Flex
justifyContent={
currentStep != null ? JUSTIFY_SPACE_BETWEEN : JUSTIFY_FLEX_END
}
>
{currentStep != null ? (
<StyledText desktopStyle="headingSmallBold">
{i18n.format(currentStep.stepName, 'capitalize')}
</StyledText>
) : null}
<ToggleGroup
selectedValue={deckView}
leftText={leftString}
rightText={rightString}
leftClick={() => {
setDeckView(leftString)
}}
rightClick={() => {
setDeckView(rightString)
}}
/>
</Flex>
<Flex
flexDirection={DIRECTION_COLUMN}
gridGap={SPACING.spacing16}
Expand Down Expand Up @@ -127,7 +126,9 @@ export function ProtocolSteps(): JSX.Element {
</Box>
) : null}
</Flex>
{selectedSubstep ? <SubstepsToolbox stepId={selectedSubstep} /> : null}
{formData == null && selectedSubstep ? (
<SubstepsToolbox stepId={selectedSubstep} />
) : null}
<StepForm />
{isMultiSelectMode ? <BatchEditToolbox /> : null}
</Flex>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ export const getUnoccupiedLabwareLocationOptions: Selector<
)
})
.map(slotId => ({ name: slotId, value: slotId }))
const offDeck = { name: 'Off-deck', value: 'offDeck' }
const offDeck = { name: 'Off-Deck', value: 'offDeck' }
const wasteChuteSlot = {
name: 'Waste Chute in D3',
value: WASTE_CHUTE_CUTOUT,
Expand Down

0 comments on commit 2f8a751

Please sign in to comment.