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

feat(odd): add run failed modal for running protocol #12483

Merged
merged 11 commits into from
Apr 13, 2023

Conversation

koji
Copy link
Contributor

@koji koji commented Apr 12, 2023

Overview

Add run failed modal. When runStatus is failed, the app shows the modal.

The current modal is different from the design since we need a new error handling system to display the information in the Figma. This PR is using the first of errors from run/${runId}. (Also error codes aren't defined yet.)
When the system is ready, we will be able to align this modal with the design.

I comment out the event for stopRun and I will follow up this with Shlok, Jesse, and Anthony.

Screenshot 2023-04-12 at 5 46 35 PM

[design]
https://www.figma.com/file/AoTLAYuWawlaWItB1umOjr/Release%3A-Opentrons-Flex-Touchscreen-Designs?node-id=8860-128824&t=I3cn6xOFvLdgrX4y-0

close RCORE-563

Test Plan

  • Send a protocol that an OT-3 or dev kit is missing something.
  • Tap All Protocols
  • Tap the protocol you sent
  • Tap run
    Wait for a little bit and check the modal is showed up

Changelog

  • add a new component and test for this modal
  • update Modal component for isError

Review requests

Risk assessment

low

@codecov
Copy link

codecov bot commented Apr 12, 2023

Codecov Report

Merging #12483 (e027fd5) into edge (d7f567a) will increase coverage by 0.00%.
The diff coverage is 77.77%.

Impacted file tree graph

@@           Coverage Diff           @@
##             edge   #12483   +/-   ##
=======================================
  Coverage   73.78%   73.78%           
=======================================
  Files        2247     2248    +1     
  Lines       61778    61805   +27     
  Branches     6452     6457    +5     
=======================================
+ Hits        45580    45601   +21     
- Misses      14678    14681    +3     
- Partials     1520     1523    +3     
Flag Coverage Δ
app 72.49% <77.77%> (+0.01%) ⬆️
components 67.23% <ø> (ø)
labware-library 49.74% <ø> (ø)
protocol-designer 46.34% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
app/src/molecules/BackgroundOverlay/index.tsx 100.00% <ø> (ø)
app/src/pages/OnDeviceDisplay/RunningProtocol.tsx 63.63% <62.50%> (-0.26%) ⬇️
...OnDeviceDisplay/RunningProtocol/RunFailedModal.tsx 82.35% <82.35%> (ø)
app/src/molecules/Modal/OnDeviceDisplay/Modal.tsx 100.00% <100.00%> (ø)
...rc/molecules/Modal/OnDeviceDisplay/ModalHeader.tsx 100.00% <100.00%> (ø)

@@ -57,7 +57,7 @@ export function Modal(props: ModalProps): JSX.Element {
border={`0.375rem solid ${isError ? COLORS.red_two : COLORS.white}`}
width={modalWidth}
height="max-content"
maxHeight="32.5rem"
maxHeight="33.5rem"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set the size of Run Failed's modal size

@@ -23,7 +24,7 @@ export function ModalHeader(props: ModalHeaderProps): JSX.Element {
color={isError ? COLORS.white : COLORS.black}
height="6.25rem"
width="100%"
padding={SPACING.spacing6}
padding={isError ? SPACING.spacing5 : SPACING.spacing6}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isError: true padding 24px
isError: false padding 32px
I'm asking about this to Mel since less conditional statement in ui-component would make maintain easier.

@koji koji requested review from ewagoner, jerader and shlokamin April 12, 2023 22:45
@koji koji marked this pull request as ready for review April 12, 2023 22:45
@koji koji requested a review from a team as a code owner April 12, 2023 22:45
@koji koji removed the request for review from a team April 12, 2023 22:45

if (errors == null) return null
const modalHeader: ModalHeaderBaseProps = {
title: 'Run failed',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this come from a translation file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, you are right. I will update that.

Copy link
Contributor

@ewagoner ewagoner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked good on my odd!

@koji koji merged commit 072ac6d into edge Apr 13, 2023
@koji koji deleted the feat_add-run-failed-modal-for-runningprotocol branch June 17, 2023 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants