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

[Claim] Rejected/failed/errors in claim flow #2169

Merged
merged 7 commits into from
Jan 18, 2022
Merged

Conversation

W3stside
Copy link
Contributor

@W3stside W3stside commented Jan 17, 2022

Summary

Screenshots

Approving
Screenshot 2022-01-17 at 11 14 20

Airdrop Claim (first page)
image

To Test

AIRDROP

  1. using an account with airdrop and investments: 0xB1B98E968E9D7c6Ee1aa3496A753ae8922FC4683
  2. first only leave airdrop selected (the preselected one)
  3. hit claim
  4. reject wallet tx
  5. check error is as my screenshot shows
    INVESTMENT
  6. using an account with airdrop and investments: 0xB1B98E968E9D7c6Ee1aa3496A753ae8922FC4683
  7. select the investment option (unselected one)
  8. go thru the screens until on the investment approve screen
  9. since this acct already has approves, click the "invest max" button (it is test coded to reject)
  10. check error is as my screenshot shows for investment

@W3stside W3stside changed the base branch from develop to claim January 17, 2022 14:00
@W3stside W3stside changed the title Claim reject tx [Claim] Rejected/failed/errors in claim flow Jan 17, 2022
@W3stside W3stside requested review from a team January 17, 2022 14:04
@elena-zh
Copy link

Could I get a preview link, pls?

@github-actions
Copy link
Contributor

  • 🔭 GP Swap: Gnosis Protocol v2 Swap UI

@elena-zh
Copy link

Hey @W3stside , errors work nicely, however, I'd make them to be displayed a bit better in terms of UI. Maybe @biocom could help with this in a separate PR.
Besides, I'd like to propose to show a error message when something goes wrong with the TX and it fails. WDYT?
image
Anyways, it is not an issue for this PR, so let me know if I need to create a separate issue for this.

I have to notice, that mentioned above error handling works somehow for airdrop, so it would be nice to adjust this error:
image

But still I can receive a 'Success' message when Claim fails (reported in #2082)
image

@W3stside
Copy link
Contributor Author

Hey @W3stside , errors work nicely, however, I'd make them to be displayed a bit better in terms of UI. Maybe @biocom could help with this in a separate PR. Besides, I'd like to propose to show a error message when something goes wrong with the TX and it fails. WDYT? image Anyways, it is not an issue for this PR, so let me know if I need to create a separate issue for this.

so instead of the message below showing it failed, showing a popup?

I have to notice, that mentioned above error handling works somehow for airdrop, so it would be nice to adjust this error: image

same question as above

But still I can receive a 'Success' message when Claim fails (reported in #2082) image

let's make this it's own issue, not really for this PR

Copy link
Contributor

@anxolin anxolin left a comment

Choose a reason for hiding this comment

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

Can we make it coherent with the swap? there we show
image

@elena-zh
Copy link

The message has become shifted after the latest fix
image

image

@W3stside
Copy link
Contributor Author

@elena-zh @anxolin this PR is the base now for #2193 - the UX will be Error modals now like the swap page (what anxo screenshotted) and no longer those error messages. Please check #2193 first

* change hook name and add new hooks

* add new modal enum

* add hooks in app

* useMemo in useErrorMessageAndModal hook

* remove testing fn
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants