Profile: Add COW to Metamask: Waterfall PR[3] #378

fairlighteth · 2022-04-05T17:30:10Z

Summary

Adds the 'Add COW to Metamask' button
Adds the 'Contract' external url for vCOW.

github-actions · 2022-04-05T17:30:22Z

CLA Assistant Lite:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.

I have read the CLA Document and I hereby sign the CLA

_{You can retrigger the CLA Action by commenting recheckcla in this Pull Request}

github-actions · 2022-04-05T17:37:54Z

🔭 GP Swap: CoW Protocol v2 Swap UI

elena-zh · 2022-04-06T10:34:46Z

Hey @fairlighteth , great changes!
Some notes from my side:

'Added COW' message is shifted towards all the rest links on the widget
'Added COW' button is not changed to 'Add to MM' when switch accounts/change networks: https://watch.screencastify.com/v/0Dl9oGUP05nFEepQ7SiU
It is an known issue, but I will report it once again: COW token is added with a broken icon to MM

Besides, it shows 'Added COW' message when I have not added the token to MM (cancelled it request)

Thank you!

fairlighteth · 2022-04-06T10:43:06Z

@elena-zh
Thanks for reviewing. Some comments:

This should already be fixed in one of the later waterfall PRs.
Can you double check? I've tested it like this:

I connected with MetaMask: Shows 'Add Token' component
I switch to WalletConnect (Gnosis Safe) -> Add token is changed for the 'copy contract'. At least this happens on profile-actions-6 (a later waterfall PR...)

This is outside this PR, but something we should fix for sure. I have created a PR on MetaMask (Add (v)COW tokens for Ethereum Mainnet. MetaMask/contract-metadata#1024) that got merged. That hopefully should soon fix this.

elena-zh · 2022-04-06T10:43:37Z

BTW, I noticed, that 'Add to MM' button is displayed in 1Inch wallet.

I assume, that it might be related to the existing issue #322 , however, would be nice to fix, as the button does not work in this wallet.

fairlighteth · 2022-04-06T10:50:43Z

@elena-zh That's definitely an issue. In this case I'm inheriting the same component. So perhaps we update the original issue, saying that the component is also used on the Profile page.

elena-zh · 2022-04-06T11:13:40Z

Can you double check? I've tested it like this:
I connected with MetaMask: Shows 'Add Token' component
I switch to WalletConnect (Gnosis Safe) -> Add token is changed for the 'copy contract'. At least this happens on profile-actions-6 (a later waterfall PR...)

It is a bit another case.

Connect to MM
Add COW token to MM
Switch a network in MM: 'added' is still displayed
Switch an account in MM: 'added is still displayed.

elena-zh · 2022-04-06T11:33:15Z

@fairlighteth , I have noticed that the button does not work in MM integrated browser: I press on it, it shows me that the token in added, but it does not appear in the wallet's token list

Skype_Video.mp4

fairlighteth · 2022-04-06T11:35:14Z

@elena-zh Does it have the same behavior in the transaction modal, where we have this Add To Metamask button? If so, we should add this bug to the other issue.

elena-zh · 2022-04-06T11:46:46Z

@elena-zh Does it have the same behavior in the transaction modal, where we have this Add To Metamask button? If so, we should add this bug to the other issue.

@fairlighteth , I have just retested it.. And it appears that the issue is no longer reproducible.
So, it might have been a MM glitch.
I'm sorry for the false alarm..

fairlighteth · 2022-04-06T11:50:00Z

@elena-zh No worries. Perhaps it is a bug/glitch worth keeping an eye on.

elena-zh · 2022-04-06T12:34:21Z

At least this happens on profile-actions-6

@fairlighteth , I have retested the issue on profile-actions-6 pr, it is still there.

fairlighteth · 2022-04-06T13:23:22Z

@elena-zh We need a separate issue for this, given it touches on the AddToMetamask component.

elena-zh · 2022-04-06T13:29:58Z

@fairlighteth , I have reported #389 issue for this.

fairlighteth · 2022-04-06T14:48:39Z

@elena-zh Thank you 🙏🏼

W3stside · 2022-04-06T17:22:24Z

just the chainid issue and all good

* Buy COW route * Profile: Orange link hover and shorter Add to Metamask label: Waterfall PR[5] (#381) * Orange link on hover. * Add shorter label option. * Profile: Conditional 'Add Token' vs 'Copy contract address': Waterfall PR[6] (#383) * Detect MetaMask: conditional AddToMetaMask button * Click to copy token contract address. * Profile: Make elements responsive: Waterfall PR[7] (#384) * Mobile responsive. * Mobile responsive fix + add copy contract vCOW * Use string directly * Replace 1 by ChainId * Add check * Set default mainnet chain when deconstructing. * Update src/custom/pages/Profile/index.tsx Co-authored-by: dave <[email protected]> * Remove conditional check Co-authored-by: dave <[email protected]> Co-authored-by: dave <[email protected]>

fairlighteth · 2022-04-06T18:44:57Z

Tested and addressed comments. Continuing merging. Feel free to review post-merge.

anxolin

approve

anxolin · 2022-04-07T11:00:05Z

src/custom/pages/Profile/index.tsx

+                Contract ↗
+              </ExtLink>
+              <CopyHelper toCopy={V_COW_CONTRACT_ADDRESS[chainId]}>
+                <div title="Click to copy token contract address">Copy contract</div>


Are we sure we prefer to copy the contract address over sending the user to Etherscan?

In this case it was not added as a preference per se: You've got both options there (View Contract and Copy Contract).

Rather it was a ported action from the COW balance card, where if you are not connected with MetaMask we show you 'Copy Contract' instead of 'Add Token' as a fall back.

Discussed with @elena-zh that it might not make sense to have 'Add Token' (to MetaMask) for vCOW (given it's non transferable anyway..) but might be OK to have the 'Copy contract' action.

Let me know your thoughts.

anxolin · 2022-04-07T11:01:35Z

src/custom/pages/Profile/index.tsx

+              title="View contract"
+              href={getBlockExplorerUrl(chainId, COW_CONTRACT_ADDRESS[chainId], 'address')}
+            >
+              Contract ↗


More than contract, could be View Token and send you to Etherscan Token page?

i.e.

https://etherscan.io/token/0xDEf1CA1fb7FBcDC777520aa7f396b4E015F497aB

https://blockscout.com/xdai/mainnet/token/0x177127622c4A00F3d409B75571e12cB3c8973d3c/token-transfers

Yes that might be a better destination. Agree View Token looks more clear. Will check how it fits with the other labels.

Add to metamask and vcow contract.

eda9bf7

fairlighteth requested review from a team April 5, 2022 17:30

elena-zh mentioned this pull request Apr 6, 2022

1Inch: 'Add token to MM' button is displayed on the Confirm order modal in the integrated browser #322

Closed

elena-zh mentioned this pull request Apr 6, 2022

'Added COW' is displayed when change networks/accounts in MM #389

Closed

elena-zh approved these changes Apr 6, 2022

View reviewed changes

nenadV91 approved these changes Apr 6, 2022

View reviewed changes

fairlighteth merged commit 04c8a11 into profile-actions-2 Apr 6, 2022

github-actions bot locked and limited conversation to collaborators Apr 6, 2022

anxolin reviewed Apr 7, 2022

View reviewed changes

fairlighteth deleted the profile-actions-3 branch July 4, 2024 15:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Profile: Add COW to Metamask: Waterfall PR[3] #378

Profile: Add COW to Metamask: Waterfall PR[3] #378

fairlighteth commented Apr 5, 2022

github-actions bot commented Apr 5, 2022

github-actions bot commented Apr 5, 2022

elena-zh commented Apr 6, 2022

fairlighteth commented Apr 6, 2022

elena-zh commented Apr 6, 2022

fairlighteth commented Apr 6, 2022

elena-zh commented Apr 6, 2022

elena-zh commented Apr 6, 2022 •

edited

Loading

fairlighteth commented Apr 6, 2022

elena-zh commented Apr 6, 2022

fairlighteth commented Apr 6, 2022

elena-zh commented Apr 6, 2022

fairlighteth commented Apr 6, 2022

elena-zh commented Apr 6, 2022

fairlighteth commented Apr 6, 2022

W3stside commented Apr 6, 2022

fairlighteth commented Apr 6, 2022

anxolin left a comment

anxolin Apr 7, 2022

fairlighteth Apr 7, 2022

anxolin Apr 7, 2022

anxolin Apr 7, 2022

fairlighteth Apr 7, 2022

Profile: Add COW to Metamask: Waterfall PR[3] #378

Profile: Add COW to Metamask: Waterfall PR[3] #378

Conversation

fairlighteth commented Apr 5, 2022

Summary

github-actions bot commented Apr 5, 2022

github-actions bot commented Apr 5, 2022

elena-zh commented Apr 6, 2022

fairlighteth commented Apr 6, 2022

elena-zh commented Apr 6, 2022

fairlighteth commented Apr 6, 2022

elena-zh commented Apr 6, 2022

elena-zh commented Apr 6, 2022 • edited Loading

fairlighteth commented Apr 6, 2022

elena-zh commented Apr 6, 2022

fairlighteth commented Apr 6, 2022

elena-zh commented Apr 6, 2022

fairlighteth commented Apr 6, 2022

elena-zh commented Apr 6, 2022

fairlighteth commented Apr 6, 2022

W3stside commented Apr 6, 2022

fairlighteth commented Apr 6, 2022

anxolin left a comment

Choose a reason for hiding this comment

anxolin Apr 7, 2022

Choose a reason for hiding this comment

fairlighteth Apr 7, 2022

Choose a reason for hiding this comment

anxolin Apr 7, 2022

Choose a reason for hiding this comment

anxolin Apr 7, 2022

Choose a reason for hiding this comment

fairlighteth Apr 7, 2022

Choose a reason for hiding this comment

elena-zh commented Apr 6, 2022 •

edited

Loading