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): robot dashboard recently run protocols #12490

Merged
merged 32 commits into from
Apr 20, 2023

Conversation

jerader
Copy link
Collaborator

@jerader jerader commented Apr 13, 2023

closes RCORE-559 RCORE-586

Overview

coAuthored with @koji

Creates EmptyRecentRuns, and RecentRunProtocolCard in RobotDashboard so UI is wired up and changes depending on if you have recent runs or not.

In terms of copy for Missing, we are still waiting for Mel's feedback.
thread on Slack

Currently RecentRunProtocolCard is located under organisms since we get data via redux state and this component is used by the only one screen. If we need to use the component in somewhere, we can change the current implementation.

Screenshot 2023-04-17 at 5 14 20 PM

Test Plan

  • on the ODD, when there are no recent runs, examine the Robot dashboard. There should be an image with the text saying there are no recent runs. After you start a run and cancel/exit out, that run should appear in the robot dashboard. Observe, it should match designs. Should display up to 8 recent runs.

Changelog

  • fix assets for EmptyRecentRun and other misc assets
  • deletes MiniCardButton - i think it is no longer needed but @koji can explain why he deleted it.
    MiniCardButton was for the old design and there is no similar button in hi-fi design, so it is no longer needed.
  • alphabetizes lineHeight constants
  • adds RecentRunCard to RobotDashboard , with correct UI
  • creates new hook useMissingProtocolHardware that takes in protocolId and spits out the hardware missing that is needed in the protocol.
  • creates tests for EmptyRecentRun, RobotDashboard, and useMissingProtocolHardware

Review requests

  • see test plan

Risk assessment

low

@jerader jerader requested a review from a team as a code owner April 13, 2023 13:22
@jerader jerader requested review from brenthagen, ewagoner, koji and shlokamin and removed request for a team April 13, 2023 13:22
@codecov
Copy link

codecov bot commented Apr 13, 2023

Codecov Report

Merging #12490 (2687acf) into edge (32c898a) will increase coverage by 0.02%.
The diff coverage is 78.20%.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #12490      +/-   ##
==========================================
+ Coverage   73.67%   73.69%   +0.02%     
==========================================
  Files        2252     2257       +5     
  Lines       61976    62388     +412     
  Branches     6517     6693     +176     
==========================================
+ Hits        45658    45976     +318     
- Misses      14757    14822      +65     
- Partials     1561     1590      +29     
Flag Coverage Δ
app 72.34% <77.92%> (+0.08%) ⬆️
components 67.25% <100.00%> (+0.02%) ⬆️
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/atoms/Chip/index.tsx 100.00% <ø> (ø)
app/src/organisms/NameRobot/ConfirmRobotName.tsx 100.00% <ø> (ø)
app/src/organisms/OnDeviceDisplay/SleepScreen.tsx 0.00% <ø> (ø)
app/src/pages/OnDeviceDisplay/ConnectViaUSB.tsx 100.00% <ø> (ø)
.../pages/OnDeviceDisplay/ProtocolDashboard/index.tsx 14.06% <ø> (ø)
app/src/pages/OnDeviceDisplay/Welcome.tsx 100.00% <ø> (ø)
...splay/RobotDashboard/RecentRunProtocolCarousel.tsx 53.33% <53.33%> (ø)
...ceDisplay/RobotDashboard/RecentRunProtocolCard.tsx 91.17% <91.17%> (ø)
.../OnDeviceDisplay/RobotDashboard/EmptyRecentRun.tsx 100.00% <100.00%> (ø)
app/src/pages/OnDeviceDisplay/RobotDashboard.tsx 100.00% <100.00%> (ø)
... and 2 more

... and 23 files with indirect coverage changes

Comment on lines 130 to 134
"missing_module": "missing {{num}} module",
"missing_module_plural": "missing {{num}} modules",
"missing_pipette": "missing {{num}} pipette",
"missing_pipettes_plural": "missing {{num}} pipettes",
"missing_both": "missing {{numMod}} module(s) and {{numPip}} pipette(s)"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no idea what correct copy is supposed to be here so for now, i made the decision to pluralize some of them

@koji
Copy link
Contributor

koji commented Apr 13, 2023

I was too lazy... Seems that Robot Dashboard needs carousel.
I will take a look.

Screenshot 2023-04-13 at 10 16 11 AM

@@ -0,0 +1,26 @@
import * as React from 'react'
Copy link
Contributor

@koji koji Apr 13, 2023

Choose a reason for hiding this comment

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

@jerader I updated the test a little bit.

  • add the image name check
  • remove MemoryRoute

@jerader
Copy link
Collaborator Author

jerader commented Apr 13, 2023

I was too lazy... Seems that Robot Dashboard needs carousel. I will take a look.

Screenshot 2023-04-13 at 10 16 11 AM

@koji, nice catch, thank you!

},
],
} as any

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I only created a test for useMissingProtocolHardware but there should be tests made for useRequiredProtocolHardware and useRequiredProtocolLabware. Maybe can be done in a followup PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I added tests for useRequiredProtocolLabware.
So we will need to add useRequiredProtocolHardware.

@koji
Copy link
Contributor

koji commented Apr 13, 2023

  • merge edge
  • rename RecentRunCard -> RecentProtocolRunCard and move that into organisms
    since this component need to get data via redux state
  • add test for RecentProtocolRunCard
  • need to add carousel for Cards

@koji
Copy link
Contributor

koji commented Apr 14, 2023

  • update RecentRunProtocolCard
    • protocol name wrapping
    • missing case

@koji koji requested a review from b-cooper April 17, 2023 21:16
@ewagoner
Copy link
Contributor

Screenshot 2023-04-19 at 11 31 43 AM

The name truncation isn't quite working. Looks like the designs have an ellipses at the end rather than in the middle like we usually do.

@jerader
Copy link
Collaborator Author

jerader commented Apr 20, 2023

protocol name truncate issue has been resolved:

Screen Shot 2023-04-20 at 8 13 24 AM

-webkit-box-orient: vertical;
-webkit-line-clamp: 5;
overflow: hidden;
width: 100%;
Copy link
Contributor

Choose a reason for hiding this comment

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

@jerader thank you for the update.
I will update this style a little bit (remove unnecessary properties)

@koji koji self-requested a review April 20, 2023 12:35
Copy link
Contributor

@koji koji left a comment

Choose a reason for hiding this comment

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

Looks good to me 🏎️

@jerader jerader merged commit 5bf61cc into edge Apr 20, 2023
@jerader jerader deleted the feat_update-robotdashboard-design branch April 20, 2023 15:46
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.

4 participants