fix: Refactor buy with crypto modal #2130
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
…/refactor-buy-with-crypto-modal
Tested both of these and they worked correctly :D |
|
There is an issue when selecting 0 balance tokens, the estimations are not being updated. This will not affect the user given that with 0 balance they cannot proceed, just as a reminder. |
webapp/src/components/Modals/BuyWithCryptoModal/BuyWithCryptoModal.tsx
Outdated
Show resolved
Hide resolved
| }) | ||
| }, [nft, order, getContract, onAuthorizedAction, onExecuteOrder]) | ||
|
|
||
| const onBuyWithCard = useCallback(() => onExecuteOrderWithCard(nft), [nft]) |
There was a problem hiding this comment.
this looks a little bit unnecessary IMO, it might be simpler to just use
| const onBuyWithCard = useCallback(() => onExecuteOrderWithCard(nft), [nft]) | |
| const onBuyWithCard = (nft) => onExecuteOrderWithCard(nft) |
or if you really want to avoid those re-definitions of the fn in each render, maybe like this?
| const onBuyWithCard = useCallback(() => onExecuteOrderWithCard(nft), [nft]) | |
| const onBuyWithCard = useCallback((nft) => onExecuteOrderWithCard(nft), [onExecuteOrderWithCard]) |
| import { Props } from './MintNftWithCryptoModal.types' | ||
| import { OnGetCrossChainRoute, OnGetGasCost } from '../BuyWithCryptoModal.types' | ||
|
|
||
| const MintNftWithCryptoModalHOC = (props: Props) => { |
There was a problem hiding this comment.
I believe we could turn this to a generic HOC that has a prop like action that could be MINT/BUY that abstracts most of the logic to avoid having repetead code that has really small differences.
E.g:
onBuyNativelyis almost identical, it just changes the contract and nft <=> item- onGetCrossChainRoute as well is too similar, just changes the nft/tem
- similar case for other props
| ) | ||
| } | ||
|
|
||
| export const useCrossChainMintNftRoute = ( |
There was a problem hiding this comment.
Similar case here, a lot of props and params repetead between the 2, could we think of making this more generic maybe?
export const useCrossChainBuyRoute = (
item?: Item,
order?: Order,
....
) => {
const fn = item ? getMintNFTRoute : getBuyNFTRoute
const payload = item ? { item : { collectionAddress: .... } } : { nft: { ... } }
...
return crossChainProvider.fn({ ...baseParams, ...payload })
This might be a big refactor, so maybe we could leave it for later. WDYT?
…yptoModal.tsx Co-authored-by: Juanma Hidalgo <juanma06@gmail.com> Signed-off-by: Fernando Zavalia <24811313+fzavalia@users.noreply.github.com>
This PR refactors the
BuyWithCryptoModalby:MintNftWithCryptoModalandBuyNftWithCryptoModal). This logic has been moved and refactored into hooks.PaymentSelectorand thePurchaseTotal