5afe / safe-react

Deprecated! New repo – https://github.com/safe-global/web-core
MIT License
332 stars 363 forks source link

Asset View Improvements #136

Closed lukasschor closed 4 years ago

lukasschor commented 5 years ago

Screenshot 2019-09-13 at 20.15.20.png

tschubotz commented 5 years ago

Activate tokens automatically in "Manage Tokens" if they are (1) whitelisted and (2) balance is >0

What means "whitelisted" in this case?

mmv08 commented 5 years ago

Activate tokens automatically in "Manage Tokens" if they are (1) whitelisted and (2) balance is >0

IIRC on the mainnet we have ~200 tokens, this would be 200 balances requests performed each 5s (we check the balance every 5s) I think this is a little bit too much. What we can do: 1) Check the balances only on first safe load to get list of tokens with positive balances and enable them 2) Implement an endpoint /v1/safes/{address}/balances/ from relay in tx-history-service. Wdyt @Uxio0 ?

Currently,​ we use 5 item pagination as a default, for users that have a lot of tokens, this is not convenient. Set number to 25. Option B: Automatically show all tokens, no matter how many

We can use a number of tokens with positive balances to dynamically set number of items, but I'd personally just increase the number to 10 and let the user handle that further. Another option: a search bar, this would be helpful for people with a lot of assets

lukasschor commented 5 years ago

@tschubotz

What means "whitelisted" in this case?

Tokens that we show in the "Manage Tokens" list

lukasschor commented 5 years ago

What we can do: Check the balances only on first safe load to get list of tokens with positive balances and enable them Implement an endpoint /v1/safes/{address}/balances/ from relay in tx-history-service. Wdyt @Uxio0 ?

1) Would already work in my opinion. I don't think we need to update this every 5s, but would of course be ideal if the user would not have to reload the page after sending a new token to the Safe.

We can use a number of tokens with positive balances to dynamically set number of items, but I'd personally just increase the number to 10 and let the user handle that further. Another option: a search bar, this would be helpful for people with a lot of assets

Most users will probably hold 1-10 different tokens. So I think a search bar is not needed at this point, but might be an option in the future. I guess increasing the minimum to 10 would already be a quick fix. Additionally, does it make sense to save this information? Currently, it resets to the minimum every time the page is loaded.

mmv08 commented 5 years ago

Additionally, does it make sense to save this information?

@lukasschor definitely, let's make this a requirement too

germartinez commented 5 years ago

We should also make the spinner look nicer (adding some margins) when the token list is empty. Also, if there are no tokens to show, it's weird that we have a spinner instead of a message/static image.

lukasschor commented 5 years ago

We should also make the spinner look nicer (adding some margins) when the token list is empty. Also, if there are no tokens to show, it's weird that we have a spinner instead of a message/static image.

Agree. I would just do a static, center-aligned text "No balance found. Please add funds to this Safe.". Maybe even have a hyperlink on "add" that would open the receive view. What do you think?

germartinez commented 5 years ago

We should also make the spinner look nicer (adding some margins) when the token list is empty. Also, if there are no tokens to show, it's weird that we have a spinner instead of a message/static image.

Agree. I would just do a static, center-aligned text "No balance found. Please add funds to this Safe.". Maybe even have a hyperlink on "add" that would open the receive view. What do you think?

Looks good to me ;)

mmv08 commented 5 years ago

We should also make the spinner look nicer (adding some margins) when the token list is empty. Also, if there are no tokens to show, it's weird that we have a spinner instead of a message/static image.

But the ETH is always present and cannot be disabled, why do we need the spinner? 🤔

germartinez commented 5 years ago

Eth is not always present. If you have 0 eth and 0 tokens and click on "Hide zero balances" the current spinner will be displayed. We don't need a spinner, but something. The message Lukas mentioned looks good.

Uxio0 commented 5 years ago

Hi @mikheevm. From the tx service is not really easy to do that, but if you batch the txs it shouldn't take too much to get the balances even if there are 200 tokens. After that I would just refresh the ones with balance > 0, at least so often