Skip to content

Commit

Permalink
fix(app): Fix Error Recovery design inconsistencies (#16030)
Browse files Browse the repository at this point in the history
  • Loading branch information
mjhuff authored Aug 16, 2024
1 parent 216d031 commit 23eede5
Show file tree
Hide file tree
Showing 20 changed files with 109 additions and 102 deletions.
3 changes: 2 additions & 1 deletion app/src/assets/localization/en/error_recovery.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@
"next_step": "Next step",
"next_try_another_action": "Next, you can try another recovery action or cancel the run.",
"no_liquid_detected": "No liquid detected",
"overpressure_is_usually_caused": "Overpressure is usually caused by a tip contacting labware, a clog, or moving viscous liquid too quickly. If the issue persists, cancel the run and make the necessary changes to the protocol.",
"overpressure_is_usually_caused": "Overpressure is usually caused by a tip contacting labware, a clog, or moving viscous liquid too quickly",
"if_issue_persists": " If the issue persists, cancel the run and make the necessary changes to the protocol",
"pick_up_tips": "Pick up tips",
"pipette_overpressure": "Pipette overpressure",
"preserve_aspirated_liquid": "First, do you need to preserve aspirated liquid?",
Expand Down
1 change: 1 addition & 0 deletions app/src/assets/localization/en/run_details.json
Original file line number Diff line number Diff line change
Expand Up @@ -154,5 +154,6 @@
"view_current_step": "View current step",
"view_error": "View error",
"view_error_details": "View error details",
"view_warning_details": "View warning details",
"warning_details": "Warning details"
}
24 changes: 15 additions & 9 deletions app/src/atoms/buttons/LargeButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -151,24 +151,30 @@ export function LargeButton(props: LargeButtonProps): JSX.Element {
}
&:active {
background-color: ${LARGE_BUTTON_PROPS_BY_TYPE[buttonType]
.activeBackgroundColor};
${activeColorFor(buttonType)};
background-color: ${disabled
? LARGE_BUTTON_PROPS_BY_TYPE[buttonType].disabledBackgroundColor
: LARGE_BUTTON_PROPS_BY_TYPE[buttonType].activeBackgroundColor};
${!disabled && activeColorFor(buttonType)};
border: ${BORDERS.borderRadius4} solid
${LARGE_BUTTON_PROPS_BY_TYPE[buttonType].activeBackgroundColor};
${disabled
? LARGE_BUTTON_PROPS_BY_TYPE[buttonType].disabledBackgroundColor
: LARGE_BUTTON_PROPS_BY_TYPE[buttonType].activeBackgroundColor};
}
&:active #btn-icon {
${activeIconStyle(buttonType)};
}
&:focus-visible {
background-color: ${LARGE_BUTTON_PROPS_BY_TYPE[buttonType]
.focusVisibleBackgroundColor};
${activeColorFor(buttonType)};
background-color: ${disabled
? LARGE_BUTTON_PROPS_BY_TYPE[buttonType].disabledBackgroundColor
: LARGE_BUTTON_PROPS_BY_TYPE[buttonType].focusVisibleBackgroundColor};
${!disabled && activeColorFor(buttonType)};
padding: calc(${SPACING.spacing24} + ${SPACING.spacing2});
border: ${SPACING.spacing2} solid ${COLORS.transparent};
outline: 3px solid
${LARGE_BUTTON_PROPS_BY_TYPE[buttonType].focusVisibleOutlineColor};
outline: ${disabled
? 'none'
: `3px solid
${LARGE_BUTTON_PROPS_BY_TYPE[buttonType].focusVisibleOutlineColor}`};
background-clip: padding-box;
box-shadow: none;
}
Expand Down
1 change: 1 addition & 0 deletions app/src/molecules/Modal/ModalHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export function ModalHeader(props: ModalHeaderBaseProps): JSX.Element {
color={iconColor}
size="2rem"
alignSelf={ALIGN_CENTER}
marginRight={SPACING.spacing16}
/>
) : null}
<LegacyStyledText
Expand Down
4 changes: 3 additions & 1 deletion app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -994,7 +994,9 @@ function TerminalRunBanner(props: TerminalRunProps): JSX.Element | null {
onClick={handleFailedRunClick}
textDecoration={TYPOGRAPHY.textDecorationUnderline}
>
{t('view_error')}
{runStatus === RUN_STATUS_SUCCEEDED
? t('view_warning_details')
: t('view_error_details')}
</LinkButton>
</Flex>
</Banner>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,7 @@ describe('ProtocolRunHeader', () => {
.thenReturn(RUN_STATUS_FAILED)
render()

fireEvent.click(screen.getByText('View error'))
fireEvent.click(screen.getByText('View error details'))
screen.getByText('mock RunFailedModal')
})

Expand Down
2 changes: 1 addition & 1 deletion app/src/organisms/DropTipWizardFlows/BeforeBeginning.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export const BeforeBeginning = ({
? JUSTIFY_SPACE_BETWEEN
: JUSTIFY_FLEX_END
}
marginTop={issuedCommandsType === 'fixit' ? '6.875rem' : undefined}
marginTop={issuedCommandsType === 'fixit' ? '6.875rem' : 'auto'}
>
{fixitCommandTypeUtils != null ? (
<TextOnlyButton
Expand Down
4 changes: 2 additions & 2 deletions app/src/organisms/DropTipWizardFlows/Success.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ export const Success = (props: SuccessProps): JSX.Element => {
<img
src={SuccessIcon}
alt="Success Icon"
width="170px"
height="141px"
width={isOnDevice ? '282px' : '170px'}
height={isOnDevice ? '234px' : '141px'}
/>
<StyledText desktopStyle="headingSmallBold" oddStyle="level3HeaderBold">
{message}
Expand Down
5 changes: 1 addition & 4 deletions app/src/organisms/DropTipWizardFlows/TipsAttachedModal.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as React from 'react'
import capitalize from 'lodash/capitalize'
import NiceModal, { useModal } from '@ebay/nice-modal-react'
import { Trans, useTranslation } from 'react-i18next'

Expand Down Expand Up @@ -82,9 +81,7 @@ const TipsAttachedModal = NiceModal.create(
}

const is96Channel = specs.channels === 96
const displayMountText = is96Channel
? '96-Channel'
: capitalize(mount as string)
const displayMountText = is96Channel ? '96-Channel' : (mount as string)

return (
<ApiHostProvider {...host} hostname={host?.hostname ?? null}>
Expand Down
5 changes: 2 additions & 3 deletions app/src/organisms/ErrorRecoveryFlows/RunPausedSplash.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export function RunPausedSplash(
position={POSITION_ABSOLUTE}
flexDirection={DIRECTION_COLUMN}
gridGap={SPACING.spacing60}
paddingY={SPACING.spacing40}
padding={SPACING.spacing40}
backgroundColor={COLORS.red50}
zIndex={5}
>
Expand All @@ -131,7 +131,7 @@ export function RunPausedSplash(
<Icon name="ot-alert" size="4.5rem" color={COLORS.white} />
<SplashHeader>{title}</SplashHeader>
</Flex>
<Flex width="49rem" justifyContent={JUSTIFY_CENTER}>
<Flex width="100%" justifyContent={JUSTIFY_CENTER}>
<StepInfo
{...props}
oddStyle="level3HeaderBold"
Expand Down Expand Up @@ -227,7 +227,6 @@ const SplashFrame = styled(Flex)`
justify-content: ${JUSTIFY_CENTER};
align-items: ${ALIGN_CENTER};
grid-gap: ${SPACING.spacing40};
padding: ${SPACING.spacing24};
padding-bottom: 0px;
`

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ describe('RunPausedSplash', () => {

it('should render a generic paused screen if there is no handled errorType', () => {
render(props)
screen.getByText('Error')
screen.getByText('Tip not detected')
screen.getByText('MOCK STEP INFO')
})

Expand Down
3 changes: 2 additions & 1 deletion app/src/organisms/ErrorRecoveryFlows/hooks/useErrorName.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ export function useErrorName(errorKind: ErrorKind): string {
return t('pipette_overpressure')
case ERROR_KINDS.OVERPRESSURE_WHILE_DISPENSING:
return t('pipette_overpressure')
// The only "general error" case currently is tipPhysicallyMissing.
default:
return t('error')
return t('tip_not_detected')
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,6 @@ export function useRecoveryTipStatus(

setIsLoadingTipStatus(false)
setFailedCommandPipette(head(failedCommandPipettes) ?? null)
console.log(
'=>(useRecoveryTipStatus.ts:76) failedCommandPipettes',
failedCommandPipettes
)

return Promise.resolve(pipettesWithTip)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ export function OverpressureBanner(): JSX.Element | null {
<InlineNotification
type="alert"
heading={t('overpressure_is_usually_caused')}
message={t('if_issue_persists')}
/>
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ export function TwoColTextAndFailedStepNextStep(
<Flex
flexDirection={DIRECTION_COLUMN}
css={css`
gap: ${SPACING.spacing16};
gap: ${SPACING.spacing8};
@media ${RESPONSIVENESS.touchscreenMediaQuerySpecs} {
gap: ${SPACING.spacing8};
gap: ${SPACING.spacing12};
}
`}
>
Expand All @@ -58,12 +58,16 @@ export function TwoColTextAndFailedStepNextStep(
>
{leftColTitle}
</StyledText>
<StyledText
oddStyle="bodyTextRegular"
desktopStyle="bodyDefaultRegular"
>
{leftColBodyText}
</StyledText>
{typeof leftColBodyText === 'string' ? (
<StyledText
oddStyle="bodyTextRegular"
desktopStyle="bodyDefaultRegular"
>
{leftColBodyText}
</StyledText>
) : (
leftColBodyText
)}
</Flex>
<FailedStepNextStep {...props} />
</TwoColumn>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ describe('ErrorDetailsModal', () => {
expect(vi.mocked(Modal)).toHaveBeenCalledWith(
expect.objectContaining({
header: {
title: 'Error',
title: 'Tip not detected',
hasExitIcon: true,
},
onOutsideClick: props.toggleModal,
Expand Down Expand Up @@ -129,7 +129,9 @@ describe('OverpressureBanner', () => {
expect.objectContaining({
type: 'alert',
heading:
'Overpressure is usually caused by a tip contacting labware, a clog, or moving viscous liquid too quickly. If the issue persists, cancel the run and make the necessary changes to the protocol.',
'Overpressure is usually caused by a tip contacting labware, a clog, or moving viscous liquid too quickly',
message:
' If the issue persists, cancel the run and make the necessary changes to the protocol',
}),
{}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,6 @@ import { Skeleton } from '../../../atoms/Skeleton'
import { useMissingProtocolHardware } from '../../../pages/Protocols/hooks'
import { useCloneRun } from '../../ProtocolUpload/hooks'
import { useRerunnableStatusText } from './hooks'
import {
useRobotInitializationStatus,
INIT_STATUS,
} from '../../../resources/health/hooks'

import type { Run, RunData, RunStatus } from '@opentrons/api-client'
import type { ProtocolResource } from '@opentrons/shared-data'
Expand Down Expand Up @@ -96,9 +92,6 @@ export function ProtocolWithLastRun({
navigate(`runs/${createRunResponse.data.id}/setup`)
}
const { cloneRun } = useCloneRun(runData.id, onResetSuccess)
const robotInitStatus = useRobotInitializationStatus()
const isRobotInitializing =
robotInitStatus === INIT_STATUS.INITIALIZING || robotInitStatus == null
const [showSpinner, setShowSpinner] = React.useState<boolean>(false)

const protocolName =
Expand Down Expand Up @@ -173,7 +166,7 @@ export function ProtocolWithLastRun({
}
).replace('about ', '')

return isProtocolFetching || isLookingForHardware || isRobotInitializing ? (
return isProtocolFetching || isLookingForHardware ? (
<Skeleton
height="24.5rem"
width="25.8125rem"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ import { useCloneRun } from '../../../ProtocolUpload/hooks'
import { useRerunnableStatusText } from '../hooks'
import { RecentRunProtocolCard } from '../'
import { useNotifyAllRunsQuery } from '../../../../resources/runs'
import {
useRobotInitializationStatus,
INIT_STATUS,
} from '../../../../resources/health/hooks'

import type { NavigateFunction } from 'react-router-dom'
import type { ProtocolHardware } from '../../../../pages/Protocols/hooks'
Expand All @@ -54,7 +50,6 @@ vi.mock('../../../../organisms/ProtocolUpload/hooks')
vi.mock('../../../../redux/analytics')
vi.mock('../hooks')
vi.mock('../../../../resources/runs')
vi.mock('../../../../resources/health/hooks')

const RUN_ID = 'mockRunId'
const ROBOT_NAME = 'otie'
Expand Down Expand Up @@ -160,9 +155,6 @@ describe('RecentRunProtocolCard', () => {
runTimeParameters: [],
},
} as any)
vi.mocked(useRobotInitializationStatus).mockReturnValue(
INIT_STATUS.SUCCEEDED
)
when(useTrackProtocolRunEvent).calledWith(RUN_ID, ROBOT_NAME).thenReturn({
trackProtocolRunEvent: mockTrackProtocolRunEvent,
})
Expand Down Expand Up @@ -269,20 +261,6 @@ describe('RecentRunProtocolCard', () => {
screen.getByText('mock Skeleton')
})

it('should render the skeleton when the robot server is initializing', () => {
vi.mocked(useRobotInitializationStatus).mockReturnValue(
INIT_STATUS.INITIALIZING
)
render(props)
screen.getByText('mock Skeleton')
})

it('should render the skeleton when the robot server is unresponsive', () => {
vi.mocked(useRobotInitializationStatus).mockReturnValue(null)
render(props)
screen.getByText('mock Skeleton')
})

it('should push to protocol details if protocol contains runtime parameters', () => {
vi.mocked(useProtocolAnalysisAsDocumentQuery).mockReturnValue({
data: simpleAnalysisFileFixture,
Expand Down
6 changes: 2 additions & 4 deletions app/src/organisms/RunDetails/ConfirmCancelModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,8 @@ export function ConfirmCancelModal(
onClose={isCanceling ? undefined : onClose}
title={t('cancel_run_modal_heading')}
>
<Flex flexDirection={DIRECTION_COLUMN}>
<LegacyStyledText as="p" marginBottom={SPACING.spacing24}>
{cancelRunAlertInfo}
</LegacyStyledText>
<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing8}>
<LegacyStyledText as="p">{cancelRunAlertInfo}</LegacyStyledText>
<LegacyStyledText as="p" marginBottom={SPACING.spacing24}>
{t('cancel_run_module_info')}
</LegacyStyledText>
Expand Down
Loading

0 comments on commit 23eede5

Please sign in to comment.