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

feat: included AddTokenReview in component ManageTokensModal #1503

Conversation

AntonioVentilii-DFINITY
Copy link
Collaborator

Motivation

We add the component AddTokenReview (specific to Ethereum network) in the generic modal ManageTokensModal.

Changes

  • Copied function save from Ethereum component AddTokenModal and pasted it in ManageTokensModal.
  • Created new variables erc20Metadata and erc20ContractAddress to pass to AddTokenReview.
  • Included AddTokenReview and created conditional rendering based on network.

@AntonioVentilii-DFINITY AntonioVentilii-DFINITY marked this pull request as ready for review June 20, 2024 14:14
@AntonioVentilii-DFINITY AntonioVentilii-DFINITY requested a review from a team as a code owner June 20, 2024 14:14
@@ -90,6 +100,69 @@
}
};

let erc20Metadata: Erc20Metadata | undefined;

const saveErc20Token = async () => {
Copy link
Member

Choose a reason for hiding this comment

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

If we are already there, I would rather like to have this in a service or at least everything we can extract. Or are you planing to refactor that afterwards?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i was planning to do it afterwrds actually, so that we can move step by step, it seems the best way to have smaller PRs

what do you think? I refactor first?

@AntonioVentilii-DFINITY
Copy link
Collaborator Author

You are right, better refactor first... closing this

@AntonioVentilii-DFINITY AntonioVentilii-DFINITY deleted the feature/chain-fusion/include-ethereum-add-token-flow-in-modal branch June 20, 2024 15:58
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