Skip to content
This repository has been archived by the owner on Jan 22, 2018. It is now read-only.

Fix bug not showing dappIcon #29

Merged
merged 2 commits into from
Dec 18, 2017
Merged

Fix bug not showing dappIcon #29

merged 2 commits into from
Dec 18, 2017

Conversation

amaury1093
Copy link
Contributor

@amaury1093 amaury1093 commented Dec 18, 2017

  • Fix bug of dappIcons not showing
  • Remove imageRef when hiding broken images, use CSS class instead

switch (app.type) {
case 'view': {
const fallbackSrc =
window.location.protocol === 'file:' ? `dapps/${app.id}/icon.png` : `${dappHost}/dapps/${app.id}/icon.png`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jacogr Can you confirm that we are not using file:// anymore?

In that case I'll just put fallbackSrc = ${dappHost}/dapps/${app.id}/icon.png

Copy link
Contributor

Choose a reason for hiding this comment

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

We are using it for Electron. (WIP, will become important going forward)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK so I'll leave it like this

case 'builtin':
case 'network':
default:
imageSrc = app.image; // TODO: should be `${UI_URL}${app.image}` where UI_URL=http://localhost:8180
Copy link
Contributor Author

@amaury1093 amaury1093 Dec 18, 2017

Choose a reason for hiding this comment

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

I see parity_dappsUrl and parity_wsUrl, but no parity_uiUrl. I left it like this (app.image is something like /api/content/{hash}/icon.png), and it works fine. But the ideal scenario would be to use UI_URL given by the API.

Copy link
Contributor

Choose a reason for hiding this comment

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

UI url is always window.location.host.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In a dapp, window.location.host give localhost:3001, and window.parent.location.host is inaccessible

@jacogr jacogr merged commit 3cfa220 into master Dec 18, 2017
@jacogr jacogr deleted the am-dappicon branch December 18, 2017 15:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants