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

Improve input field claim #2186

Merged
merged 21 commits into from
Jan 19, 2022
Merged

Improve input field claim #2186

merged 21 commits into from
Jan 19, 2022

Conversation

fairlighteth
Copy link
Contributor

@fairlighteth fairlighteth commented Jan 18, 2022

Summary

Account used: 0x4bD39704a83dCB41B74b550988A832b83d5b9Cb0

Tries the achieve:

  • For the Airdrop row, hide the Price element.
  • Making the investment amount input field editable, binding the value. Will probably need more work.
  • The default value for the value attribute is always 0 (String) based on investedAmount. Prefer to return an empty string so it falls back to the placeholder attribute behavior.

Note

  • Crashes right now when entering a very big number
  • the max attribute is currently not respected. Even when adding the type="number" attribute/value to the Input element. Probably something to do for the onChange handler.
Screen.Recording.2022-01-18.at.12.46.56.mov

@W3stside
Copy link
Contributor

actually why is max written in abbreviated form? can and should just be Invest max possible or Invest maximum possible

@github-actions
Copy link
Contributor

  • 🔭 GP Swap: Gnosis Protocol v2 Swap UI

@fairlighteth
Copy link
Contributor Author

@W3stside The shorter the better (because of a variable balance width/size). But agreed, we could just rename max. to max

Will commit.

@elena-zh
Copy link

@biocom , I was also able to catch the app crash when I entered a period/comma into the input amount (decimal that starts with 0). Btw, impossible to enter '0' into the field
image
image

the same is when I enter letters. special symbols, etc.
image
image

Also, the app is crashed when enter an amount, then clear out the field.
image

@elena-zh
Copy link

Also, it is possible to input a negative amount into the field (I copied and pasted it)
image

@elena-zh
Copy link

I'm not sure if it is related to this PR or not, but it would be great to disable the field and set its value to 100% when claim on behalf, as a user for this case will have to claim 100% amount only for an investment option.
image

@fairlighteth
Copy link
Contributor Author

@elena-zh

I'm not sure if it is related to this PR or not, but it would be great to disable the field and set its value to 100% when claim on behalf, as a user for this case will have to claim 100% amount only for an investment option.

Yes, this is a scenario where we need to enable the disable attribute on the input element.

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.

Approving!

I see there's some issues, however the two PRs michel is working on are mostly to tackle design.

The error for input empty string for example, could be dealt in a future PR. Actually @nenadV91 is working in sth similar, so i think we can ignore for now.

Same as investing on behalf of someone else, Elena you are right! although we will deal in another one.

Copy link
Contributor

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

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

Looks good

One small suggestion and this issue which will likely be fixed by Nenad, but just point out because it's kinda funny
Screen Shot 2022-01-18 at 10 21 31
The USDC amounts are probably not using the right amount of decimals :)
Or it's that issue with Rinkeby USDC used that has 18 rather than 6 decimals 🤷

src/custom/pages/Claim/ClaimsTable.tsx Outdated Show resolved Hide resolved
@anxolin anxolin merged commit 2146760 into claim Jan 19, 2022
@alfetopito alfetopito deleted the claim-styleImprove-11 branch February 2, 2022 21:03
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.

6 participants