Skip to content

Commit

Permalink
fix(app): address Design QA comments for modules in deck config (#15150)
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
b-cooper authored May 9, 2024
1 parent 146cf1f commit d367217
Show file tree
Hide file tree
Showing 12 changed files with 87 additions and 112 deletions.
3 changes: 1 addition & 2 deletions app/src/assets/localization/en/device_details.json
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down
16 changes: 8 additions & 8 deletions app/src/assets/localization/en/protocol_setup.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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.",
Expand Down Expand Up @@ -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.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ export function AddFixtureModal({
}
>
<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing32}>
<StyledText as="p">{t('add_to_slot_description')}</StyledText>
<StyledText as="p">{t('add_fixture_description')}</StyledText>
<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing8}>
{fixtureOptions}
{nextStageOptions}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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')
Expand Down Expand Up @@ -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')
Expand All @@ -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')
Expand All @@ -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')
Expand All @@ -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()
Expand Down
33 changes: 11 additions & 22 deletions app/src/organisms/Devices/ProtocolRun/ProtocolRunSetup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -213,7 +204,9 @@ export function ProtocolRunSetup({
protocolAnalysis={protocolAnalysis}
/>
),
description: moduleDescription,
description: isFlex
? flexDeckHardwareDescription
: ot2DeckHardwareDescription,
},
[LPC_KEY]: {
stepInternals: (
Expand Down Expand Up @@ -273,11 +266,7 @@ export function ProtocolRunSetup({
</StyledText>
) : (
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' &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?? []
Expand Down Expand Up @@ -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}
/>
)
Expand All @@ -117,7 +117,7 @@ export const ChooseModuleToConfigureModal = (

const contents =
fixtureOptions.length > 0 ? (
<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing16}>
<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing32}>
<StyledText as="p">{t('add_this_deck_hardware')}</StyledText>
<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing8}>
{fixtureOptions}
Expand Down Expand Up @@ -145,13 +145,7 @@ export const ChooseModuleToConfigureModal = (
onClick: onCloseClick,
}}
>
<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing32}>
<Flex flexDirection={DIRECTION_COLUMN}>
<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing8}>
{contents}
</Flex>
</Flex>
</Flex>
{contents}
</Modal>
) : (
<LegacyModal
Expand All @@ -169,16 +163,8 @@ export const ChooseModuleToConfigureModal = (
onClose={onCloseClick}
width="27.75rem"
>
<Flex flexDirection={DIRECTION_COLUMN}>
<Flex paddingY={SPACING.spacing16} flexDirection={DIRECTION_COLUMN}>
<Flex
flexDirection={DIRECTION_COLUMN}
paddingTop={SPACING.spacing8}
gridGap={SPACING.spacing8}
>
{contents}
</Flex>
</Flex>
<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing8}>
{contents}
</Flex>
</LegacyModal>
),
Expand Down Expand Up @@ -226,15 +212,16 @@ function NoUnconfiguredModules(props: NoUnconfiguredModulesProps): JSX.Element {
<Flex
paddingX={SPACING.spacing80}
paddingY={SPACING.spacing40}
gridGap={isOnDevice ? SPACING.spacing40 : SPACING.spacing10}
gridGap={isOnDevice ? SPACING.spacing32 : SPACING.spacing10}
borderRadius={isOnDevice ? BORDERS.borderRadius12 : BORDERS.borderRadius8}
backgroundColor={COLORS.grey35}
backgroundColor={isOnDevice ? COLORS.grey35 : COLORS.grey30}
flexDirection={DIRECTION_COLUMN}
alignItems={ALIGN_CENTER}
>
<Icon
size={isOnDevice ? '2rem' : '1.25rem'}
marginLeft={SPACING.spacing8}
color={COLORS.grey60}
name="ot-spinner"
spin
/>
Expand All @@ -248,7 +235,10 @@ function NoUnconfiguredModules(props: NoUnconfiguredModulesProps): JSX.Element {
</Flex>
)
return (
<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing32}>
<Flex
flexDirection={DIRECTION_COLUMN}
gridGap={isOnDevice ? SPACING.spacing32 : SPACING.spacing24}
>
{configuredModuleMatches.length > 0 ? (
<>
<StyledText as="p">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -45,11 +48,12 @@ export const NotConfiguredModal = (
updateDeckConfiguration(newDeckConfig)
onCloseClick()
}

const cutoutDisplayName = getCutoutDisplayName(cutoutId)
return createPortal(
<LegacyModal
title={t('add_fixture', {
fixtureName: getFixtureDisplayName(requiredFixtureId),
locationName: cutoutDisplayName,
})}
onClose={onCloseClick}
width="27.75rem"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as React from 'react'
import { useTranslation } from 'react-i18next'
import {
Box,
DIRECTION_COLUMN,
Flex,
SPACING,
Expand All @@ -16,28 +15,26 @@ export const UnMatchedModuleWarning = (): JSX.Element | null => {
if (!showBanner) return null

return (
<Box marginTop={SPACING.spacing8}>
<Banner
iconMarginRight={SPACING.spacing16}
iconMarginLeft={SPACING.spacing8}
type="warning"
size={SPACING.spacing20}
onCloseClick={() => setShowBanner(false)}
>
<Flex flexDirection={DIRECTION_COLUMN}>
<StyledText
as="p"
fontWeight={TYPOGRAPHY.fontWeightSemiBold}
data-testid="UnMatchedModuleWarning_title"
>
{t('extra_module_attached')}
</StyledText>
<Banner
iconMarginRight={SPACING.spacing16}
iconMarginLeft={SPACING.spacing8}
type="warning"
size={SPACING.spacing20}
onCloseClick={() => setShowBanner(false)}
>
<Flex flexDirection={DIRECTION_COLUMN}>
<StyledText
as="p"
fontWeight={TYPOGRAPHY.fontWeightSemiBold}
data-testid="UnMatchedModuleWarning_title"
>
{t('extra_module_attached')}
</StyledText>

<StyledText as="p" data-testid="UnMatchedModuleWarning_body">
{`${t('module_mismatch_body')}.`}
</StyledText>
</Flex>
</Banner>
</Box>
<StyledText as="p" data-testid="UnMatchedModuleWarning_body">
{`${t('module_mismatch_body')}.`}
</StyledText>
</Flex>
</Banner>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' }))
Expand Down
Loading

0 comments on commit d367217

Please sign in to comment.