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.

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

Fix: fixing cursor position in case of a bad input or bad pasting #39

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

alexf2
Copy link

@alexf2 alexf2 commented Apr 30, 2018

There was a bug: if you input an illegal character or paste an illegal text the cursor used to move right, whereas the value hadn't had changed. This commit corrects the issue by preserving cursor position on each keyDown and restoring it on bad validation.

Also, some code refining was done: getting ride of "var" declarations, using more "destructuring", using arrow functions and changing React.Component to React.PureComponent.

Also, I revealed more issues, but hasn't figured out how to fix them easily.

  1. Enhancement. DynamicInput has "separator" props, which can be either: dot or comma. It would be good to substitute the separator on incorrect input automatically. I.e. when it is set to comma and user inputs a dot, then accept it as a comma. Now user is unable to input a dot in this case. And vise versa.

This looks a bit frustrating. Users often mistake dot and comma, so auto-substitution would be helpful.

  1. Bug. If "thousand" is used, then the user is unable to input "separator" at the end of the number. User has to input all the digits, then to return cursor back and to input fraction "separator" (dot or comma).

It is inconvenient and confusing. First time I didn't understand what was wrong and why I can't make a fractional part.

UPD. I managed to propose a fix for issue "2" in the last commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant