From d367217941def8311c3cf22fecc4ca6dcd60926a Mon Sep 17 00:00:00 2001 From: Brian Arthur Cooper Date: Thu, 9 May 2024 17:58:24 -0400 Subject: [PATCH] fix(app): address Design QA comments for modules in deck config (#15150) Fix Deck hardware table padding in desktop protocol setup. Fix padding in no modules detected modal. standardize copy for title and description fo protocol setup steps. standardize copy for header and description in ODD location conflict modal Closes [RQA-2687](https://opentrons.atlassian.net/browse/RQA-2687), Closes [RQA-2686](https://opentrons.atlassian.net/browse/RQA-2686), Closes [RQA-2684](https://opentrons.atlassian.net/browse/RQA-2684), Closes [RQA-2685](https://opentrons.atlassian.net/browse/RQA-2685), Closes [RQA-2692](https://opentrons.atlassian.net/browse/RQA-2692) --- .../localization/en/device_details.json | 3 +- .../localization/en/protocol_setup.json | 16 +++---- .../AddFixtureModal.tsx | 2 +- .../__tests__/AddFixtureModal.test.tsx | 12 +++--- .../Devices/ProtocolRun/ProtocolRunSetup.tsx | 33 +++++--------- .../ChooseModuleToConfigureModal.tsx | 36 ++++++---------- .../SetupModuleAndDeck/NotConfiguredModal.tsx | 8 +++- .../UnMatchedModuleWarning.tsx | 43 +++++++++---------- .../__tests__/LocationConflictModal.test.tsx | 2 +- .../__tests__/NotConfiguredModal.test.tsx | 4 +- .../ProtocolRun/SetupModuleAndDeck/index.tsx | 8 +--- .../__tests__/ProtocolRunSetup.test.tsx | 32 +++++++------- 12 files changed, 87 insertions(+), 112 deletions(-) diff --git a/app/src/assets/localization/en/device_details.json b/app/src/assets/localization/en/device_details.json index d3fdab0b04c..73c74021176 100644 --- a/app/src/assets/localization/en/device_details.json +++ b/app/src/assets/localization/en/device_details.json @@ -3,8 +3,7 @@ "about_module": "About {{name}}", "about_pipette_name": "About {{name}} Pipette", "about_pipette": "About pipette", - "add_fixture_description": "Add this item to your deck configuration. It will be referenced during protocol analysis.", - "add_to_slot_description": "Choose an item below to add to your deck configuration. It will be referenced during protocol analysis.", + "add_fixture_description": "Add this hardware to your deck configuration. It will be referenced during protocol analysis.", "add_to_slot": "Add to slot {{slotName}}", "add": "Add", "an_error_occurred_while_updating_module": "An error occurred while updating your {{moduleName}}. Please try again.", diff --git a/app/src/assets/localization/en/protocol_setup.json b/app/src/assets/localization/en/protocol_setup.json index 9406bbe16f5..3f4155a5c54 100644 --- a/app/src/assets/localization/en/protocol_setup.json +++ b/app/src/assets/localization/en/protocol_setup.json @@ -3,10 +3,10 @@ "action_needed": "Action needed", "adapter_slot_location_module": "Slot {{slotName}}, {{adapterName}} on {{moduleName}}", "adapter_slot_location": "Slot {{slotName}}, {{adapterName}}", - "add_fixture": "Add {{fixtureName}} to deck configuration", + "add_fixture": "Add {{fixtureName}} to {{locationName}}", "additional_labware": "{{count}} additional labware", "additional_off_deck_labware": "Additional Off-Deck Labware", - "add_this_deck_hardware": "Add this deck hardware to your deck configuration. It will be referenced during protocol analysis.", + "add_this_deck_hardware": "Add this hardware to your deck configuration. It will be referenced during protocol analysis.", "add_to_slot": "Add to slot {{slotName}}", "attach_gripper_failure_reason": "Attach the required gripper to continue", "attach_gripper": "attach gripper", @@ -83,7 +83,9 @@ "initial_liquids_num_plural": "{{count}} initial liquids", "initial_liquids_num": "{{count}} initial liquid", "initial_location": "Initial Location", - "install_modules_and_fixtures": "Install the required modules and power them on. Install the required fixtures and review the deck configuration.", + "install_modules": "Install the required module.", + "install_modules_plural": "Install the required modules.", + "install_modules_and_fixtures": "Install and calibrate the required modules. Install the required fixtures.", "instrument_calibrations_missing_plural": "Missing {{count}} calibrations", "instrument_calibrations_missing": "Missing {{count}} calibration", "instruments_connected_plural": "{{count}} instruments attached", @@ -132,16 +134,13 @@ "missing_pipettes": "Missing {{count}} pipette", "missing": "Missing", "modal_instructions_title": "{{moduleName}} Setup Instructions", - "module_and_deck_setup": "Modules & deck", "module_connected": "Connected", "module_disconnected": "Disconnected", "module_instructions_link": "{{moduleName}} setup instructions", "module_mismatch_body": "Check that the modules connected to this robot are of the right type and generation", "module_name": "Module", "module_not_connected": "Not connected", - "module_setup_step_description_plural": "Install the required modules and power them on.", - "module_setup_step_description": "Install the required modules and power them on.", - "module_setup_step_title": "Modules", + "module_setup_step_title": "Deck hardware", "module_slot_location": "Slot {{slotName}}, {{moduleName}}", "module": "Module", "modules_connected_plural": "{{count}} modules attached", @@ -164,6 +163,7 @@ "name": "Name", "no_custom_values": "No custom values specified", "no_data": "no data", + "no_deck_hardware_specified": "No deck hardware are specified for this protocol.", "no_labware_offset_data": "no labware offset data yet", "no_modules_or_fixtures": "No modules or fixtures are specified for this protocol.", "no_modules_specified": "no modules are specified for this protocol.", @@ -251,7 +251,7 @@ "slot_number": "Slot Number", "status": "Status", "step": "STEP {{index}}", - "there_are_no_unconfigured_modules": "No {{module}} is connected. Attach one and place it in {{slot}}", + "there_are_no_unconfigured_modules": "No {{module}} is connected. Attach one and place it in {{slot}}.", "there_are_other_configured_modules": "A {{module}} is already configured in a different slot. Exit run setup and update your deck configuration to move to an already connected module. Or attach another {{module}} to continue setup.", "tip_length_cal_description_bullet": "Perform Tip Length Calibration for each new tip type used on a pipette.", "tip_length_cal_description": "This measures the Z distance between the bottom of the tip and the pipette’s nozzle. If you redo the tip length calibration for the tip you used to calibrate a pipette, you will also have to redo that Pipette Offset Calibration.", diff --git a/app/src/organisms/DeviceDetailsDeckConfiguration/AddFixtureModal.tsx b/app/src/organisms/DeviceDetailsDeckConfiguration/AddFixtureModal.tsx index b8297db8f84..47ec3077d61 100644 --- a/app/src/organisms/DeviceDetailsDeckConfiguration/AddFixtureModal.tsx +++ b/app/src/organisms/DeviceDetailsDeckConfiguration/AddFixtureModal.tsx @@ -347,7 +347,7 @@ export function AddFixtureModal({ } > - {t('add_to_slot_description')} + {t('add_fixture_description')} {fixtureOptions} {nextStageOptions} diff --git a/app/src/organisms/DeviceDetailsDeckConfiguration/__tests__/AddFixtureModal.test.tsx b/app/src/organisms/DeviceDetailsDeckConfiguration/__tests__/AddFixtureModal.test.tsx index de4538d3253..38cc283f8e9 100644 --- a/app/src/organisms/DeviceDetailsDeckConfiguration/__tests__/AddFixtureModal.test.tsx +++ b/app/src/organisms/DeviceDetailsDeckConfiguration/__tests__/AddFixtureModal.test.tsx @@ -58,7 +58,7 @@ describe('Touchscreen AddFixtureModal', () => { render(props) screen.getByText('Add to slot D3') screen.getByText( - 'Choose an item below to add to your deck configuration. It will be referenced during protocol analysis.' + 'Add this hardware to your deck configuration. It will be referenced during protocol analysis.' ) screen.getByText('Fixtures') screen.getByText('Modules') @@ -80,7 +80,7 @@ describe('Touchscreen AddFixtureModal', () => { render(props) screen.getByText('Add to slot D3') screen.getByText( - 'Choose an item below to add to your deck configuration. It will be referenced during protocol analysis.' + 'Add this hardware to your deck configuration. It will be referenced during protocol analysis.' ) expect(screen.queryByText('Staging area slot')).toBeNull() screen.getByText('Trash bin') @@ -111,7 +111,7 @@ describe('Desktop AddFixtureModal', () => { render(props) screen.getByText('Add to slot D3') screen.getByText( - 'Add this item to your deck configuration. It will be referenced during protocol analysis.' + 'Add this hardware to your deck configuration. It will be referenced during protocol analysis.' ) screen.getByText('Fixtures') @@ -131,7 +131,7 @@ describe('Desktop AddFixtureModal', () => { render(props) screen.getByText('Add to slot A1') screen.getByText( - 'Add this item to your deck configuration. It will be referenced during protocol analysis.' + 'Add this hardware to your deck configuration. It will be referenced during protocol analysis.' ) screen.getByText('Fixtures') screen.getByText('Modules') @@ -145,7 +145,7 @@ describe('Desktop AddFixtureModal', () => { render(props) screen.getByText('Add to slot B3') screen.getByText( - 'Add this item to your deck configuration. It will be referenced during protocol analysis.' + 'Add this hardware to your deck configuration. It will be referenced during protocol analysis.' ) screen.getByText('Fixtures') screen.getByText('Modules') @@ -160,7 +160,7 @@ describe('Desktop AddFixtureModal', () => { render(props) screen.getByText('Add to slot B2') screen.getByText( - 'Add this item to your deck configuration. It will be referenced during protocol analysis.' + 'Add this hardware to your deck configuration. It will be referenced during protocol analysis.' ) screen.getByText('Magnetic Block GEN1') expect(screen.getByRole('button', { name: 'Add' })).toBeInTheDocument() diff --git a/app/src/organisms/Devices/ProtocolRun/ProtocolRunSetup.tsx b/app/src/organisms/Devices/ProtocolRun/ProtocolRunSetup.tsx index f558f1df037..05ee4b41b40 100644 --- a/app/src/organisms/Devices/ProtocolRun/ProtocolRunSetup.tsx +++ b/app/src/organisms/Devices/ProtocolRun/ProtocolRunSetup.tsx @@ -150,33 +150,24 @@ export function ProtocolRunSetup({ if (robot == null) return null const liquids = protocolAnalysis?.liquids ?? [] - const liquidsInLoadOrder = protocolAnalysis != null ? parseLiquidsInLoadOrder(liquids, protocolAnalysis.commands) : [] - const hasLiquids = liquidsInLoadOrder.length > 0 - const hasModules = protocolAnalysis != null && modules.length > 0 - // need config compatibility (including check for single slot conflicts) const requiredDeckConfigCompatibility = getRequiredDeckConfig( deckConfigCompatibility ) - const hasFixtures = requiredDeckConfigCompatibility.length > 0 - - let moduleDescription: string = t(`${MODULE_SETUP_KEY}_description`, { - count: modules.length, - }) - if (!hasModules && !isFlex) { - moduleDescription = i18n.format(t('no_modules_specified'), 'capitalize') - } else if (isFlex && (hasModules || hasFixtures)) { - moduleDescription = t('install_modules_and_fixtures') - } else if (isFlex && !hasModules && !hasFixtures) { - moduleDescription = t('no_modules_or_fixtures') - } + const flexDeckHardwareDescription = + hasModules || hasFixtures + ? t('install_modules_and_fixtures') + : t('no_deck_hardware_specified') + const ot2DeckHardwareDescription = hasModules + ? t('install_modules', { count: modules.length }) + : t('no_deck_hardware_specified') const StepDetailMap: Record< StepKey, @@ -213,7 +204,9 @@ export function ProtocolRunSetup({ protocolAnalysis={protocolAnalysis} /> ), - description: moduleDescription, + description: isFlex + ? flexDeckHardwareDescription + : ot2DeckHardwareDescription, }, [LPC_KEY]: { stepInternals: ( @@ -273,11 +266,7 @@ export function ProtocolRunSetup({ ) : ( stepsKeysInOrder.map((stepKey, index) => { - const setupStepTitle = t( - isFlex && stepKey === MODULE_SETUP_KEY - ? `module_and_deck_setup` - : `${stepKey}_title` - ) + const setupStepTitle = t(`${stepKey}_title`) const showEmptySetupStep = (stepKey === 'liquid_setup_step' && !hasLiquids) || (stepKey === 'module_setup_step' && diff --git a/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/ChooseModuleToConfigureModal.tsx b/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/ChooseModuleToConfigureModal.tsx index a2c967ac2a8..ebeaa0101fb 100644 --- a/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/ChooseModuleToConfigureModal.tsx +++ b/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/ChooseModuleToConfigureModal.tsx @@ -62,7 +62,7 @@ export const ChooseModuleToConfigureModal = ( robotName, displaySlotName, } = props - const { t } = useTranslation(['protocol_setup', 'shared']) + const { t, i18n } = useTranslation(['protocol_setup', 'shared']) const attachedModules = useModulesQuery({ refetchInterval: EQUIPMENT_POLL_MS })?.data?.data ?? [] const deckConfig = useNotifyDeckConfigurationQuery()?.data ?? [] @@ -106,7 +106,7 @@ export const ChooseModuleToConfigureModal = ( handleConfigureModule(serialNumber) }} optionName={getFixtureDisplayName(moduleFixtures[0].id, usbPort)} - buttonText={t('shared:add')} + buttonText={i18n.format(t('shared:add'), 'capitalize')} isOnDevice={isOnDevice} /> ) @@ -117,7 +117,7 @@ export const ChooseModuleToConfigureModal = ( const contents = fixtureOptions.length > 0 ? ( - + {t('add_this_deck_hardware')} {fixtureOptions} @@ -145,13 +145,7 @@ export const ChooseModuleToConfigureModal = ( onClick: onCloseClick, }} > - - - - {contents} - - - + {contents} ) : ( - - - - {contents} - - + + {contents} ), @@ -226,15 +212,16 @@ function NoUnconfiguredModules(props: NoUnconfiguredModulesProps): JSX.Element { @@ -248,7 +235,10 @@ function NoUnconfiguredModules(props: NoUnconfiguredModulesProps): JSX.Element { ) return ( - + {configuredModuleMatches.length > 0 ? ( <> diff --git a/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/NotConfiguredModal.tsx b/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/NotConfiguredModal.tsx index 76ceaf202b6..c5cef1823b0 100644 --- a/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/NotConfiguredModal.tsx +++ b/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/NotConfiguredModal.tsx @@ -13,7 +13,10 @@ import { StyledText, TYPOGRAPHY, } from '@opentrons/components' -import { getFixtureDisplayName } from '@opentrons/shared-data' +import { + getCutoutDisplayName, + getFixtureDisplayName, +} from '@opentrons/shared-data' import { TertiaryButton } from '../../../../atoms/buttons/TertiaryButton' import { getTopPortalEl } from '../../../../App/portal' import { LegacyModal } from '../../../../molecules/LegacyModal' @@ -45,11 +48,12 @@ export const NotConfiguredModal = ( updateDeckConfiguration(newDeckConfig) onCloseClick() } - + const cutoutDisplayName = getCutoutDisplayName(cutoutId) return createPortal( { if (!showBanner) return null return ( - - setShowBanner(false)} - > - - - {t('extra_module_attached')} - + setShowBanner(false)} + > + + + {t('extra_module_attached')} + - - {`${t('module_mismatch_body')}.`} - - - - + + {`${t('module_mismatch_body')}.`} + + + ) } diff --git a/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/__tests__/LocationConflictModal.test.tsx b/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/__tests__/LocationConflictModal.test.tsx index 5314acbb283..20e167fc549 100644 --- a/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/__tests__/LocationConflictModal.test.tsx +++ b/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/__tests__/LocationConflictModal.test.tsx @@ -100,7 +100,7 @@ describe('LocationConflictModal', () => { expect(props.onCloseClick).toHaveBeenCalled() fireEvent.click(screen.getByRole('button', { name: 'Update deck' })) screen.getByText('Heater-Shaker Module GEN1 in USB-1') - fireEvent.click(screen.getByRole('button', { name: 'add' })) + fireEvent.click(screen.getByRole('button', { name: 'Add' })) expect(mockUpdate).toHaveBeenCalled() }) it('should render the modal information for a single slot fixture conflict', () => { diff --git a/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/__tests__/NotConfiguredModal.test.tsx b/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/__tests__/NotConfiguredModal.test.tsx index e1a29a6a38e..955b827ec66 100644 --- a/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/__tests__/NotConfiguredModal.test.tsx +++ b/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/__tests__/NotConfiguredModal.test.tsx @@ -39,9 +39,9 @@ describe('NotConfiguredModal', () => { }) it('renders the correct text and button works as expected', () => { const { getByText, getByRole } = render(props) - getByText('Add Trash bin to deck configuration') + getByText('Add Trash bin to B3') getByText( - 'Add this deck hardware to your deck configuration. It will be referenced during protocol analysis.' + 'Add this hardware to your deck configuration. It will be referenced during protocol analysis.' ) getByText('Trash bin') fireEvent.click(getByRole('button', { name: 'Add' })) diff --git a/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/index.tsx b/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/index.tsx index 0de1a163356..33451516e41 100644 --- a/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/index.tsx +++ b/app/src/organisms/Devices/ProtocolRun/SetupModuleAndDeck/index.tsx @@ -84,13 +84,9 @@ export const SetupModuleAndDeck = ({ justifyContent={JUSTIFY_SPACE_BETWEEN} marginTop={SPACING.spacing16} marginLeft={SPACING.spacing20} - marginBottom={SPACING.spacing4} + marginBottom={SPACING.spacing12} > - + {i18n.format(t('deck_hardware'), 'capitalize')} { const ROBOT_NAME = 'otie' const RUN_ID = '1' -const MOCK_ROTOCOL_LIQUID_KEY = { liquids: [] } +const MOCK_PROTOCOL_LIQUID_KEY = { liquids: [] } const render = () => { return renderWithProviders( { .calledWith(RUN_ID) .thenReturn({ ...noModulesProtocol, - ...MOCK_ROTOCOL_LIQUID_KEY, + ...MOCK_PROTOCOL_LIQUID_KEY, } as any) when(vi.mocked(useProtocolAnalysisErrors)).calledWith(RUN_ID).thenReturn({ analysisErrors: null, @@ -97,7 +97,7 @@ describe('ProtocolRunSetup', () => { .calledWith(RUN_ID) .thenReturn(({ ...noModulesProtocol, - ...MOCK_ROTOCOL_LIQUID_KEY, + ...MOCK_PROTOCOL_LIQUID_KEY, } as unknown) as SharedData.ProtocolAnalysisOutput) vi.mocked(parseAllRequiredModuleModels).mockReturnValue([]) vi.mocked(parseLiquidsInLoadOrder).mockReturnValue([]) @@ -251,7 +251,7 @@ describe('ProtocolRunSetup', () => { .calledWith(RUN_ID) .thenReturn({ ...withModulesProtocol, - ...MOCK_ROTOCOL_LIQUID_KEY, + ...MOCK_PROTOCOL_LIQUID_KEY, } as any) when(vi.mocked(useRunHasStarted)).calledWith(RUN_ID).thenReturn(false) when(vi.mocked(useModuleCalibrationStatus)) @@ -280,7 +280,7 @@ describe('ProtocolRunSetup', () => { render() screen.getByText('STEP 2') - screen.getByText('Modules & deck') + screen.getByText('Deck hardware') screen.getByText('Calibration needed') }) @@ -305,7 +305,7 @@ describe('ProtocolRunSetup', () => { render() screen.getByText('STEP 2') - screen.getByText('Modules & deck') + screen.getByText('Deck hardware') screen.getByText('Action needed') }) @@ -339,13 +339,13 @@ describe('ProtocolRunSetup', () => { render() screen.getByText('STEP 2') - screen.getByText('Modules & deck') + screen.getByText('Deck hardware') screen.getByText('Action needed') }) it('renders module setup and allows the user to proceed to labware setup', () => { render() - const moduleSetup = screen.getByText('Modules') + const moduleSetup = screen.getByText('Deck hardware') fireEvent.click(moduleSetup) screen.getByText('Mock SetupModules') }) @@ -359,9 +359,9 @@ describe('ProtocolRunSetup', () => { 'Review required pipettes and tip length calibrations for this protocol.' ) screen.getByText('STEP 2') - screen.getByText('Modules') + screen.getByText('Deck hardware') - screen.getByText('Install the required modules and power them on.') + screen.getByText('Install the required modules.') screen.getByText('STEP 3') screen.getByText('Labware') @@ -375,7 +375,7 @@ describe('ProtocolRunSetup', () => { .calledWith(RUN_ID) .thenReturn({ ...withModulesProtocol, - ...MOCK_ROTOCOL_LIQUID_KEY, + ...MOCK_PROTOCOL_LIQUID_KEY, modules: [ { id: '1d57adf0-67ad-11ea-9f8b-3b50068bd62d:magneticModuleType', @@ -395,9 +395,9 @@ describe('ProtocolRunSetup', () => { 'Review required pipettes and tip length calibrations for this protocol.' ) screen.getByText('STEP 2') - screen.getByText('Modules') + screen.getByText('Deck hardware') - screen.getByText('Install the required modules and power them on.') + screen.getByText('Install the required module.') screen.getByText('STEP 3') screen.getByText('Labware') screen.getByText( @@ -411,7 +411,7 @@ describe('ProtocolRunSetup', () => { .calledWith(RUN_ID) .thenReturn({ ...withModulesProtocol, - ...MOCK_ROTOCOL_LIQUID_KEY, + ...MOCK_PROTOCOL_LIQUID_KEY, modules: [ { id: '1d57adf0-67ad-11ea-9f8b-3b50068bd62d:magneticModuleType', @@ -426,9 +426,9 @@ describe('ProtocolRunSetup', () => { render() screen.getByText('STEP 2') - screen.getByText('Modules & deck') + screen.getByText('Deck hardware') screen.getByText( - 'Install the required modules and power them on. Install the required fixtures and review the deck configuration.' + 'Install and calibrate the required modules. Install the required fixtures.' ) })