feat(app): Add protocol file info page #2221

Kadee80 · 2018-09-11T19:08:04Z

overview

This PR addresses 95% of the UI in #2129.

For now, information section displays protocol name (as file name) and created by (based on file type). Author, last modified, and description will be added in the small follow up PR once @mcous finishes a separate PR that adds actual JSON protocol parsing to the app.

changelog

feat(app): Add protocol file info page

review requests

Upload a protocol with wrong/no pipettes attached on robot

required pipettes list renders (no check icon for wrong/missing pipettes)
warning text and button linking to attach pipettes renders

Upload a protocol with wrong/no modules attached on robot

required modules list renders (no check icon for wrong/missing modules)
warning text and button linking to attach modules renders

Upload a protocol with correct pipettes/modules attached/connected

required pipettes list renders with check icon
required modules list renders with check icon
no warning or button/link to attach instruments render when correct instruments are attached

Currently, clicking [Proceed to Calibrate] directes the user to tip probe by default.

IanLondon

Pipette (P300 single) and module (magdeck) don't show up in the file info page, but do show up in pipette & modules page

codecov · 2018-09-11T19:33:46Z

Codecov Report

Merging #2221 into edge will decrease coverage by 0.68%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##             edge    #2221      +/-   ##
==========================================
- Coverage   30.62%   29.93%   -0.69%     
==========================================
  Files         479      498      +19     
  Lines        7814     8057     +243     
==========================================
+ Hits         2393     2412      +19     
- Misses       5421     5645     +224

Impacted Files	Coverage Δ
app/src/components/UploadPanel/index.js	`0% <ø> (ø)`	⬆️
app/src/http-api-client/modules.js	`45.45% <ø> (ø)`	⬆️
app/src/components/FileInfo/ProtocolLabwareCard.js	`0% <0%> (ø)`
app/src/components/FileInfo/InstrumentWarning.js	`0% <0%> (ø)`
app/src/components/FileInfo/Continue.js	`0% <0%> (ø)`
app/src/components/FileInfo/InstrumentItem.js	`0% <0%> (ø)`
app/src/components/FileInfo/index.js	`0% <0%> (ø)`
app/src/components/FileInfo/LabwareTable.js	`0% <0%> (ø)`
app/src/components/layout/SectionContentHalf.js	`0% <0%> (ø)`
app/src/components/layout/index.js	`0% <0%> (ø)`	⬆️
... and 46 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4a8ebbe...7ec1e0a. Read the comment docs.

IanLondon

tested and works! left some code comments

IanLondon · 2018-09-11T20:14:23Z

app/src/components/FileInfo/InformationCard.js

+  const {name} = props
+
+  let createdBy = 'Opentrons API'
+  if (name.includes('json')) {


this will probably be done upstream of this component and passed down as props (?) but instead of includes you could do endsWith or a regex /*.\.json$/ b/c dilute_from_json_data.ot2.py or whatever will be seen as a JSON protocol, even though it ends with py

@IanLondon yup, upcoming parse PR handles this in a selector using endsWith

IanLondon · 2018-09-11T20:15:48Z

app/src/components/FileInfo/InstrumentItem.js

+      <StatusIcon match={props.match}/>
+      {props.mount && (
+        <span className={styles.mount_label}>
+          {props.mount.toUpperCase()}


might wanna guard props.mount && props.mount.toUpperCase() in case mount really is undefined (I'm not sure why flow isn't failing here)

line 16 handles if props.mount is undefined

IanLondon · 2018-09-11T20:20:51Z

app/src/components/FileInfo/ProtocolLabwareCard.js

+  const labwareCount = countBy(labware, 'name')
+  let labwareList = []
+
+  forEach(labwareCount, function (value, key) {


could this use map instead of forEach, since it's pushing each time?

Separate issue: does it matter that since labwareCount is an object, the iteration order (and thus the order of labwareList) is arbitrary?

It doesn't matter that the order is arbitrary. I tried this with map and Object.entries and got some whacky results, hence the forEach.
Open to other working suggestions but this seemed the most straight forward after using _countBy to get the xN

I think @IanLondon was suggesting:

import map from 'lodash/map' // ... const labwareList = map(labwareCount, (count, type) => ( <tr key={type}> <td>{type}</td> <td>{count > 1 ? `x${count}` : ''</td> </tr> ))

This would work too if we didn't want the lodash import:

const labwareList = Object.keys(labwareCount)map(type => ( <tr key={type}> <td>{type}</td> <td>{labwareCount[type] > 1 ? `x${labwareCount[type]}` : ''</td> </tr> ))

mcous

🌻

mcous · 2018-09-12T12:36:24Z

app/src/components/FileInfo/ProtocolLabwareCard.js

+  const labwareCount = countBy(labware, 'name')
+  let labwareList = []
+
+  forEach(labwareCount, function (value, key) {


I think @IanLondon was suggesting:

import map from 'lodash/map' // ... const labwareList = map(labwareCount, (count, type) => ( <tr key={type}> <td>{type}</td> <td>{count > 1 ? `x${count}` : ''</td> </tr> ))

This would work too if we didn't want the lodash import:

const labwareList = Object.keys(labwareCount)map(type => ( <tr key={type}> <td>{type}</td> <td>{labwareCount[type] > 1 ? `x${labwareCount[type]}` : ''</td> </tr> ))

mcous · 2018-09-12T13:13:07Z

app/src/components/FileInfo/InformationCard.js

+  const {name} = props
+
+  let createdBy = 'Opentrons API'
+  if (name.includes('json')) {


@IanLondon yup, upcoming parse PR handles this in a selector using endsWith

mcous · 2018-09-12T13:15:05Z

app/src/components/FileInfo/ProtocolModulesCard.js

+
+    const actualModel = actualModules && actualModules.modules.find((m) => m.name === module.name)
+    let modulesMatch = false
+    if (actualModel && actualModel.name === module.name) {


Could have done:

const modulesMatch = actualModel != null && actualModel.name === module.name

Regardless, we should explore moving this logic into a selector seeing as it's showing up in a few places now

Kadee80 added 7 commits September 11, 2018 12:51

Render file info page on successful upload

c0b4832

Add required pipettes, modules, and labware sections

7fe26b5

Add missing types

93a0be4

Disable run button until all intruments connected

3aa702b

Cleanup props

abab1fc

Add type checking to continue component

680b6c8

Match file info page designs

cc052d6

Kadee80 requested review from mcous, b-cooper and IanLondon September 11, 2018 19:08

Kadee80 self-assigned this Sep 11, 2018

Kadee80 requested review from b-cooper and IanLondon and removed request for b-cooper and IanLondon September 11, 2018 19:08

Kadee80 added feature Ticket is a feature request / PR introduces a feature app Affects the `app` project ready for review labels Sep 11, 2018

IanLondon suggested changes Sep 11, 2018

View reviewed changes

Kadee80 added 4 commits September 11, 2018 15:39

Fix check connected vs expected module logic

4a3ded9

Better missing module logic

fff6413

Add mount to pipette list

28718b7

Match typography style for mount

c28376b

IanLondon approved these changes Sep 11, 2018

View reviewed changes

Address file name endsWith comment

e1c21d0

mcous approved these changes Sep 12, 2018

View reviewed changes

Kadee80 added 2 commits September 12, 2018 09:50

Add x1 labware to table

5b6c029

Remove forEach, update mathcing modules logic

7ec1e0a

mcous merged commit e861365 into edge Sep 12, 2018

mcous deleted the app_file-info-page branch September 12, 2018 15:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(app): Add protocol file info page #2221

feat(app): Add protocol file info page #2221

Kadee80 commented Sep 11, 2018 •

edited

Loading

IanLondon left a comment

codecov bot commented Sep 11, 2018 •

edited

Loading

IanLondon left a comment

IanLondon Sep 11, 2018

Kadee80 Sep 11, 2018

mcous Sep 12, 2018

IanLondon Sep 11, 2018

Kadee80 Sep 11, 2018

IanLondon Sep 12, 2018

IanLondon Sep 11, 2018 •

edited

Loading

Kadee80 Sep 11, 2018 •

edited

Loading

mcous Sep 12, 2018

mcous left a comment

mcous Sep 12, 2018

mcous Sep 12, 2018

mcous Sep 12, 2018

feat(app): Add protocol file info page #2221

feat(app): Add protocol file info page #2221

Conversation

Kadee80 commented Sep 11, 2018 • edited Loading

overview

changelog

review requests

IanLondon left a comment

Choose a reason for hiding this comment

codecov bot commented Sep 11, 2018 • edited Loading

Codecov Report

IanLondon left a 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

IanLondon Sep 11, 2018 • edited Loading

Choose a reason for hiding this comment

Kadee80 Sep 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcous left a 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

Kadee80 commented Sep 11, 2018 •

edited

Loading

codecov bot commented Sep 11, 2018 •

edited

Loading

IanLondon Sep 11, 2018 •

edited

Loading

Kadee80 Sep 11, 2018 •

edited

Loading