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

Pr2232/follow up #2236

Merged
merged 8 commits into from
Jan 20, 2022
Merged

Pr2232/follow up #2236

merged 8 commits into from
Jan 20, 2022

Conversation

alfetopito
Copy link
Contributor

Summary

Follow up to #2232

Screen Shot 2022-01-20 at 09 59 04

  • Fixed remaining percentage
  • Refactored a bit how percentage is calculated
  • Added xDai logo

@alfetopito alfetopito self-assigned this Jan 20, 2022
@alfetopito alfetopito requested a review from a team January 20, 2022 18:01
@github-actions
Copy link
Contributor

  • 🔭 GP Swap: CoW Protocol v2 Swap UI

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.

Great cleanups.

I don't think is from this PR, but I think we should always use this formatter with thousands separators. In your screenshot we can see it without

@alfetopito
Copy link
Contributor Author

Great cleanups.

I don't think is from this PR, but I think we should always use this formatter with thousands separators. In your screenshot we can see it without

Yes, I'll get there :)

@fairlighteth
Copy link
Contributor

Small suggestion after seeing the last screenshot:

  • When 100% of investing available is not utilized, we could instead say:

Note: You will not be able to invest anymore after claiming.

@alfetopito
Copy link
Contributor Author

Small suggestion after seeing the last screenshot:

* When 100% of investing available is **not** utilized, we could instead say:

Note: You will not be able to invest anymore after claiming.

@biocom Like this?
Screen Shot 2022-01-20 at 11 55 19

@alfetopito
Copy link
Contributor Author

I've applied the change as suggested. If we want to get back the remaining % just revert the commit 10b8a86

@alfetopito alfetopito merged commit b4ea095 into develop Jan 20, 2022
@alfetopito alfetopito deleted the pr2232/follow-up branch January 20, 2022 22:17
@fairlighteth
Copy link
Contributor

@alfetopito I actually meant to have a conditional note:

  • 0% / nothing invested -> Note: You will not be able to invest anymore after claiming.
  • Partial investment -> Use our previous solution with the 'can't invest the XX% remainder..."

But at the end, I like this general note as well, as it covers both scenarios.

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.

3 participants