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

Delete key not working #21

Closed
mquandalle opened this issue Jan 31, 2017 · 7 comments
Closed

Delete key not working #21

mquandalle opened this issue Jan 31, 2017 · 7 comments
Milestone

Comments

@mquandalle
Copy link
Contributor

Hello,

I've noticed a bug with the Delete key handling. Let's say I have an input field with the number USD 1,000 in it. Let's use | to represent the cursor position.

Current State Key pressed Expected Resulting state
`USD 1, 000` Backspace `USD 1
`USD 1 ,000` Delete `USD 1,

Does that make sense?

@s-yadav
Copy link
Owner

s-yadav commented Jan 31, 2017 via email

@mquandalle
Copy link
Contributor Author

mquandalle commented Feb 1, 2017

Well actually on second though, I would say that the expected behavior is to delete a digit every time Detete or Backspace is pressed, like so:

Current State Key pressed Expected
`USD 1 ,500` Backspace
`USD 1, 500` Backspace
`USD 500` Backspace
`USD 1 ,500` Delete
`USD 1, 500` Delete

This could be implemented by registering an handler onKeyDown to handle event.keyCode 8 (for Backspace) and 46 (for Delete).

What do you think @s-yadav?

@s-yadav
Copy link
Owner

s-yadav commented Feb 2, 2017

Yes I have done the same fix. Will push it with some other changes.

@s-yadav
Copy link
Owner

s-yadav commented Feb 2, 2017

Somehow I feel, it's better to have your first case (that is just changing the cursor position). The second case seems visually wrong and confuses (at least to me).

@s-yadav
Copy link
Owner

s-yadav commented Feb 2, 2017

@mquandalle Can you cross check version 1.1.0-alpha or branch 1.1.0-alpha.

@mquandalle
Copy link
Contributor Author

Version 1.1.0-alpha looks good to me (I've just read to code, not tested yet).

About my second suggestion, maybe this could be an option? At least in French, when the group delimiter is a blank space, moving the cursors doesn't make much sense.

@s-yadav
Copy link
Owner

s-yadav commented Feb 3, 2017

Moving cursor does not feel very odd, but deleting a character not near to cursor feels odd, mainly when doing backspace, and behaviour of backspace and delete should be same just on other direction.
Lets keep the first case. Anyway as long user is able to delete we are ok.

@s-yadav s-yadav added this to the 1.1.0 milestone Feb 3, 2017
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

No branches or pull requests

2 participants