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): fix failed error details modal styling and long protocol me display issue #13133

Merged
merged 9 commits into from
Jul 21, 2023

Conversation

koji
Copy link
Contributor

@koji koji commented Jul 19, 2023

Overview

  • Fix play button's disabled condition issue in ProtocolSetup
    Currently, users can call the function when tapping the button.
  • Update the box width for a case that an error is very short
  • Change overflow-wrap from break-word to anywhere

RunSummary View Error Details Modal

before

Screenshot 2023-07-19 at 11 56 46 AM

after

Screenshot 2023-07-19 at 12 30 51 PM

ProtocolDetails page

before

Screenshot 2023-07-21 at 12 33 10 PM

after

Screenshot 2023-07-21 at 12 32 56 PM

close RAUT-573

Test Plan

  • Try to run a protocol that is missing instruments or modules
    Check the play button in ProtocolSetup screen doesn't do anything.

Changelog

  • add width="100%" to flexbox for error information to keep the same width
  • update overflow-wrap in RunningProtocol related screens
  • remove marginTop from OnDeviceDisplayAppFallback (error boundary) because of no red stroke

Review requests

Risk assessment

…me display

update the box width for a case that an error is very short. change overflow-wrap from break-word to
anywhere.

RAUT-
@codecov
Copy link

codecov bot commented Jul 19, 2023

Codecov Report

Merging #13133 (48b4186) into edge (15fcb0e) will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #13133      +/-   ##
==========================================
- Coverage   72.47%   72.46%   -0.02%     
==========================================
  Files        2353     2355       +2     
  Lines       64902    65047     +145     
  Branches     7188     7259      +71     
==========================================
+ Hits        47040    47135      +95     
- Misses      16123    16143      +20     
- Partials     1739     1769      +30     
Flag Coverage Δ
app 71.15% <ø> (-0.08%) ⬇️

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

Impacted Files Coverage Δ
app/src/App/OnDeviceDisplayAppFallback.tsx 100.00% <ø> (ø)
.../RunningProtocol/CurrentRunningProtocolCommand.tsx 68.75% <ø> (ø)
...OnDeviceDisplay/RunningProtocol/RunFailedModal.tsx 81.25% <ø> (ø)
...lay/RunningProtocol/RunningProtocolCommandList.tsx 52.38% <ø> (ø)
app/src/organisms/ProtocolDetails/index.tsx 39.24% <ø> (-1.46%) ⬇️
app/src/pages/OnDeviceDisplay/RunSummary.tsx 11.66% <ø> (ø)

... and 8 files with indirect coverage changes

@koji koji requested a review from a team July 19, 2023 16:31
@koji koji marked this pull request as ready for review July 19, 2023 16:31
@koji koji requested a review from a team as a code owner July 19, 2023 16:31
@koji koji requested review from TamarZanzouri and removed request for a team and TamarZanzouri July 19, 2023 16:31
@koji koji requested a review from jerader July 20, 2023 21:54
Comment on lines 494 to 495
(protocolHasModules && attachedModules.length === 0) ||
allPipettesCalibrationData.data.length === 0
Copy link
Contributor Author

@koji koji Jul 20, 2023

Choose a reason for hiding this comment

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

allPipettesCalibrationData is object

{
  data(array)
  links
}

In my understanding, this disabled is true if there is no calibration data. We need to check the length of allPipettesCalibrationData.data

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be fixed in a following PR

@koji koji marked this pull request as draft July 21, 2023 14:33
@koji koji marked this pull request as ready for review July 21, 2023 16:36
@koji koji requested a review from shlokamin July 21, 2023 16:36
@@ -50,7 +50,6 @@ export function OnDeviceDisplayAppFallback({
return (
<Modal header={modalHeader}>
Copy link
Contributor

Choose a reason for hiding this comment

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

header needs iconColor: COLORS.red2
Screen Shot 2023-07-21 at 2 26 41 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you!

@koji koji merged commit 159ec95 into edge Jul 21, 2023
@koji koji deleted the fix_failed-view-details-modal-box-size-and-plus_alpha branch November 29, 2023 04:35
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