From 2f8a751800465c84a425b38dfd29e00110690ebd Mon Sep 17 00:00:00 2001 From: Nick Diehl <47604184+ncdiehl11@users.noreply.github.com> Date: Tue, 22 Oct 2024 20:46:39 -0400 Subject: [PATCH] feat(protocol-designer): add step names to protocol steps and disable 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 --- components/src/organisms/Toolbox/index.tsx | 2 +- .../ProtocolSteps/BatchEditToolbox/index.tsx | 1 - .../StepForm/StepFormToolbox.tsx | 1 - .../Timeline/StepOverflowMenu.tsx | 14 +++++ .../Timeline/SubstepsToolbox.tsx | 16 +++--- .../__tests__/ProtocolSteps.test.tsx | 51 +++++++++++++---- .../pages/Designer/ProtocolSteps/index.tsx | 57 ++++++++++--------- .../top-selectors/labware-locations/index.ts | 2 +- 8 files changed, 93 insertions(+), 51 deletions(-) diff --git a/components/src/organisms/Toolbox/index.tsx b/components/src/organisms/Toolbox/index.tsx index e73de42e06b..e7437e1e57f 100644 --- a/components/src/organisms/Toolbox/index.tsx +++ b/components/src/organisms/Toolbox/index.tsx @@ -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 ( { return ( {t('protocol_steps:batch_edit')} diff --git a/protocol-designer/src/pages/Designer/ProtocolSteps/StepForm/StepFormToolbox.tsx b/protocol-designer/src/pages/Designer/ProtocolSteps/StepForm/StepFormToolbox.tsx index 709cff49d7c..f101237cb45 100644 --- a/protocol-designer/src/pages/Designer/ProtocolSteps/StepForm/StepFormToolbox.tsx +++ b/protocol-designer/src/pages/Designer/ProtocolSteps/StepForm/StepFormToolbox.tsx @@ -206,7 +206,6 @@ export function StepFormToolbox(props: StepFormToolboxProps): JSX.Element { } - height="calc(100vh - 64px)" title={ diff --git a/protocol-designer/src/pages/Designer/ProtocolSteps/Timeline/StepOverflowMenu.tsx b/protocol-designer/src/pages/Designer/ProtocolSteps/Timeline/StepOverflowMenu.tsx index cf4424af5f6..3650c23120e 100644 --- a/protocol-designer/src/pages/Designer/ProtocolSteps/Timeline/StepOverflowMenu.tsx +++ b/protocol-designer/src/pages/Designer/ProtocolSteps/Timeline/StepOverflowMenu.tsx @@ -19,6 +19,8 @@ import { toggleViewSubstep, } from '../../../../ui/steps/actions/actions' import { + getBatchEditFormHasUnsavedChanges, + getCurrentFormHasUnsavedChanges, getSavedStepForms, getUnsavedForm, } from '../../../../step-forms/selectors' @@ -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>() const formData = useSelector(getUnsavedForm) const savedStepFormData = useSelector(getSavedStepForms)[stepId] @@ -93,6 +101,7 @@ export function StepOverflowMenu(props: StepOverflowMenuProps): JSX.Element { {multiSelectItemIds != null && multiSelectItemIds.length > 0 ? ( <> { duplicateMultipleSteps() setStepOverflowMenu(false) @@ -117,6 +126,7 @@ export function StepOverflowMenu(props: StepOverflowMenuProps): JSX.Element { )} {isPipetteStep || isThermocyclerProfile ? ( { setStepOverflowMenu(false) dispatch(hoverOnStep(stepId)) @@ -127,6 +137,7 @@ export function StepOverflowMenu(props: StepOverflowMenuProps): JSX.Element { ) : null} { duplicateStep(stepId) setStepOverflowMenu(false) @@ -166,5 +177,8 @@ const MenuButton = styled.button` &:disabled { color: ${COLORS.grey40}; cursor: auto; + &:hover { + background-color: ${COLORS.transparent}; + } } ` diff --git a/protocol-designer/src/pages/Designer/ProtocolSteps/Timeline/SubstepsToolbox.tsx b/protocol-designer/src/pages/Designer/ProtocolSteps/Timeline/SubstepsToolbox.tsx index 5605667bf59..922220b7e81 100644 --- a/protocol-designer/src/pages/Designer/ProtocolSteps/Timeline/SubstepsToolbox.tsx +++ b/protocol-designer/src/pages/Designer/ProtocolSteps/Timeline/SubstepsToolbox.tsx @@ -3,6 +3,7 @@ import { useDispatch, useSelector } from 'react-redux' import { FLEX_MAX_CONTENT, Flex, + Icon, PrimaryButton, SPACING, StyledText, @@ -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' || @@ -57,14 +63,10 @@ export function SubstepsToolbox( } + onCloseClick={handleClose} confirmButton={ - { - dispatch(toggleViewSubstep(null)) - dispatch(hoverOnStep(null)) - }} - width="100%" - > + {t('shared:done')} } diff --git a/protocol-designer/src/pages/Designer/ProtocolSteps/__tests__/ProtocolSteps.test.tsx b/protocol-designer/src/pages/Designer/ProtocolSteps/__tests__/ProtocolSteps.test.tsx index e5a37810a3f..1dafa1d2c71 100644 --- a/protocol-designer/src/pages/Designer/ProtocolSteps/__tests__/ProtocolSteps.test.tsx +++ b/protocol-designer/src/pages/Designer/ProtocolSteps/__tests__/ProtocolSteps.test.tsx @@ -1,15 +1,22 @@ -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') @@ -17,6 +24,7 @@ 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') @@ -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(
mock TimelineToolbox
) @@ -37,6 +62,12 @@ describe('ProtocolSteps', () => { vi.mocked(getSelectedSubstep).mockReturnValue(null) vi.mocked(SubstepsToolbox).mockReturnValue(
mock SubstepsToolbox
) 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', () => { @@ -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() @@ -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') + }) }) diff --git a/protocol-designer/src/pages/Designer/ProtocolSteps/index.tsx b/protocol-designer/src/pages/Designer/ProtocolSteps/index.tsx index 7f240f4da00..c2ca9ced4ee 100644 --- a/protocol-designer/src/pages/Designer/ProtocolSteps/index.tsx +++ b/protocol-designer/src/pages/Designer/ProtocolSteps/index.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect } from 'react' +import { useState } from 'react' import { useSelector } from 'react-redux' import { useTranslation } from 'react-i18next' import { @@ -12,6 +12,7 @@ import { JUSTIFY_SPACE_BETWEEN, POSITION_FIXED, SPACING, + StyledText, Tag, ToggleGroup, } from '@opentrons/components' @@ -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) @@ -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 = @@ -76,29 +69,35 @@ export function ProtocolSteps(): JSX.Element { - {formData == null || formType === 'moveLabware' ? ( - - { - setDeckView(leftString) - }} - rightClick={() => { - setDeckView(rightString) - }} - /> - - ) : null} + + {currentStep != null ? ( + + {i18n.format(currentStep.stepName, 'capitalize')} + + ) : null} + { + setDeckView(leftString) + }} + rightClick={() => { + setDeckView(rightString) + }} + /> + ) : null} - {selectedSubstep ? : null} + {formData == null && selectedSubstep ? ( + + ) : null} {isMultiSelectMode ? : null} diff --git a/protocol-designer/src/top-selectors/labware-locations/index.ts b/protocol-designer/src/top-selectors/labware-locations/index.ts index 7741f3a7314..df674025b74 100644 --- a/protocol-designer/src/top-selectors/labware-locations/index.ts +++ b/protocol-designer/src/top-selectors/labware-locations/index.ts @@ -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,