Feat: Add Mint name with crypto modal #2140

LautaroPetaccio · 2024-02-17T16:13:35Z

This PR adds the MintNameWithCryptoModal which uses the BuyWithCryptoModal.

…/refactor-buy-with-crypto-modal

vercel · 2024-02-17T16:13:40Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
marketplace	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 22, 2024 3:30pm

coveralls · 2024-02-17T16:16:03Z

coverage: 65.958% (+0.1%) from 65.86%
when pulling cc9acb7 on feat/mint-name-with-crypto-modal
into 558dd01 on master.

…t/mint-name-with-crypto-modal

juanmahidalgo

LGTM!! great job 👏👏👏

juanmahidalgo · 2024-02-22T13:22:48Z

...components/AssetPage/SaleActionBox/BuyNFTButtons/BuyWithCryptoButton/BuyWithCryptoButton.tsx

@@ -0,0 +1,13 @@
+import { t } from 'decentraland-dapps/dist/modules/translation'


the file webapp/src/components/AssetPage/SaleActionBox/BuyNFTButtons/BuyWithCryptoButton/BuyWithCryptoButton.container.ts looks empty, should re remove it?

Removed it!

juanmahidalgo · 2024-02-22T13:26:22Z

webapp/src/components/Modals/BuyWithCryptoModal/BuyWithCryptoModal.tsx

-          {t(`buy_with_crypto_modal.buy_with_card`)}
-        </ChainButton>
+        {onBuyWithCard && (
+          <ChainButton


might not be part of the scope of the PR but should we change this ChainButton to a regular one? Since it's buying with card and it won't matter in which chain is the asset, right?
Also, can we turn onBuyWithCard to be a mandatory param so we don't have to do this checks? as the one above in line 344. WDYT?

It seems that we should definitely change it. I'll apply the change in a different PR with more refactors.

juanmahidalgo · 2024-02-22T13:29:17Z

...nents/Modals/BuyWithCryptoModal/MintNameWithCryptoModal/MintNameWithCryptoModal.container.ts

+
+const mapState = (state: RootState): MapStateProps => {
+  return {
+    isClaimingName: isLoadingType(getLoading(state), CLAIM_NAME_REQUEST),


Can we try to unify the verb here? Either claim or mint, WDYT?

Done! I've used Minting as the modal name is called Mint.

juanmahidalgo · 2024-02-22T13:30:01Z

...src/components/Modals/BuyWithCryptoModal/MintNameWithCryptoModal/MintNameWithCryptoModal.tsx

+  } = props
+
+  const onBuyNatively = useCallback(() => {
+    console.log('Executing on buy natively')


can we remove this log?

Removed it!

juanmahidalgo · 2024-02-22T13:43:16Z

webapp/src/components/Modals/BuyWithCryptoModal/hooks.ts

+  if (item.price === '0' || isPriceTooLow(item.price)) {
+    return {
+      gasCost: undefined,
+      isFetchingGasCost: false


should we calculate the gast cost for this cases? the meta tx are not available and it's not going through the provider, right?

Removed it!

juanmahidalgo · 2024-02-22T13:46:52Z

webapp/src/components/Modals/BuyWithCryptoModal/hooks.ts

@@ -246,8 +245,15 @@ export const useBuyNftGasCost = (
    selectedChain,
    order.network
  )
+
+  if (order.price === '0' || isPriceTooLow(order.price)) {


in case we keep this logic, should we move it into a function to avoid having it repeated?

Removed it!

juanmahidalgo · 2024-02-22T13:50:55Z

webapp/src/modules/ens/sagas.ts

@@ -117,4 +135,48 @@ export function* ensSaga() {
      yield put(claimNameFailure(ensError))
    }
  }
+
+  function* handleClaimNameCrossChainRequest(


WDYT if we create something generic like EXECUTE_ROUTE_REQUEST and use it to execute cross-chain routes and later on dispatch the success action depending on what is getting bought? (might get out of scope if too complex, we can leave it for later)

Totally on board with this change, I'll apply it in a second PR with the sagas tests.

juanmahidalgo · 2024-02-22T13:54:04Z

webapp/src/modules/ens/sagas.ts

@@ -74,7 +87,12 @@ export function* ensSaga() {
          content: ethers.constants.AddressZero,
          contractAddress: dclRegistrarContract.address
        }
-        yield put(claimNameSuccess(ens, subdomain, hash))
+
+        if (isCrossChain) {


can't we just put the claimNameSuccess action in both cases? I don't fully understand the difference between putting the 2 different actions.

As we split the loading selector into buying natively and cross chain, this action should eventually end up clearing the cross chain request action loader. I'll keep it for now and I'll try to refactor the modal into a single loader to later see if we can simplify this and other requests as well.

juanmahidalgo · 2024-02-22T13:54:33Z

webapp/src/modules/ens/actions.ts

@@ -2,6 +2,7 @@ import { action } from 'typesafe-actions'
 import { ChainId } from '@dcl/schemas'
 import { buildTransactionPayload } from 'decentraland-dapps/dist/modules/transaction/utils'
 import { ENS, ENSError } from './types'
+import { Route } from 'decentraland-transactions/crossChain'


can we move this import up a few lines?

juanmahidalgo · 2024-02-22T14:07:17Z

webapp/src/components/Modals/ClaimNameFatFingerModal/ClaimNameFatFingerModal.container.ts

-import { getIsClaimingNamesWithFiatEnabled } from '../../../modules/features/selectors'
+import {
+  getIsClaimingNamesWithFiatEnabled,
+  getIsMintingNamesWithAxelarEnabled


nit-pick: can we change the name Axelar for CrossChain to keep consistency?

Changed it!

LautaroPetaccio added 14 commits February 13, 2024 09:53

fix: Refactor buy with crypto modal

d1d3d99

fix: Complete refactor

740341c

fix: Loading correct modal

e3c6985

fix: Use refactored modals

b388876

fix: Tests

a665687

fix: Casing

6765588

fix: Removed old casing

0b30479

fix: Re-case

367ccd0

fix: Remove old cased directory

8d6079f

fix: Re-case

4e54d83

fix: Little improvements

c2b0fc8

Merge branch 'master' of github.com:decentraland/marketplace into fix…

9823102

…/refactor-buy-with-crypto-modal

feat: Add MintNameWithCryptoModal

1a6c455

feat: Use the new modal

70b745c

fix: Test type

df5f287

vercel bot deployed to Preview February 17, 2024 16:19 View deployment

LautaroPetaccio added 2 commits February 18, 2024 13:32

feat: Add some hook tests

7968df6

fix: Remove selectedToken unnecessary checks

ed9312f

vercel bot deployed to Preview February 18, 2024 12:48 View deployment

fix: Add use name minting gas tests

65519c3

vercel bot deployed to Preview February 19, 2024 18:55 View deployment

vercel bot deployed to Preview February 20, 2024 12:55 View deployment

Base automatically changed from fix/refactor-buy-with-crypto-modal to master February 20, 2024 14:35

fzavalia force-pushed the feat/mint-name-with-crypto-modal branch from d39194e to 65519c3 Compare February 20, 2024 15:21

fix: Merge branch 'master' into feat/mint-name-with-crypto-modal

878c512

vercel bot deployed to Preview February 20, 2024 15:54 View deployment

fix: Refech route on change token

8a1d379

vercel bot deployed to Preview February 20, 2024 16:51 View deployment

Merge branch 'master' of github.com:decentraland/marketplace into fea…

c23be95

…t/mint-name-with-crypto-modal

vercel bot deployed to Preview February 22, 2024 12:25 View deployment

feat: On go back

903e3f1

vercel bot deployed to Preview February 22, 2024 13:35 View deployment

juanmahidalgo approved these changes Feb 22, 2024

View reviewed changes

LautaroPetaccio added 2 commits February 22, 2024 11:54

fix: Remove buying price from hooks

73b06c9

fix: Review fixes

cc9acb7

vercel bot deployed to Preview February 22, 2024 15:30 View deployment

LautaroPetaccio merged commit 2e18a55 into master Feb 22, 2024
7 checks passed

LautaroPetaccio deleted the feat/mint-name-with-crypto-modal branch February 22, 2024 15:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: Add Mint name with crypto modal #2140

Feat: Add Mint name with crypto modal #2140

LautaroPetaccio commented Feb 17, 2024

vercel bot commented Feb 17, 2024 •

edited

coveralls commented Feb 17, 2024 •

edited

juanmahidalgo left a comment

juanmahidalgo Feb 22, 2024

LautaroPetaccio Feb 22, 2024

juanmahidalgo Feb 22, 2024

LautaroPetaccio Feb 22, 2024

juanmahidalgo Feb 22, 2024

LautaroPetaccio Feb 22, 2024

juanmahidalgo Feb 22, 2024

LautaroPetaccio Feb 22, 2024

juanmahidalgo Feb 22, 2024

LautaroPetaccio Feb 22, 2024

juanmahidalgo Feb 22, 2024

LautaroPetaccio Feb 22, 2024

juanmahidalgo Feb 22, 2024

LautaroPetaccio Feb 22, 2024

juanmahidalgo Feb 22, 2024

LautaroPetaccio Feb 22, 2024

juanmahidalgo Feb 22, 2024

LautaroPetaccio Feb 22, 2024

juanmahidalgo Feb 22, 2024

LautaroPetaccio Feb 22, 2024

		@@ -0,0 +1,13 @@
		import { t } from 'decentraland-dapps/dist/modules/translation'

Feat: Add Mint name with crypto modal #2140

Feat: Add Mint name with crypto modal #2140

Conversation

LautaroPetaccio commented Feb 17, 2024

vercel bot commented Feb 17, 2024 • edited

coveralls commented Feb 17, 2024 • edited

juanmahidalgo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented Feb 17, 2024 •

edited

coveralls commented Feb 17, 2024 •

edited