refactor: pool's Deposit, Withdraw and Swap forms #362
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
amytsang
force-pushed
the
refactor/deposit-withdraw-swap-forms
branch
from
f905873
to
9d650ba
Compare
amytsang
dismissed stale reviews from OnlyJousting and michwill
via
d92af23
| userPoolBalances={userPoolBalances} | ||
| updateFormValues={updateFormValues} | ||
| /> | ||
| <DepositContext.Provider |
There was a problem hiding this comment.
idem here, imo this component is too large
apps/main/src/components/PagePool/Deposit/components/FormDeposit.tsx
Outdated
Show resolved
Hide resolved
apps/main/src/components/PagePool/Deposit/components/FormStake.tsx
Outdated
Show resolved
Hide resolved
| warning: '', | ||
| } | ||
| export function calcNewCrvApr(crvApr: number, lpToken: string, gaugeTotalSupply: string | number) { | ||
| if (!crvApr && !gaugeTotalSupply && Number(gaugeTotalSupply) === 0 && Number(lpToken) === 0) return null |
There was a problem hiding this comment.
What's the expected values for gaugeTotalSupply? Won't this go wrong with a '0' string?
| value: string | ||
| } | ||
|
|
||
| const FieldAmount: React.FC<Props> = ({ idx, estimatedGas, error, tokenObj, value }) => { |
There was a problem hiding this comment.
Just out of curiosity, why are we inverting the forms and fields names? I would expect this to be called AmountField
| }, [basePlusPriority, estimatedGas, estimatedGasIsLoading, fromAddress, updateFormValues, fromBalance]) | ||
|
|
||
| return ( | ||
| <div> |
There was a problem hiding this comment.
what's the wrapper <div> for?
There was a problem hiding this comment.
Prevents a gap between the input field and dollar amount.
| usdRatesMapper: QueryRespUsdRatesMapper | undefined | ||
| } | ||
|
|
||
| const FieldTo: React.FC<Props> = ({ usdRatesMapper }) => { |
There was a problem hiding this comment.
This is almost a duplicate from FieldFrom couldn't we use the same field?
apps/main/src/components/PagePool/Claim/components/BtnClaim.tsx
Outdated
Show resolved
Hide resolved
apps/main/src/components/PagePool/Claim/components/BtnClaimCrv.tsx
Outdated
Show resolved
Hide resolved
apps/main/src/components/PagePool/Claim/components/BtnClaimRewards.tsx
Outdated
Show resolved
Hide resolved
|
|
||
| export const ClaimContext = createContext<ClaimContextType | null>(null) | ||
|
|
||
| export const useClaimContext = () => { |
There was a problem hiding this comment.
I understand you created the context so you don't have to use react-hook-forms?
There was a problem hiding this comment.
No, so I don't have to keep on passing it as a prop. This PR does not have anything to do with switching to react-hook-form
There was a problem hiding this comment.
Yeah, I can see why you did it. React-hook-forms also uses context to pass data between components, essentially does the same thing you did, but gives you the ability to work with the form. Migrating those forms to this library will be the next step.
| import Spinner, { SpinnerWrapper } from '@/ui/Spinner' | ||
| import TransferActions from '@/components/PagePool/components/TransferActions' | ||
|
|
||
| const FormClaim = () => { |
There was a problem hiding this comment.
| const FormClaim = () => { | |
| const FormClaim = ({chainId, poolId}) => { |
I would suggest passing the basic parameters that define exactly what this form refers to through the props. In the specific case, these are chainId and poolId. This can be useful in the future when creating static components, it will exclude their memoization when changing the parameter (like key in lists). Also it will be useful if we make modal windows and there will be a situation when 2 identical modal windows are rendered for different pools.
| return { mutation, enabled: conditions.enableClaim(params) } | ||
| } | ||
|
|
||
| // helpers |
| UnstakeEstGas, | ||
| } from '@/entities/withdraw' | ||
|
|
||
| export const withdrawKeys = { |
| selected, | ||
| lpToken, | ||
| amounts, | ||
| selectedTokenAddress, |
packages/curve-lib/src/web3.ts
Outdated
There was a problem hiding this comment.
The path should be like src/shared/transaction/status.ts
amytsang
force-pushed
the
refactor/deposit-withdraw-swap-forms
branch
from
d92af23
to
110a24d
Compare
amytsang
force-pushed
the
refactor/deposit-withdraw-swap-forms
branch
from
110a24d
to
10e87e2
Compare
No description provided.