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

Claim available on profile #2063

Closed
wants to merge 5 commits into from
Closed

Conversation

alfetopito
Copy link
Contributor

Summary

Depends on #2062

Shows Claimable amount on Profile page
Screen Shot 2022-01-07 at 15 19 09

To Test

  1. Load this test PK on Rinkeby 0xdd0cd0b5ab9055ca8b4ac7fcc6c54d179f38e9c9f59b52d22b4022fbb672187a
  2. Go to profile page
  • It should show you how much this account can claim
  • It should show you a button to the claim page
  • If you don't have anything to claim, it won't be visible (duh)
  • Networks where vCOW is not deployed (anything but Rinkeby) will not show you anything

Note horrible styled, meant to only connect the dots. Styling work desperately needed

@alfetopito alfetopito self-assigned this Jan 7, 2022
@alfetopito alfetopito requested a review from a team January 7, 2022 23:24
@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2022

  • 🔭 GP Swap: Gnosis Protocol v2 Swap UI

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.

Light approve.
My initial thinking was that i was not sure if we wanted to do this. It was not part of the initial plan at least. Since it's done, we can leave it, but this will require someone to style the profile a bit better.

The whole profile could use a bit of redesign, but im happy if at least we present the balance and claimable balance a bit better and we keep in mind to do a deeper redesign at a later point.

@elena-zh
Copy link

I think, the idea is great to show a user an amount of available to claim tokens.
Besides, in the future I'd add something like a tooltip with claimable tokens expiration date or so.
Btw, how the claimable amount is calculated? Will it show only airdrop tokens?
And what if a user has an investment opportunity? Will we show this info?

Getting back to the current implementation, I agree on the point that the UI looks a bit horrible.

  1. Balance field becomes too big when there are no unclaimed tokens
    image

  2. The fields are jumping when Profile page is loading

  3. Mobile view needs to be adjusted
    claim
    image

  4. Text in the Unclaimable tokens section could look better (add margins, etc.)
    image

@alfetopito
Copy link
Contributor Author

Btw, how the claimable amount is calculated? Will it show only airdrop tokens? And what if a user has an investment opportunity? Will we show this info?

Everything that's still claimable summed up. Includes both airdrop and investment opportunities, given that their time windows are still open.
For more details the user should go to the claim page where the breakdown is displayed.

Getting back to the current implementation, I agree on the point that the UI looks a bit horrible.

Yeah, that's what I meant :D
Design work desperately needed and not intended to be part of this PR

@alfetopito
Copy link
Contributor Author

Light approve. My initial thinking was that i was not sure if we wanted to do this. It was not part of the initial plan at least. Since it's done, we can leave it, but this will require someone to style the profile a bit better.

Of course, styling to be done for sure - if we decided to keep this.

As for why, this is a compromise for not showing the value on top and for showing the vCOW balance on this page.

What I mean is:

  1. Uniswap and our initial plan was to show the claimable balance on top but for various reasons we settled only on the button to always be displayed.
  2. We will show this vCOW balance in this page, and people who might not have checked the claim page yet (🤷) would see here in the same place whether they have something claimable

Code wise, it's ready. Style wise of course not.

Is this reasoning strong enough to justify it?
What do you all think @gnosis/gp-frontend @annamsgeorge

@alfetopito alfetopito requested review from a team and removed request for a team January 11, 2022 18:23
nenadV91 pushed a commit that referenced this pull request Jan 12, 2022
* set language using url param in language dropdown

* refactor common code into a hook

* add hook

* address pr feedback
@W3stside
Copy link
Contributor

nice! bit of a margin issue:
image

@anxolin
Copy link
Contributor

anxolin commented Jan 16, 2022

Is this reasoning strong enough to justify it?

sure :)
im not against it, that's a good reasoning. It was just in the train of thought of removing some optionals. Happy to include it.

if this doens't add to much distraction, im up for it 👌

Base automatically changed from vcow-on-profile to claim January 18, 2022 19:08
@anxolin anxolin changed the base branch from claim to develop January 20, 2022 18:08
@anxolin
Copy link
Contributor

anxolin commented Jan 20, 2022

@alfetopito i change your base branch to develop

@alfetopito
Copy link
Contributor Author

I'm leaving this on-hold until the other stuff is ready

@anxolin anxolin marked this pull request as draft January 25, 2022 10:28
@alfetopito
Copy link
Contributor Author

Let's not go with this

@alfetopito alfetopito closed this Feb 18, 2022
@alfetopito alfetopito deleted the claim-available-on-profile branch February 18, 2022 16:02
@github-actions github-actions bot locked and limited conversation to collaborators Feb 18, 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.

4 participants