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: Buy COW token. Waterfall PR[4] #379

Merged
merged 2 commits into from
Apr 6, 2022

Conversation

fairlighteth
Copy link
Contributor

Summary

  • Add the Buy COW internal Link with the outputCurrency parameter using the COW token by chainId.

Screen Shot 2022-04-05 at 18 42 56

Note

  • Might want to set also the inputCurrency (USDC?) and an amount for quicker action. Or as initially proposed, checking the user's balance and pick a token (if === USDC/WETH/ETH...etc).

@fairlighteth fairlighteth requested review from a team April 5, 2022 17:43
@github-actions
Copy link
Contributor

github-actions bot commented Apr 5, 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 5, 2022

  • 🔭 GP Swap: CoW Protocol v2 Swap UI

@elena-zh
Copy link

elena-zh commented Apr 6, 2022

Hey @fairlighteth , great changes!
However, some my notes:

  1. Buy COW link is always underlined
    image

  2. When I press on the Buy COW button, I see the URL is changed. And its parameters aren't cleared out when refresh the page (I can't say that it is an issue)

  3. Important: URL parameters aren't cleared out when I change a network, so a user sees 'Select a token' in From and TO fields
    url when change network

The same issue is reproducible when copy and paste the link to a different browser

  1. As COW Protocol token list was not added to Rinkeby (Add COW token list to Rinkeby #363), I see this hanging message when I'm redirected to the SWAP page
    рфтпы
    It is displayed until I manually press on the Import button.
    (Again, not sure if is an issue)

Thanks!

@fairlighteth
Copy link
Contributor Author

  1. When refreshing the page, it will keep the existing URL. I think this is expected behavior. The thing we should be careful about, is if the URL is properly cleared when switching to a different route.

  2. This sounds like a valid issue. I think we should reset the URL on network change, in case there are any parameters in the URL. Basically so it routes to the /swap page (for example) and clears out any parameters. Could you create an issue for this and reference this PR?

  3. Yes makes sense that this modal is shown. Does the modal work as expected on click of the 'Import' button?

@elena-zh
Copy link

elena-zh commented Apr 6, 2022

@fairlighteth

Yes makes sense that this modal is shown. Does the modal work as expected on click of the 'Import' button?

Yes, it works fine then. I think, the issue will not be reproducible when we will add Cow protocol token list to Rinkeby.

This sounds like a valid issue. I think we should reset the URL on network change, in case there are any parameters in the URL. Basically so it routes to the /swap page (for example) and clears out any parameters. Could you create an issue for this and reference this PR?

I have created #386 issue for this.

Thank you!

Copy link

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

Approved!

…ll 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]>
@fairlighteth
Copy link
Contributor Author

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

@fairlighteth fairlighteth merged commit ba7624a into profile-actions-3 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

@fairlighteth fairlighteth deleted the profile-actions-4 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