refactor(app): protocol Upload Empty State Recent Run Listing #8746

jerader · 2021-11-12T21:05:12Z

Overview

closes #8464

Changelog

creates RerunningProtocolModal and adds on to UploadInfo component

Review requests

review AC on ticket and review styling to make sure it matches figma
the link in the RerunningProtocolModal is empty for now since we are still waiting on support to write the article

Risk assessment

low, behind ff

codecov · 2021-11-12T21:05:21Z

Codecov Report

Merging #8746 (e4654e3) into edge (d5f9e86) will decrease coverage by 0.18%.
The diff coverage is 93.33%.

@@            Coverage Diff             @@
##             edge    #8746      +/-   ##
==========================================
- Coverage   75.16%   74.97%   -0.19%     
==========================================
  Files        1841     1847       +6     
  Lines       48659    49140     +481     
  Branches     4746     4873     +127     
==========================================
+ Hits        36574    36843     +269     
- Misses      11272    11455     +183     
- Partials      813      842      +29

Flag	Coverage Δ
app	`69.97% <93.33%> (-0.70%)`	⬇️
protocol-designer	`44.13% <ø> (ø)`
shared-data	`81.72% <ø> (-0.21%)`	⬇️
step-generation	`90.33% <ø> (ø)`

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

Impacted Files	Coverage Δ
app/src/organisms/ProtocolUpload/UploadInput.tsx	`68.96% <92.00%> (+17.45%)`	⬆️
...rganisms/ProtocolUpload/RerunningProtocolModal.tsx	`100.00% <100.00%> (ø)`
app/src/organisms/ProtocolUpload/index.tsx	`64.51% <100.00%> (-8.82%)`	⬇️
app/src/organisms/RunDetails/CommandText.tsx	`56.00% <0.00%> (-44.00%)`	⬇️
app/src/organisms/RunDetails/hooks.ts	`18.18% <0.00%> (-31.82%)`	⬇️
...tup/LabwarePositionCheck/LabwareOffsetsSummary.tsx	`75.00% <0.00%> (-25.00%)`	⬇️
...ganisms/ProtocolUpload/hooks/useCloseCurrentRun.ts	`37.50% <0.00%> (-12.50%)`	⬇️
app/src/organisms/RunDetails/CommandTimer.tsx	`90.00% <0.00%> (-10.00%)`	⬇️
shared-data/js/helpers/index.ts	`56.89% <0.00%> (-2.37%)`	⬇️
... and 19 more

jerader · 2021-11-12T21:34:12Z

@emilywools - here is the modal + the empty state with and without previous run data.

emilywools · 2021-11-12T21:45:28Z

Hey @jerader this is looking great. can you add this line as the 3rd paragraph in the modal window (after the second paragraph)?

If you recalibrate your robot, it will clear the last run from the upload page.

emilywools · 2021-11-12T21:48:15Z

@jerader on the "launch protocol library" link, Library needs to be capitalized

on the "see how x works" it needs to be title case

b-cooper · 2021-11-12T22:04:04Z

app/src/organisms/ProtocolUpload/UploadInput.tsx

+ const fullRunTimestamp = runQuery.data?.data.createdAt
+ const indexToSplit = fullRunTimestamp?.indexOf('T')
+ if (fullRunTimestamp == null) return null
+ const date = fullRunTimestamp?.slice(0, indexToSplit)
+ const timeAndUTC =
+ indexToSplit != null ? fullRunTimestamp?.slice(indexToSplit + 1) : ''
+ const timeToSplit = timeAndUTC.indexOf('.')
+ const time = timeAndUTC.slice(0, timeToSplit)
+
+ const UTC = timeAndUTC?.slice(timeToSplit + 7)


We use date-fns for manipulating date strings in our JS projects.

By using a combination of their parseIso and format functions we should be able to get what we want in a cleaner, and less brittle manner.

import {format, parseIso} from 'date-fns' const fullRunTimestamp = runQuery.data?.data.createdAt dateFns.format(parseIso(fullRunTimestamp), 'Pppp')

closes #8464

shlokamin

left some nits, but no need to hold this up if its blocking other folks. also I did not test on a robot.

shlokamin · 2021-11-17T15:03:36Z

app/src/assets/localization/en/protocol_info.json

 "exit_modal_heading": "Confirm Close Protocol",
 "exit_modal_go_back": "No, go back",
 "exit_modal_exit": "Yes, close now",
- "exit_modal_body": "Are you sure you want to close this protocol?"
+ "exit_modal_body": "Are you sure you want to close this protocol?",
+ "robotName_last_run": "{{robot_name}}’s last run",


nit: this key is a mix of snake case and camel case, we usually stick with snake case it looks like

shlokamin · 2021-11-17T15:04:04Z

app/src/assets/localization/en/protocol_info.json

+ "rerunning_protocol_modal_header": "How Rerunning A Protocol Works",
+ "rerunning_protocol_modal_body": "<block>Opentrons displays the connected robot’s last protocol run on on the Protocol Upload page. If you run again, Opentrons loads this protocol and applies Labware Offset data if any exists.</block><block>Clicking “Run Again” will take you directly to the Run tab. If you’d like to review the deck setup or run Labware Position Check before running the protocol, navigate to the Protocol tab.</block><block>If you recalibrate your robot, it will clear the last run from the upload page.</block>",
+ "rerunning_protocol_modal_link": "Learn more about Labware Offset Data",
+ "no_labware_offset_Data": "No Labware Offset data"


nit: i dont think Data should be capitalized in this key

shlokamin · 2021-11-17T15:04:32Z

app/src/organisms/ProtocolUpload/RerunningProtocolModal.tsx

+ onCloseClick: () => unknown
+}
+
+const UPLOAD_PROTOCOL_URL = '#' // TODO IMMEDIATELY get actual link - Emily said this url isn't created it


can we make a ticket for this so we don't forget?

just made one!

shlokamin · 2021-11-17T15:07:18Z

app/src/organisms/ProtocolUpload/RerunningProtocolModal.tsx

+ width={SIZE_4}
+ backgroundColor={C_BLUE}
+ name="close"
+ id={'RobotCalModal_closeButton'}


looks like this copied from DeckCalibrationModal, it should be prefixed with RerunningProtocolModal

whoops, good catch!

shlokamin · 2021-11-17T15:07:58Z

app/src/organisms/ProtocolUpload/RerunningProtocolModal.tsx

+ color={C_BLUE}
+ href={UPLOAD_PROTOCOL_URL}
+ marginTop={SPACING_2}
+ id={'RerunningProtocol_ModalLink'}


this is should be prefixed with RerunningProtocolModal, right now theres an underscore between Protocol and Modal

shlokamin · 2021-11-17T15:10:03Z

app/src/organisms/ProtocolUpload/UploadInput.tsx

 const fileInput = React.useRef<HTMLInputElement>(null)
 const [isFileOverDropZone, setIsFileOverDropZone] = React.useState<boolean>(
 false
 )
+ const [rerunningProtocolModal, setRerunningProtocolModal] = React.useState(


can these state variables be prefixed with show to indicate that they control whether or not the modal should be rendered?

shlokamin · 2021-11-17T15:22:25Z

app/src/organisms/ProtocolUpload/UploadInput.tsx

+ const cloneMostRecentRun = useCloneRun(
+ mostRecentRunId != null ? mostRecentRunId : ''
+ )
+ // If mostRecentRun is null, the CTA that uses cloneRun won't appear so this will never be reached


nit: can this comment be moved to be just above line 87?

shlokamin · 2021-11-17T15:28:00Z

app/src/organisms/ProtocolUpload/index.tsx

+ <Trans
+ id={'file_input'}
+ t={t}
+ i18nKey="upload_and_simulate"
+ values={{ robot_name: robotName }}
+ />


how come this needs to be a Trans component?

can you add the robot_name without it being a Trans component?

sure can!

opentrons/app/src/organisms/ProtocolSetup/LabwarePositionCheck/hooks/useLabwarePositionCheck.ts

Lines 68 to 70 in f0f31a9

return t('confirm_position_and_move', {

next_slot: slot,

})

Trans components are only really necessary when you need to interpolate React elements. most of the time they're actually unnecessary:

As long you have no React/HTML nodes integrated into a cohesive sentence (text formatting like strong, em, link components, maybe others), you won't need it - most of the times you will be using the good old t function.

https://react.i18next.com/latest/trans-component

nevermind, i see how to do it 😃

whoops, replied to the comment without see your response. thanks for the info link!

jerader requested a review from a team as a code owner November 12, 2021 21:05

jerader requested review from Kadee80 and removed request for a team November 12, 2021 21:05

jerader requested review from b-cooper, shlokamin, smb2268 and emilywools November 12, 2021 21:05

b-cooper reviewed Nov 12, 2021

View reviewed changes

jerader added 7 commits November 15, 2021 10:53

change styling for empty state

f5f4d61

create modal

ade29ed

feat(app): protocol Upload Empty State Recent Run Listing

899aef8

closes #8464

address pr comments and increase testing coverage

621388d

uncomment lines in uploadInput

6ca7735

use parseISO

f935f60

add fileName

2c357c8

jerader force-pushed the app_pur-empty-state branch from 29d4011 to 2c357c8 Compare November 15, 2021 22:11

jerader added 4 commits November 15, 2021 17:30

fix test timestamp to be utc timezone

fa7b680

fix usage of divider

d7f7f5c

fix lint error;

49a7447

use nestedTextmatcher to fix timezone test

abf21de

shlokamin approved these changes Nov 17, 2021

View reviewed changes

address pr comments

e4654e3

jerader merged commit 59a8b90 into edge Nov 17, 2021

jerader deleted the app_pur-empty-state branch November 17, 2021 16:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(app): protocol Upload Empty State Recent Run Listing #8746

refactor(app): protocol Upload Empty State Recent Run Listing #8746

jerader commented Nov 12, 2021 •

edited

Loading

codecov bot commented Nov 12, 2021 •

edited

Loading

jerader commented Nov 12, 2021

emilywools commented Nov 12, 2021

emilywools commented Nov 12, 2021

b-cooper Nov 12, 2021

shlokamin left a comment •

edited

Loading

shlokamin Nov 17, 2021

shlokamin Nov 17, 2021

shlokamin Nov 17, 2021

jerader Nov 17, 2021

shlokamin Nov 17, 2021

jerader Nov 17, 2021

shlokamin Nov 17, 2021

shlokamin Nov 17, 2021

shlokamin Nov 17, 2021

shlokamin Nov 17, 2021

jerader Nov 17, 2021

shlokamin Nov 17, 2021

shlokamin Nov 17, 2021

jerader Nov 17, 2021

jerader Nov 17, 2021

refactor(app): protocol Upload Empty State Recent Run Listing #8746

refactor(app): protocol Upload Empty State Recent Run Listing #8746

Conversation

jerader commented Nov 12, 2021 • edited Loading

Overview

Changelog

Review requests

Risk assessment

codecov bot commented Nov 12, 2021 • edited Loading

Codecov Report

jerader commented Nov 12, 2021

emilywools commented Nov 12, 2021

emilywools commented Nov 12, 2021

Choose a reason for hiding this comment

shlokamin left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerader commented Nov 12, 2021 •

edited

Loading

codecov bot commented Nov 12, 2021 •

edited

Loading

shlokamin left a comment •

edited

Loading