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

Profile: Conditional 'Add Token' vs 'Copy contract address': Waterfall PR[6] #383

Merged
merged 3 commits into from
Apr 6, 2022

Conversation

fairlighteth
Copy link
Contributor

Summary

  • Conditionally shows the 'Add Token' (to MetaMask) component vs. the 'Copy contract' based on library (MetaMask) detection.

WalletConnect with a Safe

Screen.Recording.2022-04-06.at.11.10.13.mov

MetaMask

Screen.Recording.2022-04-06.at.11.10.38.mov

@fairlighteth fairlighteth requested review from a team April 6, 2022 10:13
@github-actions
Copy link
Contributor

github-actions bot commented Apr 6, 2022

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
Copy link
Contributor

github-actions bot commented Apr 6, 2022

  • 🔭 GP Swap: CoW Protocol v2 Swap UI

@elena-zh
Copy link

elena-zh commented Apr 6, 2022

Hey @fairlighteth , great changes!
Could we need to add a 'copy' action to the vCOW card? WDYT?
image

All the rest changes LGTM!

@fairlighteth
Copy link
Contributor Author

@elena-zh Was thinking about it indeed... you think it makes sense? In that case we add the 'Copy token contract' but not the 'Add To Metamask' ? Because the token is non transferable anyway? I think at the very least we add the 'copy token contract'.

@elena-zh
Copy link

elena-zh commented Apr 6, 2022

@fairlighteth , agree with you that there is no need to add 'Add to MM' button to the vCOW card. It is enough to have 'Copy' there with no regard to a type of connected wallet.

* Mobile responsive.

* Mobile responsive fix + add copy contract vCOW
@fairlighteth
Copy link
Contributor Author

Build complete after merging in waterfall PR [7]. Tested and works. Continuing merging.

@fairlighteth fairlighteth merged commit fe65eaa into profile-actions-5 Apr 6, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 6, 2022
Copy link
Contributor

@anxolin anxolin left a comment

Choose a reason for hiding this comment

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

approve

Copy link
Contributor

@W3stside W3stside left a comment

Choose a reason for hiding this comment

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

good

@fairlighteth fairlighteth deleted the profile-actions-6 branch July 4, 2024 15:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants