Adding errorState in useFetchProfile #1745

henrypalacios · 2021-10-29T22:49:10Z

Summary

Closes #1698

Proposal:

Include a NotificationBanner component inside the Profile Overview Card.

To Test

Go to https://pr1745--gpswapui.review.gnosisdev.com/#/profile
Connect to a wallet

src/custom/api/gnosisProtocol/api.ts

src/custom/hooks/useFetchProfile.ts

henrypalacios · 2021-10-29T22:53:33Z

src/custom/pages/Profile/index.tsx

  const lastUpdated = useTimeAgo(profileData?.lastUpdated)

+  const renderNotificationMessages = () => {


This could improve the readability to group the other notifications, what do you think @ramirotw?

I like the approach. I'm wondering though what happens if both banners are displayed, not sure if that's possible from a state perspective

while I'm aware that it is currently not possible to show both statuses at the same time now (because errors are only consulted when it is mainnet).

If you think it is better than this without grouping, let me know.

github-actions · 2021-10-29T22:56:57Z

🔭 GP Swap: Gnosis Protocol v2 Swap UI

henrypalacios · 2021-10-29T23:03:34Z

@elena-zh, @ramirotw
I have a doubt, how the user can try again?

☝️ Will this button restart the reading?

elena-zh · 2021-11-01T09:44:27Z

I have a doubt, how the user can try again?

☝️ Will this button restart the reading?

Hi @henrypalacios , no, the icon is supposed to be static (discussed in #1376 (comment), see the 4th point)

'Try again' message is displayed everywhere for error messages in different web and mobile applications. Sometimes they are accompanied with an action, sometimes not. In our case a user is able to reload the page using browser's action button/pull down the screen in a mobile device.

I'd only add 'later' to the 'Please try again' message to sound it more accomplished...

elena-zh · 2021-11-01T10:29:47Z

Hey @henrypalacios , I have taken a look into the PR.
I have to leave all the notes I reported in more or less similar task #1714 (comment) related to adding a banner:

As the banner is displayed inside, so it would be nice to remove close icon (as it is not a banner), just a warning
Important: profile data sections are missing in the mobile view
It would be nice to add a boarder radius to the banner: reported as a separate issue [Profile] implement boarder radius for info banner #1738

Also, when the PR is finished, it would be good to me to test it in a 'real' situation and check whether this banner appears only when https://protocol-mainnet.dev.gnosisdev.com/api/v1/profile/.... fails.

Thanks!

elena-zh · 2021-11-02T10:05:37Z

Hey @henrypalacios , I still see messing profile sections in the mobile view when error message is displayed.
Could you please fix it?

henrypalacios · 2021-11-02T13:43:55Z

@elena-zh It should be solved.

Regarding the banner border, this will be covered in PR-#1703, since I'm using an instance of the component that Agustin is updating in that.

ramirotw

looking good, still needs to be rebased to clear some commits

ramirotw · 2021-11-02T14:10:03Z

src/custom/pages/Profile/index.tsx

  const lastUpdated = useTimeAgo(profileData?.lastUpdated)

+  const renderNotificationMessages = () => {


I like the approach. I'm wondering though what happens if both banners are displayed, not sure if that's possible from a state perspective

src/custom/hooks/useFetchProfile.ts

elena-zh · 2021-11-03T11:09:00Z

Hey @henrypalacios , changes LGTM now!
However, you have merged @ramirotw PR #1753 into yours, so now I see shimming effect for fields/date all the time when a user is not connected/is in another network.

I think , I will need retest this once again when it will be fixed there and merged here once again...

henrypalacios · 2021-11-03T18:28:52Z

@elena-zh I have left only what concerns this problem from #1753.

alfetopito

Nipick comments on the code, the behaviour looks good

src/custom/hooks/useFetchProfile.ts

src/custom/pages/Profile/index.tsx

elena-zh · 2021-11-04T10:35:41Z

Changes LGTM now!

* Adding errorState in useFetchProfile

Co-authored-by: Crowdin Bot <[email protected]>

henrypalacios commented Oct 29, 2021

View reviewed changes

src/custom/api/gnosisProtocol/api.ts Outdated Show resolved Hide resolved

henrypalacios commented Oct 29, 2021

View reviewed changes

src/custom/hooks/useFetchProfile.ts Outdated Show resolved Hide resolved

henrypalacios commented Oct 29, 2021

View reviewed changes

henrypalacios requested review from ramirotw, alongoni and elena-zh and removed request for elena-zh October 29, 2021 23:00

henrypalacios force-pushed the 1698-warn-when-loading-error branch from bf517c4 to 21685b2 Compare November 1, 2021 20:41

henrypalacios marked this pull request as ready for review November 1, 2021 21:01

henrypalacios force-pushed the 1698-warn-when-loading-error branch from 21685b2 to 9e0f6e1 Compare November 2, 2021 13:34

ramirotw suggested changes Nov 2, 2021

View reviewed changes

henrypalacios force-pushed the 1698-warn-when-loading-error branch 2 times, most recently from 29a93b5 to 0ae32f3 Compare November 2, 2021 20:23

henrypalacios changed the base branch from develop to release/1.4.0 November 2, 2021 20:39

henrypalacios requested a review from ramirotw November 2, 2021 21:46

henrypalacios force-pushed the 1698-warn-when-loading-error branch from f3cdfb9 to 48e336c Compare November 3, 2021 00:13

henrypalacios requested a review from alfetopito November 3, 2021 00:38

ramirotw mentioned this pull request Nov 3, 2021

Add a loading indicator to Profile when loading data #1753

Merged

henrypalacios force-pushed the 1698-warn-when-loading-error branch from 48e336c to 72d5bc2 Compare November 3, 2021 15:22

henrypalacios changed the base branch from release/1.4.0 to develop November 3, 2021 15:24

alfetopito approved these changes Nov 3, 2021

View reviewed changes

src/custom/hooks/useFetchProfile.ts Outdated Show resolved Hide resolved

src/custom/pages/Profile/index.tsx Outdated Show resolved Hide resolved

ramirotw added the Protofire Handled by Protofire development team label Nov 5, 2021

Remove Loading Component added on 1753

37f8dea

henrypalacios force-pushed the 1698-warn-when-loading-error branch from c09eb53 to 4a104bf Compare November 5, 2021 16:06

Fix merging errors

2cda92f

henrypalacios force-pushed the 1698-warn-when-loading-error branch from 4a104bf to 2cda92f Compare November 5, 2021 16:14

henrypalacios requested review from ramirotw and removed request for ramirotw November 5, 2021 16:17

ramirotw approved these changes Nov 8, 2021

View reviewed changes

henrypalacios merged commit 7e08a83 into develop Nov 8, 2021

henrypalacios deleted the 1698-warn-when-loading-error branch November 8, 2021 12:56

ramirotw pushed a commit that referenced this pull request Nov 10, 2021

Adding errorState in useFetchProfile (#1745)

ded7426

* Adding errorState in useFetchProfile

ramirotw pushed a commit that referenced this pull request Nov 10, 2021

Adding errorState in useFetchProfile (#1745)

755ce84

* Adding errorState in useFetchProfile

ramirotw mentioned this pull request Nov 10, 2021

[RELEASE] 1.4.2 #1813

Merged

nenadV91 pushed a commit that referenced this pull request Nov 15, 2021

chore(i18n): synchronize translations from crowdin (#1745)

cfadd3b

Co-authored-by: Crowdin Bot <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding errorState in useFetchProfile #1745

Adding errorState in useFetchProfile #1745

henrypalacios commented Oct 29, 2021 •

edited

Loading

henrypalacios Oct 29, 2021

ramirotw Nov 2, 2021

henrypalacios Nov 2, 2021

github-actions bot commented Oct 29, 2021

henrypalacios commented Oct 29, 2021

elena-zh commented Nov 1, 2021

elena-zh commented Nov 1, 2021 •

edited

Loading

elena-zh commented Nov 2, 2021 •

edited

Loading

henrypalacios commented Nov 2, 2021

ramirotw left a comment

ramirotw Nov 2, 2021

elena-zh commented Nov 3, 2021 •

edited

Loading

henrypalacios commented Nov 3, 2021 •

edited

Loading

alfetopito left a comment

elena-zh commented Nov 4, 2021

		const lastUpdated = useTimeAgo(profileData?.lastUpdated)

		const renderNotificationMessages = () => {

Adding errorState in useFetchProfile #1745

Adding errorState in useFetchProfile #1745

Conversation

henrypalacios commented Oct 29, 2021 • edited Loading

Summary

To Test

henrypalacios Oct 29, 2021

Choose a reason for hiding this comment

ramirotw Nov 2, 2021

Choose a reason for hiding this comment

henrypalacios Nov 2, 2021

Choose a reason for hiding this comment

github-actions bot commented Oct 29, 2021

henrypalacios commented Oct 29, 2021

elena-zh commented Nov 1, 2021

elena-zh commented Nov 1, 2021 • edited Loading

elena-zh commented Nov 2, 2021 • edited Loading

henrypalacios commented Nov 2, 2021

ramirotw left a comment

Choose a reason for hiding this comment

ramirotw Nov 2, 2021

Choose a reason for hiding this comment

elena-zh commented Nov 3, 2021 • edited Loading

henrypalacios commented Nov 3, 2021 • edited Loading

alfetopito left a comment

Choose a reason for hiding this comment

elena-zh commented Nov 4, 2021

henrypalacios commented Oct 29, 2021 •

edited

Loading

elena-zh commented Nov 1, 2021 •

edited

Loading

elena-zh commented Nov 2, 2021 •

edited

Loading

elena-zh commented Nov 3, 2021 •

edited

Loading

henrypalacios commented Nov 3, 2021 •

edited

Loading