Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Rework dapps list #7206

Merged
merged 5 commits into from
Dec 6, 2017
Merged

Rework dapps list #7206

merged 5 commits into from
Dec 6, 2017

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Dec 5, 2017

A start, there is more that can be done, provides a springboard for more cleanups.

parity 2017-12-05 16-22-00

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Dec 5, 2017
app={ app }
availability={ availability }
className={ styles.dapp }
key={ `${index}_${app.id}` }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't app.id be enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, overkill (as I typed it that thought came to mind)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually no. Just "fixed", the "reverted" as I remembered why I took this longer route.

When I have the app as both a local app (linked from dapps) as well as an app of the network, the ids are the same. (It was not an issue previously since they were in separate lists)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK makes sense.

@@ -48,6 +46,7 @@ class Dapps extends Component {
}

render () {
const { availability } = this.props;
let externalOverlay = null;

if (this.store.externalOverlayVisible) {
Copy link
Contributor

@amaury1093 amaury1093 Dec 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use a ? : operator here for externalOverlay

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

@5chdn 5chdn added this to the 1.9 milestone Dec 6, 2017
@amaury1093 amaury1093 added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 6, 2017
@jacogr jacogr merged commit b151765 into master Dec 6, 2017
@jacogr jacogr deleted the jg-dapp-list branch December 6, 2017 15:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants