hirosystems / connect

A library for building excellent user experiences with Stacks.
https://connect.stacks.js.org
MIT License
76 stars 40 forks source link

Fix loading of illustration in connection modal #173

Closed markmhendrickson closed 2 years ago

markmhendrickson commented 2 years ago

image

nokhodian commented 2 years ago

We are trying to modify the photo on this modal, but we didn't find any documentation is there anyway of doing this?

fbwoolf commented 2 years ago

We are trying to modify the photo on this modal, but we didn't find any documentation is there anyway of doing this?

This is an illustration specific to the Hiro Wallet. No, it can't be changed unless you fork this repo. It is not something we support/document.

markmhendrickson commented 2 years ago

@nokhodian Do you want to change the illustration itself or the app icon to the bottom-right corner of the illustration?

Illustration

Screen Shot 2022-04-04 at 13 14 28

App icon

image

If the app icon, you can pass the URL for it during authentication with Connect.

image
markmhendrickson commented 2 years ago

@fbwoolf what remains to standardize these days with the intro modal here? Should we keep this issue open and modify it to capture the remaining work more specifically, or close out?

nokhodian commented 2 years ago

@nokhodian Do you want to change the illustration itself or the app icon to the bottom-right corner of the illustration?

Illustration

Screen Shot 2022-04-04 at 13 14 28

App icon

image

If the app icon, you can pass the URL for it during authentication with Connect.

image

The problem was that no image was showing up and we couldn't modify the path, the only solution was statically putting the image where it was hardcoded, we ended up implementing the whole modal at our side though :)

markmhendrickson commented 2 years ago

The problem was that no image was showing up and we couldn't modify the path

Sorry I'm still not clear on which image wasn't showing up exactly. Do you mean the app icon or illustration or both?

we ended up implementing the whole modal at our side though :)

Did you fork the library or create a new custom modal out of curiosity?

nokhodian commented 2 years ago

image that's what the modal looks like, we are making a new custom modal, still, it's not done though

markmhendrickson commented 2 years ago

Thanks for the screenshot for clarification 🙏 If you right click on the broken image, can you copy and paste the URL that's attempted for loading here?

fbwoolf commented 2 years ago

@markmhx this is outdated. The illustration in Connect has been updated and no longer includes the app icon.

nokhodian commented 2 years ago

Thanks for the screenshot for clarification 🙏 If you right-click on the broken image, can you copy and paste the URL that's attempted for loading here?

sure,,,, /assets/download-hiro-wallet.png ... that's the image and if you statically put an image there it will show up, however, we didn't want to compromise on the structure of our project.

markmhendrickson commented 2 years ago

Thanks! By "we didn't want to compromise on the structure of our project", what do you mean exactly?

nokhodian commented 2 years ago

Thanks! By "we didn't want to compromise on the structure of our project", what do you mean exactly?

we didn't want to add a folder and file exactly like its defined, we have another folder for static resources

fbwoolf commented 2 years ago

I feel like this was reported as a separate bug regarding the app icon, but now there is a different bug with the png being used for the new illustration. Just wanting to make note of that bc when this issue was created in October /assets/download-hiro-wallet.png did not exist. I can def look into fixing why the new png isn't loading.

fbwoolf commented 2 years ago

Nvm, I read back through the thread here. I see the thread is a bit confusing ...but this is def a bug with the new png illustration, so I'll fix it.

fbwoolf commented 2 years ago

@markmhx is this something I should take on today and get fixed?

fbwoolf commented 2 years ago

There seem to be a few open issues in stenciljs regarding the getAssetPath and setAssetPath not working. It doesn't seem like a great library to use considering these issues have been open for quite some time. I've tried suggested workarounds today, but no luck. We would need to instruct the app to copy the assets in their webpack config (I think) if we want to use pngs. @kyranjamie any thoughts?

https://github.com/ionic-team/stencil/issues/2269 https://github.com/ionic-team/stencil/issues/1868

kyranjamie commented 2 years ago

I'm not super familiar with this problem, but indeed these issues look very stale.

I would try either: