search

changhuixu / ngx-digit-only

An Angular directive to only allow [0-9] in the input box when typing, pasting or drag/dropping.
https://changhuixu.github.io/ngx-digit-only/
MIT License
161 stars 70 forks source link

Support for negative values #49

Closed krzysztof-zych closed 3 years ago

krzysztof-zych commented 3 years ago

This PR will resolve #34 and add support for negative values

User can specify the negative sign with negativeSign input value (default -).

changhuixu commented 3 years ago

@krzysztof-zych Thank you for your contribution. I will review it this weekend.

changhuixu commented 3 years ago

Thanks a lot for your pull request. Negative values can be complicated. Please see the following screenshot based on your updates.

2021-04-10-07-06-14-Ngx-Digit-Only

The first input value is invalid, which was generated by overwriting contents in selection. The second input could be valid, but I hope that negative values would not be turned on automatically. You can refer to "decimal", which needs to be explicitly turned on.

In one line of code, you used this.min < 0 as a part of a conditional clause. I am afraid that condition could be not guaranteed. Or maybe you have other intention by using that piece of code. In that case, could you tell me more?

Please feel free to continue working on this pull request. Thanks again.

krzysztof-zych commented 3 years ago

Hi @changhuixu Thanks for the review.

The first input is definitely invalid and I believe I fixed it in the last commit. The problem was related to the forecastValue function so please check it also. I have changed some other functions and add more conditions to prevent pasting and typing negative sign in places where they should not be valid.

I do not want to use another flag to explicit say that this particular input has support for negative values, because this.min variable can control it. If we set that variable greater or equal to 0, then we won't be able to put a negative sign in the control.

Thanks again and looking for your comment.

krzysztof-zych commented 3 years ago

Hi @changhuixu Have you found some time to review this PR?

Thanks.

changhuixu commented 3 years ago

Could you pull the latest master branch? I updated an e2e test in order to prevent from breaking some existing applications.

changhuixu commented 3 years ago

I have published a new version v3.1.0. Please give it a try. Also, please let me know if you want a version for Angular v11 specifically.

Currently, the Travis CI is not working due to the transition their domain names or due to other reasons. Thus the demo site is not up-to-date. I am thinking of finding a new way for CI/CD.

changhuixu commented 3 years ago

I also changed the default branch name to "main". FYI.

changhuixu commented 3 years ago

CI/CD is working now. FYI. The demo site is updated.

patrykdawid commented 3 years ago

Also, please let me know if you want a version for Angular v11 specifically.

Hi! I need these changes for Angular v11. Will you be able to make changes also in v2.x?

changhuixu commented 3 years ago

@patrykdawid Could you try v2.3.0? Thanks.

patrykdawid commented 3 years ago

1) v2.3.0 is not visible everywhere, e.g. it is missing here and in the changelog here. 2) I found it on NPM here, but it also behaves differently, e.g. "@2.3.0" is missing from the installation command. 3) This version installs correctly, but doesn't know allowNegatives :(

patrykdawid commented 3 years ago 
1. v2.3.0 is not visible everywhere, e.g. it is missing [here](https://github.com/changhuixu/ngx-digit-only/releases) and in the changelog [here](https://github.com/changhuixu/ngx-digit-only#readme).

2. I found it on NPM [here](https://www.npmjs.com/package/@uiowa/digit-only/v/2.3.0), but it also behaves differently, e.g. "@2.3.0" is missing from the installation command.

3. This version installs correctly, but doesn't know _allowNegatives_ :(

Quick autocorrect :) The property is allowNegatives recognized. Point 1 and 2 valid.

Thanks!