Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(app): address Design QA comments for modules in deck config #15150

Merged
merged 5 commits into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading