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

Claim last investment page #2227

Merged
merged 12 commits into from
Jan 20, 2022
Merged

Claim last investment page #2227

merged 12 commits into from
Jan 20, 2022

Conversation

alfetopito
Copy link
Contributor

Summary

Wire up last investment step, the investment summary

Screen Shot 2022-01-19 at 16 29 13

To Test

  1. Open the claim page and check one account that has at least one paid claim. At the bottom there are a couple with multiple claims
  2. Select at least one paid claim and go all the way to the review page
  • All the selected claims (including all free claims) should be visible
  • The amounts must match what was selected and shown in the previous step

Accounts

  • 0xfD40fcF9AD20C8fe4936E552833696496EA88E01
  • 0xC9A49b13A88Bd25E133AA1A709E5c31a2fc61591
  • 0x5239d4A1ff381f089D990Cd50B441cC9EFfE0ba3
  • 0x9470a102B9d6a3a00103537C2bC35ac0587Bd436
  • 0x2F144ff590C94871f118583E61fdDd06b98120e4
  • 0x42cE50b0387A081b51DAAE489DD4129CEc5a78f1
  • 0x6f14181e316377f683a9AE331B244717d38E619b
  • 0x05A1b71BF498d4A3930BA1f940E2D58C2150b399
  • 0x525Aa961cb6A422F325445F4529d58A5df3d19cc
  • 0x059E927e96079AffCC63583399641d1Da33e9Dd2

@alfetopito alfetopito self-assigned this Jan 20, 2022
@alfetopito alfetopito requested review from a team January 20, 2022 01:07
@github-actions
Copy link
Contributor

  • 🔭 GP Swap: Gnosis Protocol v2 Swap UI

@fairlighteth
Copy link
Contributor

Looks good. I enabled all investment options but have no balance. I guess it's to be expected to get this error, when wanting to execute:
Screen Shot 2022-01-20 at 11 04 40

`url(${
symbol === 'GNO' ? LogoGNO : symbol === 'ETH' ? LogoETH : symbol === 'USDC' ? LogoUSDC : theme.blueShade3
}) no-repeat center/contain`};
background: ${({ symbol, theme }) => `url(${_getLogo(symbol) || theme.blueShade3}) no-repeat center/contain`};
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! Thanks for doing this.

<span>
<b>Investment amount:</b>{' '}
<i>
{formattedCost} {symbol} ({percentage}% of available investing opportunity)
Copy link
Contributor

Choose a reason for hiding this comment

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

@michelbio, should we style differently the non 100% cases?
not investing 100% is something that cannot be undone.

I would even omit the percentage if it's 100%.
This could be done in another PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes we can review.

() =>
freeClaims.concat(selectedClaims).map((claim) => _enhancedUserClaimToClaimWithInvestment(claim, investFlowData)),
[freeClaims, investFlowData, selectedClaims]
)
Copy link
Contributor

Choose a reason for hiding this comment

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

you don't need two useMemos right? the first one could already return allClaims. I say this cause we don't use freeClaims selectedClaims anywhere but in this memo

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've split because as explained with comments in the code, one section is used for step 1 and 2 while other section is only for step 2.
I don't want to update the step 2 path unnecessarily while on step 1

@anxolin anxolin merged commit a116ab5 into claim Jan 20, 2022
@anxolin
Copy link
Contributor

anxolin commented Jan 20, 2022

small thing for a follow up, shouldn't we format using thousands?

image

I feel that should be our default everywhere

@elena-zh
Copy link

I'd add somewhere a total of claimed tokens (airdrop and investment options) that a user will receive after a successful transaction.
image

@alfetopito alfetopito deleted the claim-last-investment-page branch January 20, 2022 14:26
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