Skip to content
This repository has been archived by the owner on Jun 24, 2022. It is now read-only.

Improve claim someone #2319

Merged
merged 3 commits into from
Jan 27, 2022
Merged

Improve claim someone #2319

merged 3 commits into from
Jan 27, 2022

Conversation

anxolin
Copy link
Contributor

@anxolin anxolin commented Jan 26, 2022

Summary

Improve a bit the link to the account you claim on behalf of

  • Shorten link (as suggested by @alfetopito )
  • Kill the brackets (as suggested by @elena-zh and @alfetopito )
  • Add copy button as suggested, well add copy button cause when we shorten we are supposed to do that!

Before:
image

After:
image

To Test

  1. Claim of behalf of someone
  2. Test the link, copy the address

@github-actions
Copy link
Contributor

  • 🔭 GP Swap: CoW Protocol v2 Swap UI

@fairlighteth
Copy link
Contributor

Love the improvements, makes it look way better. Some small nits:

  • On mobile the address is not centered, this is due to the parent button element having a min-width setting:

Screen Shot 2022-01-26 at 21 47 59

Screen Shot 2022-01-26 at 21 49 42

I can address these separately in case you want to leave it here.

@W3stside
Copy link
Contributor

can you clean the history please?

@alfetopito
Copy link
Contributor

Also, shouldn't this be pointing to the release branch instead?

@anxolin anxolin changed the base branch from develop to release/1.10 January 27, 2022 10:58
@anxolin
Copy link
Contributor Author

anxolin commented Jan 27, 2022

can you clean the history please?

Also, shouldn't this be pointing to the release branch instead?

All the same thing. It was pointing to develop instead of the release/1.10

@anxolin anxolin requested review from a team January 27, 2022 11:15
@anxolin
Copy link
Contributor Author

anxolin commented Jan 27, 2022

On mobile the address is not centered, this is due to the parent button element having a min-width setting:

I can address these separately in case you want to leave it here.

Yes, the min-width was used just to reserve some space for the message you see when you click.
I removed it for responsive.

@elena-zh
Copy link

Changes LGTM besides the issue @biocom has mentioned above

Nitpick that is not related to the current PR, but anyways: I'd not separate an amount from the vCOW token name when moving to another line
image

@anxolin anxolin merged commit ce73a5b into release/1.10 Jan 27, 2022
@anxolin
Copy link
Contributor Author

anxolin commented Jan 27, 2022

Merging, let me know if you want me to address any other issues.

For your issue Elena, I'll leave it up for @michelbio . I guess I would go for no-wrap for the amount + symbol, happy to give it a try if you want.

@alfetopito alfetopito deleted the improve-claim-someone branch February 1, 2022 17:42
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.

5 participants