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

Simplify error messages #2217

Merged
merged 2 commits into from
Jan 20, 2022
Merged

Simplify error messages #2217

merged 2 commits into from
Jan 20, 2022

Conversation

anxolin
Copy link
Contributor

@anxolin anxolin commented Jan 19, 2022

Summary

Follow up on nenad's pr just to simplify the error messages.

Just uses the same message for the cases where you have insufficient balance. It doesn't matter if its for self account or not.
Also reduces the text, imo should be enough, and if we add too much, sometimes they don't even read 😆

@github-actions
Copy link
Contributor

  • 🔭 GP Swap: Gnosis Protocol v2 Swap UI

Initial = 'Insufficient balance to cover the selected investment amount. Either modify the investment amount or unselect the investment option to move forward',
Balance = 'Insufficient balance to cover the selected investment amount, please modify the selected amount to move forward',
MaxCost = 'Selected amount is higher than available investment amount, please modify the input amount to move forward',
InsufficientBalance = 'Insufficient balance to cover the investment',
Copy link
Contributor

Choose a reason for hiding this comment

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

the investment sounds weird, I would say:

Suggested change
InsufficientBalance = 'Insufficient balance to cover the investment',
InsufficientBalance = 'Insufficient balance to cover investment amount',

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.

approve with comment

Balance = 'Insufficient balance to cover the selected investment amount, please modify the selected amount to move forward',
MaxCost = 'Selected amount is higher than available investment amount, please modify the input amount to move forward',
InsufficientBalance = 'Insufficient balance to cover the investment',
SurplusMaxInvestment = `Your investment amount can't be above the maximum investment allowed`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Surplus means something different to me. What about OverMaxInvestment

@@ -22,9 +22,8 @@ import { ONE_HUNDRED_PERCENT, ZERO_PERCENT } from 'constants/misc'
import { PERCENTAGE_PRECISION } from 'constants/index'

enum ErrorMsgs {
Initial = 'Insufficient balance to cover the selected investment amount. Either modify the investment amount or unselect the investment option to move forward',
Copy link
Contributor

Choose a reason for hiding this comment

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

We might want to tells users that they can either modify OR unselect it, which can only be done in the first screen.

Also, you can't change the amount when claiming for someone else, we should distinguish that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We might want to tells users that they can either modify OR unselect it, which can only be done in the first screen.
Also, you can't change the amount when claiming for someone else, we should distinguish that

My thinking is that messages should be shorter and Insufficient balance to cover investment amount works for 100% investments where you don't have enough tokens, or for self claiming.

I'll ask for suggestions, not too strongly opinionated so happy to go either way as long as we keep it short.

Copy link
Contributor

Choose a reason for hiding this comment

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

short is fine by me

@anxolin
Copy link
Contributor Author

anxolin commented Jan 20, 2022

Merging then, @alfetopito feel free to PR on top.

@anxolin anxolin merged commit eca3a88 into claim Jan 20, 2022
@alfetopito alfetopito deleted the error-messages-investment branch February 1, 2022 17:35
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