Skip to content
This repository was archived by the owner on Jun 24, 2022. It is now read-only.

Add a loading indicator to Profile when loading data #1753

Merged
merged 8 commits into from
Nov 5, 2021

Conversation

ramirotw
Copy link
Contributor

@ramirotw ramirotw commented Nov 2, 2021

Summary

Closes #1632

This adds a loading indicator to Profile page labels using the shimmer effect. It also sets an interval to fetch profile data each 5 minutes.

To Test

  1. Connect wallet and open /profile page
  2. Labels should glow while profile is loading
  3. Wait 5 minutes to validate profile data is fetched again

@ramirotw ramirotw added the Protofire Handled by Protofire development team label Nov 2, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Nov 2, 2021

  • 🔭 GP Swap: Gnosis Protocol v2 Swap UI

@elena-zh
Copy link

elena-zh commented Nov 2, 2021

Hey @ramirotw , cool changes.
However, loading effect is so quick, that it seems like blinking again.
Could we add at least 2 sec of shimmering effect to make it noticeable (something like this https://codepen.io/andru255/pen/wvBdxbb , but for elements only) ?
See the video: when data is reloading, it shows dashes instead of numbers/date (23-26 sec of the video).

@ramirotw
Copy link
Contributor Author

ramirotw commented Nov 2, 2021

See the video: when data is reloading, it shows dashes instead of numbers/date (23-26 sec of the video).

@elena-zh is it possible that the video link you referred to wasn't pasted?

@elena-zh
Copy link

elena-zh commented Nov 2, 2021

@ramirotw , yes, it appeared to be so.
Here it is https://watch.screencastify.com/v/s3YkMOFKbn1HokZA89YN

Sorry about that :(

henrypalacios added a commit that referenced this pull request Nov 2, 2021

const emptyState: FetchProfileState = {
profileData: null,
//error: '',
Copy link
Contributor

Choose a reason for hiding this comment

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

Dead code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left that commented to hint Henry to put there the error in #1745

Comment on lines 27 to 29
setProfile({ ...emptyState, isLoading: true })
const profileData = await getProfileData(chainId, account)
setProfileData(profileData)
setProfile({ ...emptyState, isLoading: false, profileData })
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to reset the state on every load?

I would prefer to have the old value visible while it's loading

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good point, it even added an annoying flash between the dash and the value while loading

@anxolin
Copy link
Contributor

anxolin commented Nov 3, 2021

Since we are deploying today, and this one has unaddressed issues, I will leave this one out.

We will be able to repoint this PR to develop branch later, please don't merge to the release/1.4.0 after we close the version

@ramirotw ramirotw force-pushed the ramirotw/issue-1632-profile-loader branch from fdd5a6a to c003bec Compare November 3, 2021 16:20
@ramirotw ramirotw changed the base branch from release/1.4.0 to develop November 3, 2021 16:20
@ramirotw
Copy link
Contributor Author

ramirotw commented Nov 3, 2021

See the video: when data is reloading, it shows dashes instead of numbers/date

Please re-test, this issue should be fixed. Later on if I find a suitable solution to delay the shimmer effect at the end I'll include it.

@ramirotw
Copy link
Contributor Author

ramirotw commented Nov 3, 2021

The build is failing due to an unrelated change:

src/custom/pages/games/CowRunner/index.tsx
Line 4:10: 'useWalletInfo' is defined but never used @typescript-eslint/no-unused-vars

@alfetopito
Copy link
Contributor

The build is failing due to an unrelated change:

src/custom/pages/games/CowRunner/index.tsx
Line 4:10: 'useWalletInfo' is defined but never used @typescript-eslint/no-unused-vars

Same thing that happened to @henrypalacios 's PR yesterday.

Couldn't find what was causing it but I assume it was a cache issue with the stored dependencies.

Will see if I can find out more about it.

@alfetopito
Copy link
Contributor

The build is failing due to an unrelated change:

src/custom/pages/games/CowRunner/index.tsx
Line 4:10: 'useWalletInfo' is defined but never used @typescript-eslint/no-unused-vars

Same thing that happened to @henrypalacios 's PR yesterday.

Couldn't find what was causing it but I assume it was a cache issue with the stored dependencies.

Will see if I can find out more about it.

In the case of this branch, it does indeed have the "broken" code

import { useWalletInfo } from '@src/custom/hooks/useWalletInfo'

And it is present on develop https://github.com/gnosis/cowswap/blob/develop/src/custom/pages/games/CowRunner/index.tsx

But it's not on master (https://github.com/gnosis/cowswap/blob/master/src/custom/pages/games/CowRunner/index.tsx) nor on release/1.4.0 (https://github.com/gnosis/cowswap/blob/release/1.4.0/src/custom/pages/games/CowRunner/index.tsx)

Probably a bad merge.

I'll send a PR to develop to fix it.

@alfetopito
Copy link
Contributor

@ramirotw this PR should have fixed it #1770
I already merged it to develop

@elena-zh
Copy link

elena-zh commented Nov 4, 2021

Hey @ramirotw , I do not see a loading effect each 5 minutes when a profile data is updated...
In addition, some new issues:

  1. There is no 'loading' effect when we change accounts (it was described in the main part of Profile: add a loader when user details are updated #1632 task)
  2. There is no 'loading' effect when we connect a wallet, a data is loaded
  3. When a profile's data loading fails, I see an eternal loading effect. Since we are going to show an error message for this case, I see no point in showing a loading effect for 5 minutes until the next page update/user's manual update.
    https://watch.screencastify.com/v/A06UhEQh0j81wAPiMNWh

@ramirotw
Copy link
Contributor Author

ramirotw commented Nov 4, 2021

1. There is no 'loading' effect when we change accounts (it was described in the main part of Profile: add a loader when user details are updated #1632 task)
2. There is no 'loading' effect when we connect a wallet, a data is loaded

That's weird, I'm testing on https://pr1753--gpswapui.review.gnosisdev.com/#/profile and locally and I can see the shimmer effect, since the API responds quickly it doesn't take that long before the effect ends though.

3. When a profile's data loading fails, I see an eternal loading effect. Since we are going to show an error message for this case, I see no point in showing a loading effect for 5 minutes until the next page update/user's manual update.
https://watch.screencastify.com/v/A06UhEQh0j81wAPiMNWh

I have added an error catch to turn off the loader

@elena-zh
Copy link

elena-zh commented Nov 4, 2021

Thanks, @ramirotw , I have simulated a slow 3g connection and was able to see the loading effect on accounts changing/data updating.
Loading effect after the error is also removed.

@elena-zh
Copy link

elena-zh commented Nov 4, 2021

@ramirotw , I have created a separate task #1778 for you to make a 'loading' effect a bit longer

@ramirotw ramirotw merged commit 1ba6f4c into develop Nov 5, 2021
@ramirotw ramirotw deleted the ramirotw/issue-1632-profile-loader branch November 5, 2021 13:49
ramirotw added a commit that referenced this pull request Nov 10, 2021
* add shimmer effect when loading profile data

* add a 5 minutes interval to profile fetch

* do not reset previous profileData

* Fix code style issues with Prettier

* add loader to profile trades/referrals values

* cancel loading when fetch fails

Co-authored-by: Lint Action <[email protected]>
ramirotw added a commit that referenced this pull request Nov 10, 2021
* add shimmer effect when loading profile data

* add a 5 minutes interval to profile fetch

* do not reset previous profileData

* Fix code style issues with Prettier

* add loader to profile trades/referrals values

* cancel loading when fetch fails

Co-authored-by: Lint Action <[email protected]>
@ramirotw ramirotw mentioned this pull request Nov 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Protofire Handled by Protofire development team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Profile: add a loader when user details are updated
5 participants