Project Presence panel #1117
Project Presence panel #1117
Conversation
| @@ -0,0 +1,33 @@ | |||
| import React from "react"; | |||
| import { Heading, UnorderedList, ListItem } from "evergreen-ui"; | |||
There was a problem hiding this comment.
You have so far only moved it and not changed to using material-ui?
There was a problem hiding this comment.
Yes, planning on taking Evergreen out soon, but I left it out of this PR
| import React from "react"; | ||
| import { Heading, UnorderedList, ListItem } from "evergreen-ui"; | ||
|
|
||
| export class HeaderPanel extends React.Component { |
There was a problem hiding this comment.
Does this name give us enough information about the component? Now the name says something about where it is used now, instead of what the component actually contains/is.
There was a problem hiding this comment.
I agree! I renamed it to PresenceIndicator, that fits with its current use, but it can be used for many other things, so I'm open to name suggestions:)
| {this.props.list.map( | ||
| (data, index) => ( | ||
| <ListItem key={index}> | ||
| {data.numberOfReports || data.inactiveDataCollectors} {data.name || data.healthRisk} |
There was a problem hiding this comment.
Shouldn't rather the data that we send in be shaped so we have the correct format of it, instead of "forcing" this component to be linked to the datastructure (and name of properties) of the input data?
There was a problem hiding this comment.
Agreed. I rewrote it, now it takes any JSON Object and lists them "value-first" no matter the keys:
{name: "Cholera", numberOfReports: "20"} --> "20 Cholera"
Not very scalable, but compatible with the current API endpoints. Could be a good idea to include this as a test in a frontend testing framework in the future.
Source/Analytics/Web.Frontend/src/components/ProjectPresence.js
Outdated
Show resolved
Hide resolved
Source/Analytics/Web.Frontend/src/components/ProjectPresence.js
Outdated
Show resolved
Hide resolved
|
I also have a question. Back in the days when I last used react we defined the proptypes. Is this something that isn't done anymore? And could maybe my need to know what a component expects be solvec by adding typescript? (Ref: #1126) |
… more agnostic towards its received data, and take out some unused code
|
Yes, I think TypeScript is a great idea. Adding proptypes also work, but TypeScript is stricter, which I think will help streamline different contributions. |
Features
Other changes