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

2382/already claimed #2394

Merged
merged 5 commits into from
Feb 10, 2022
Merged

2382/already claimed #2394

merged 5 commits into from
Feb 10, 2022

Conversation

alfetopito
Copy link
Contributor

Summary

Closes #2382

Once you load the network you have claims on, and all claims are gone, remove the banner on other networks

Screen.Recording.2022-02-04.at.16.01.00.mov

To Test

  1. Load one account that has claims on other networks
  2. Go to the other network
  3. Claim everything on that network
  4. Go to other network
  • The banner about the network you claimed everything should no longer be displayed

@alfetopito alfetopito requested review from a team February 5, 2022 00:03
@alfetopito alfetopito self-assigned this Feb 5, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Feb 5, 2022

CLA Assistant Lite:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


Leandro seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger the CLA Action by commenting recheckcla in this Pull Request

@github-actions
Copy link
Contributor

github-actions bot commented Feb 5, 2022

  • 🔭 GP Swap: CoW Protocol v2 Swap UI

@elena-zh
Copy link

elena-zh commented Feb 7, 2022

Hey @alfetopito , great!
But I noticed, that the app freezes in this PR when I change networks. See the video: https://watch.screencastify.com/v/weEOAheFUcUbbtDw89Ue
Could you please take a look at it?

Also, I noticed that the app still shows available claims banner until I switch to another network and check that the claims have already done there.
This won't be fixed, correct?

@alfetopito
Copy link
Contributor Author

Hey @alfetopito , great! But I noticed, that the app freezes in this PR when I change networks. See the video: https://watch.screencastify.com/v/weEOAheFUcUbbtDw89Ue Could you please take a look at it?

That's odd, I'll try to reproduce it.
By any chance did you notice that on other versions or only in this PR?

Also, I noticed that the app still shows available claims banner until I switch to another network and check that the claims have already done there. This won't be fixed, correct?

Correct. We can only tell if a claim is no longer available if you access the network where you are supposed to have a claim.

@elena-zh
Copy link

elena-zh commented Feb 8, 2022

That's odd, I'll try to reproduce it.
By any chance did you notice that on other versions or only in this PR?

Hey @alfetopito , I can reproduce the issue in this PR only.
I tried it in several PRs, on Stage. in #2395 : it is not reproducible there.
image

I test in Win10+Chtome. But I'm able to reproduce it in Firefox and Brave as well.

@alfetopito alfetopito force-pushed the 2382/already-claimed branch from f8c8e3f to c01a64e Compare February 9, 2022 01:25
@alfetopito
Copy link
Contributor Author

@elena-zh looking more closely I found several issues with this PR (😳)

They should all be fixed now and I think the issue you were facing was a mistake causing it to go into an infinite loop.

Hopefully all that is fixed now 🤞

Copy link

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

LGTM now!

Leandro added 5 commits February 10, 2022 11:26
- Updated state to add new dimension to claim info: account
  Otherwise when checking a different account the state would be
  preserved and would lead to bad data
- Refactored Updater to fix dependency on obj causing it to
  being stuck in a re-render loop
- Fixed logic that compared different types using `===` instead of `==`
- Fixed logic that was causing claim to be displayed when there were no claims
@alfetopito alfetopito merged commit 9189731 into develop Feb 10, 2022
@alfetopito alfetopito deleted the 2382/already-claimed branch February 10, 2022 21:30
@github-actions github-actions bot locked and limited conversation to collaborators Feb 10, 2022
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.

[1.10] Message about available claims appear when a user has already claimed
3 participants