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.

Sign up for GitHub

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

Jump to bottom

Add card actions styled components: Waterfall PR[1] #371

Merged
merged 3 commits into from
Apr 6, 2022
Merged

Add card actions styled components: Waterfall PR[1] #371

merged 3 commits into from
Apr 6, 2022

Conversation

fairlighteth
Copy link
Contributor

WIP part of waterfall PRs

  • Adds styled components for card actions (profile page)

Screen Shot 2022-04-05 at 15 36 42

Screen Shot 2022-04-05 at 15 36 30

@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

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

github-actions bot commented Apr 5, 2022

  • 🔭 GP Swap: CoW Protocol v2 Swap UI

@nenadV91
Copy link
Contributor

nenadV91 commented Apr 6, 2022

If you think its not a good idea please ignore this but maybe we could hide the Add to Metamask text and increase the icon size on the screens less then 390px for example so instead of this
Screenshot from 2022-04-06 11-26-54

We get this
Screenshot from 2022-04-06 11-28-22

nenadV91
nenadV91 approved these changes Apr 6, 2022
View reviewed changes
Copy link
Contributor

@nenadV91 nenadV91 left a comment

Choose a reason for hiding this comment

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

Approved with some minor comments.

@fairlighteth
Copy link
Contributor Author

If you think its not a good idea please ignore this but maybe we could hide the Add to Metamask text and increase the icon size on the screens less then 390px for example so instead of this Screenshot from 2022-04-06 11-26-54

We get this Screenshot from 2022-04-06 11-28-22

Thank you for reviewing this. In general I'm aiming to use explicit text labels whenever I can. Just seeing a MetaMask icon has a bit of ambiguity in expected action on click. Even though you might say the context 'should make it clear that you're adding an icon to MetaMask'. Not clear perhaps for everyone.

That being said, I was looking to reduce the amount of text needed here by changing
Add COW to MetaMask

To simply:
Add to MetaMask

In this case I'm inheriting an existing AddToMetamask component. I first need to modify this and make sure it doesn't break functionality everywhere it's important.

Thank you.

@fairlighteth fairlighteth mentioned this pull request Apr 6, 2022
Merged
@elena-zh
Copy link

elena-zh commented Apr 6, 2022

I have not encountered new issues besides the mentioned ones above (and in this comment)

@fairlighteth @W3stside
Conditional explorer external url: Waterfall PR[2] (#376)
1510630 
* Conditional explorer external url.

* Remove comment

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

* Add to metamask and vcow contract.

* Profile: Buy COW token. Waterfall PR[4]  (#379)

* 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]>

Co-authored-by: dave <[email protected]>

Co-authored-by: dave <[email protected]>
@github-actions
Copy link
Contributor

github-actions bot commented Apr 6, 2022

  • 🔭 GP Swap: CoW Protocol v2 Swap UI

@fairlighteth
Copy link
Contributor Author

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

@fairlighteth fairlighteth merged commit 76a409c into release/1.13 Apr 6, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 6, 2022
@fairlighteth fairlighteth deleted the profile-actions branch July 4, 2024 15:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nenadV91 nenadV91 nenadV91 approved these changes

@W3stside W3stside W3stside approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fairlighteth @nenadV91 @elena-zh @W3stside