Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Important details NFT #53

Merged
merged 3 commits into from
Sep 29, 2021
Merged

Important details NFT #53

merged 3 commits into from
Sep 29, 2021

Conversation

RafilxTenfen
Copy link
Contributor

Add component for Important details at NFT

Created other component to show information about the NFT we could use the same, but there would be a bunch of ternary things in the middle to check if it is NFT and change the text, So it was created a component almost exactly the same

Description

  • Change ImportantDetails to ImportantDetailsErc20
  • Created ImportantDetailsErc721
  • Add function to get information from NFT bridge like the fixed fee property

How to Test

  • Config your .env file for some config that has nftBridge set like rinkeby (4)
  • Run the server

Case 1

  1. Run the server
$~ npm run serve
  1. Click on NFT
    Expected Result
    image

Copy link
Contributor

@marcciosilva marcciosilva left a comment

Choose a reason for hiding this comment

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

lgtm!

data.sideFeeNft = fee / sideConfig.feePercentageDivider
})

const NFT_FIXED_CONFIRMATIONS_BLOCK = 3 // maybe we are going to define it into the nft bridge contract
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd maybe add a Github issue / Jira ticket under 'technical debt' to not lose sight of this comment

@RafilxTenfen RafilxTenfen merged commit bf8c41d into nftbridge Sep 29, 2021
@pmprete pmprete deleted the important-details-nft branch January 14, 2022 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants