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

coin logos in send modal #167

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

thisdotLolu
Copy link
Contributor

No description provided.

Copy link
Contributor

@michaeltout michaeltout left a comment

Choose a reason for hiding this comment

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

I understand the reasoning behind this approach but unfortunately this would cause currencies with titles like btc.michaelchain.vrsc to have the logo of btc, when in reality, they don't actually need to be mapped or otherwise related to btc, which could mislead users. I think a better approach to do this would be to check if a PBaaS currency is mapped to an ethereum ERC20 in src/components/SendModal/ConvertOrCrossChainSend/ConvertOrCrossChainSendForm/ConvertOrCrossChainSendForm.js while processing the suggestion paths, and then if so, set logoid such that getCoinLogo will render the erc20 logo for that currency, in the same way that it would if the currency logo was being rendered in the manage coins list for erc20 currencies.

@thisdotLolu
Copy link
Contributor Author

hello @michaeltout thank you for taking out time, i've been looking at how i can implement this using your suggestion but i think i need some clarification, so when you say a pbaas currency mapped to an erc20 token i believe you mean it's satisfying the condition whereby proto === 'erc20' in this function export const getCoinLogo = (id, proto, theme = 'light') => { if (CoinLogos[id]) return CoinLogos[id][theme] else if (proto === 'erc20') return CoinLogos.ETH[theme] else return CoinLogoIcons.pbaas.RenderPbaasCurrencyLogo(id)[theme] } (please correct me if i am wrong). My issue with this is that, there aren't any object values which provides the info as to whether something has a 'proto' of erc20 or mapped to erc20 in this object(this is one object returned by one of the coins in the send modal-hope it's okay to paste here) {"description": "via Bridge.vETH", "key": "3", "keywords": ["iGBs4DWztRNvNEJBt4mqHszLxfKTNHTkhM", "DAI.vETH"], "logoid": "iGBs4DWztRNvNEJBt4mqHszLxfKTNHTkhM", "right": "0.89579771", "title": "DAI.vETH", "values": {"SEND_MODAL_CONVERTTO_FIELD": "DAI.vETH", "SEND_MODAL_EXPORTTO_FIELD": "", "SEND_MODAL_PRICE_ESTIMATE": {"name": "DAI.vETH", "price": 1.1163234564883833}, "SEND_MODAL_VIA_FIELD": "Bridge.vETH"}} which makes it much harder to satisfy this condition. But another approach i could think of is by checking if the title includes a .vETH string and implement the same thing i have done in this pr with only the condition changing.

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