Conversation
ui/package.json
Outdated
"react-scripts": "1.1.4" | ||
"react-scripts": "1.1.4", | ||
"redux": "^4.0.0", | ||
"table-resolver": "^4.1.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that we should not use ^
to define the deps since it could install an upper version with break changes which will affect the project.
b9d3b7c
to
ad90060
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally, looks good 👍
ui/src/MobileClientBuildList.js
Outdated
<ListGroup className="list-group-build"> | ||
{mobileClientBuilds.map( | ||
( mobileClientBuild, index ) => ( | ||
<ListGroupItem key={index}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a mobileClientBuild.spec.uid
or mobileClientBuild.metadata.uid
?
ui/src/MobileClientBuildList.js
Outdated
</Col> | ||
<Col md={6} className="list-group-build-item-description"> | ||
<p className="build-info"> | ||
{(mobileClientBuild.status.phase === "Running") ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need something similar on https://redhat.invisionapp.com/share/GRJJGUHNSET#/screens/295058245 screen.
So it might make sense to make this a component. Happy for this to be a follow up issue though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might leave this as it is for now and add a note to this ticket: https://issues.jboss.org/browse/AEROGEAR-7712 to look at making this a reusable component. For now I have cleaned up the code a little bit to make it easier to read.
ui/src/MobileClientBuildList.js
Outdated
</div> | ||
); | ||
} | ||
convertTime = (timeString) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you check the resource I am pretty sure this exists on it.
https://github.com/aerogear/origin-web-console/blob/aerogear-mcp/app/views/overview/_mobile-client-row.html#L100?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using react-moment to do this now.
ui/src/styles/MobileClientBuild.css
Outdated
} | ||
|
||
.build-info { | ||
display: inline-block; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation
ui/src/styles/MobileClientBuild.css
Outdated
display: inline-block; | ||
} | ||
|
||
.timestamp { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you namespace these generic namings?
54b725e
to
29775e5
Compare
ad90060
to
d3163d5
Compare
d3163d5
to
89d07dc
Compare
relies on @sedroche 's PR to be merged first.
screenshot: