leather-io / extension

Leather browser extension
https://leather.io
MIT License
303 stars 142 forks source link

Refactor Stamps query and UI improvements #3648

Closed fbwoolf closed 1 year ago

fbwoolf commented 1 year ago

Continuing work from comments here to refactor the stamp-collection query and make UI improvement for adding Stamps. https://github.com/hirosystems/wallet/pull/3640

fbwoolf commented 1 year ago

@markmhx @mica000 can we capture nec UI improvements async here?

fbwoolf commented 1 year ago

I feel compelled to use Bitcoin Stamp here but not sure then ordering or if we just prefer Stamp? Also, I need an icon.

Screen Shot 2023-05-03 at 1 06 00 PM

mikeinspace commented 1 year ago

I definitely would recommend and prefer "Bitcoin Stamp" as "STAMP" is the name of the overarching blockchain-agnostic protocol. For instance, there are now Doge Stamps. So simply calling them "Stamps" rather than "Bitcoin Stamps" wouldn't be accurate.

markmhendrickson commented 1 year ago

Here's my quick mockup for the modal here, not sure if @mica000 wants to make tweaks though

image

I got the icon from https://stampchain.io/images/icon.jpg

@mikeinspace not sure if there's a better image to use?

markmhendrickson commented 1 year ago

Note we'll want to add the same row to the "Receive" modal under "Collectibles" presumably

mica000 commented 1 year ago
Screenshot 2023-05-04 at 17 19 40

Adding the tokens that needs updating to this zip file: Tokens.zip


@mikeinspace I looked around but couldn't find any visual for stamps, so I'm proposing this one with a stylised orange stamp. Let me know what you think.

@fbwoolf Im adding Ordinals again, because I updated the icon to simplify it. Could you update that one as well?

mica000 commented 1 year ago

Correct stamp icon: stamps.zip

cc @fbwoolf

mikeinspace commented 1 year ago

I've installed the latest version from the app stores (Chrome and Firefox). They both say "v4.20.0" but were released yesterday. Neither shows Stamps, but maybe this isn't the latest version in the app stores.

mikeinspace commented 1 year ago

Here's my quick mockup for the modal here, not sure if @mica000 wants to make tweaks though

image

I got the icon from https://stampchain.io/images/icon.jpg

@mikeinspace not sure if there's a better image to use?

Yes, that's the best logo we have right now. If anything changes, I'll let you know.

fbwoolf commented 1 year ago

I've installed the latest version from the app stores (Chrome and Firefox). They both say "v4.20.0" but were released yesterday. Neither shows Stamps, but maybe this isn't the latest version in the app stores.

Def in that release, but the endpoint returning stamps for an address is inconsistent and often extremely slow to load. I am often getting back empty arrays.

mikeinspace commented 1 year ago

Let me check with Kevin

hydren-crypto commented 1 year ago

I've installed the latest version from the app stores (Chrome and Firefox). They both say "v4.20.0" but were released yesterday. Neither shows Stamps, but maybe this isn't the latest version in the app stores.

Def in that release, but the endpoint returning stamps for an address is inconsistent and often extremely slow to load. I am often getting back empty arrays.

thanks for the feedback. I can definitely check into the api endpoint and get any issues there resolved.. do you have some examples where you have been seeing inconsistent data?

Currently the data does not reflect real time ownership so it may be a bit stale. i'm hoping to have the automated refreshes going here within a day or so. We got distracted with the src-20 stuff.

fwiw. queries in testing are typically sub 100ms response. i can likely speed this up, but i would be curious what you are seeing

fbwoolf commented 1 year ago

thanks for the feedback. I can definitely check into the api endpoint and get any issues there resolved.. do you have some examples where you have been seeing inconsistent data?

I think you might be ok, maybe was on our end with options set on react-query for a staleTime. Your data often comes in initially as an empty array, but we had a 5 min staleTime set so the query wasn't refreshing on it's own. I removed it and it seems to be loading better. I am opening a PR now with that change and some UI improvements, so I'll link this issue to it in a few minutes.

fbwoolf commented 1 year ago

@hydren-crypto why don't segwit addresses return any data? What is the difference b/w a creator and the query param wallet_address? Creator address can def be segwit, so I assume you can purchase/receive a stamp to a segwit address?

EDIT: Docs def suggest target_address just can't be taproot.

hydren-crypto commented 1 year ago

In general stamps are only on segwit and legacy addresses. Taproot will be coming in the future.

creator is the creator/artist of the stamp which is a 1:1 per stamp. We will be connecting those addresses to actual artist names here soon as well - likely in an artist_name field.

wallet_address is simply all stamps owned by that particular address. so if i was using my hiro wallet i would want to see everything returned by that query on my wallet address. If that query returns an empty set then that wallet doesn't own any stamps (or my data is out of date which i'm still speeding up the indexing on if you're expecting things that may have changed very recently). This should be working for both segwit and legacy as there is no differentiation in the db.

target_address is only used for our minting service API where apps connect in to create stamps. I should probably move that to a separate page.

fbwoolf commented 1 year ago

@hydren-crypto thanks for the feedback. 👍

mikeinspace commented 1 year ago

Could you let me know when this UI improvement will be deployed? At the moment, its hard to recommend Hiro to our users because they get confused over the current interface that doesn't show Stamps as an option under "Add Collectible".

When this improvement is deployed we will promote Hiro heavily to our users.

Thanks again.

markmhendrickson commented 1 year ago

Yep we'll let you know! It looks like the changes are all ready and just need code review. I imagine we could get them out by Monday.

markmhendrickson commented 1 year ago

@mikeinspace this is now live in v4.21.0