4 navbar #7
4 navbar #7
Conversation
There was a problem hiding this comment.
There are some issues, but overall great PR @skohlleffel
My biggest concern is that we should not have a static and uploaded version of each chart. Let's talk about how to pull the logic for initial data loading into the dashboard component then let a single version of each chart just handle the data it is given without needing to know where it came from.
src/App.js
Outdated
| const updated_data = data.slice(1); | ||
| const clean_data = updated_data.map(x => JSON.parse(x[0])); | ||
| // const warehouse_health_data = clean_data.filter(x => x.type==="warehouse_health"); | ||
| // const warehouse_usage_data = clean_data.filter(x => x.type==="warehouse_usage"); |
There was a problem hiding this comment.
Please clean up zombie code rather than commenting it out.
src/Components/Dashboard.js
Outdated
| if (this.props.healthcheck_data) { | ||
| return ( | ||
| <div className="mx-auto"> | ||
| <WarehouseHealthUploaded healthcheck_data={this.props.healthcheck_data}/> |
There was a problem hiding this comment.
Please filter the healthcheck data so it only passes down the data required for each chart
| @@ -0,0 +1,28 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
Please give this component a better name.
| @@ -0,0 +1,30 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
Please give this a better name. This modal does not by definition have anything to do with the nav. That's just the place we happen to trigger it from
This PR includes:
This resolves #5 #4 #6 #7 #8 #9 #10 #11.