status-mobile icon indicating copy to clipboard operation
status-mobile copied to clipboard

Do not show zeros for wallet balances if they aren't loaded yet

Open flexsurfer opened this issue 4 years ago • 11 comments

Bug Report

Problem

Currently, after sign-in zeros are shown for all token balances

Expected behavior

Loading indicator or any over indicator should be shown

flexsurfer avatar Oct 05 '20 06:10 flexsurfer

Screenshot 2020-10-05 at 11 33 31

I believe the idea was to use the same loading indicator as we do for refresh / pull to refresh gesture, to indicate that the current state is temporary and pending update

do you guys want something more specific like actually not showing wallet/token balances at all?

errorists avatar Oct 05 '20 09:10 errorists

A nice alternative to indicate a state that data hasn't finished loading yet, are skeleton screens

errorists avatar Oct 05 '20 09:10 errorists

+1 on skeleton screens for content on a screen the user has navigated to that isn't loaded yet (like this usecase).

John-44 avatar Oct 05 '20 18:10 John-44

partially implemented here https://github.com/status-im/status-react/pull/11273

flexsurfer avatar Nov 11 '20 11:11 flexsurfer

Expected behavior:

  • [ ] Last known total balance is shown when wallet is opened
  • [ ] List item and card show a skeleton loading state when new values are fetched
  • [ ] If fetching fails/no new values have been received after 3 sec, show error message (Token balance could not be updated. Pull to try again)
  • [ ] Allow pull to refresh to retry (described in https://github.com/status-im/status-react/issues/10242)

@errorists please comment if this is in line with what you would expect.

hesterbruikman avatar Nov 11 '20 11:11 hesterbruikman

I pass it over to @johnlea-quiup to ask if that's what we expect :)

List item and card show a skeleton loading state when new values are fetched

Not in the described above case where we have a cached balance we can show straightaway and instead use the pull to refresh / reload spinner to indicate that we're fetching new data. Skeletons would be my choice if we were always going from unknown, but that's not the case.

Regardless I'd love if we could implement skeleton loading for chats, so I have skin in the game and am biased towards finding a way to make those happen here too

errorists avatar Nov 11 '20 11:11 errorists

@hesterbruikman I'm not sure if we want to use skeleton loading states when data is present and new data is being fetched. Skeleton screens are normally used when there isn't any data present yet. In this case why not leave the existing content as is until the new data is fetched?

Perhaps this confusion is my fault, I re-read my comment above from 5th Oct and noticed a typo - I had written "that is loaded yet..." instead of "that isn't loaded yet"" Comment now updated, my apologies.

The rest of the expected behavior points look good to me.

John-44 avatar Nov 11 '20 12:11 John-44

@errorists @johnlea-quiup sounds like we need to know what data we can assume to be present. @flexsurfer any thoughts on what we can cache?

I assumed based on the current implementation of ... that we would only cache total balance (and fiat value). If so, other data would not be present.

Also, I noticed that on first time use; there is (obviously) no cache of Total balance. In this scenario there is for sure no data present.

Fwiw, this is first time use wallet currently when balance and assets cannot be fetched Screenshot_20201111-121139_Status PR

hesterbruikman avatar Nov 11 '20 12:11 hesterbruikman

@hesterbruikman in the case of the screenshot posted above, a skeleton loading state would be a good idea as in this case data isn't present.

John-44 avatar Nov 11 '20 12:11 John-44

I would assume we should aim at caching all data so the entire table you have here, numerical values and assets.

And yes, whenever you arrive at this view and ask Status to guesstimate its content, a skeleton state would be superior. Those redacted ••• values in an ideal world should never be shown to the user.

errorists avatar Nov 11 '20 13:11 errorists

still the case on nightly 27/07/22

churik avatar Jul 27 '22 12:07 churik

Obsolete and will be re-considered in the redesigned app. Added recheck label for easy finding that one in case of need.

churik avatar Dec 05 '22 12:12 churik