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(forms): the min and max validators should work correctly with 0 as a value #42412

Closed

Conversation

iRealNirmal
Copy link
Contributor

Resolved min and max validator issue as it wasn't working when value was set to 0.

Partially closes #42267

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Min and max validators are not working when it's set to 0.

Issue Number: #42267

What is the new behavior?

min and max validator will work when value is set to 0.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@google-cla google-cla bot added the cla: yes label May 29, 2021
@pullapprove pullapprove bot requested a review from AndrewKushnir May 29, 2021 05:00
@iRealNirmal iRealNirmal force-pushed the forms-validator-fix-minmax branch 3 times, most recently from c72296d to e0ba47b Compare May 29, 2021 06:53
@iRealNirmal
Copy link
Contributor Author

@AndrewKushnir PR is ready for hot fix of min and max validator. Let me know if you need me to include anything else in this PR

@AndrewKushnir AndrewKushnir added action: review The PR is still awaiting reviews from at least one requested reviewer area: forms target: patch This PR is targeted for the next patch release labels Jun 2, 2021
@ngbot ngbot bot modified the milestone: Backlog Jun 2, 2021
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @iRealNirmal. I've left a comment regarding tests, please have a look when you get a chance.

I'd also propose to update the commit message to provide a bit more info on the problem (especially the title of the commit message that'd be included into the changelog):

fix(forms): the `min` and `max` validators should work correctly with `0` as a value

Prior to this change the `min` and `max` validator directives would not set the `min` and `max` attributes on the host element. The problem was caused by the truthy check in host binding expression that was calculated as `false` when `0` is used as a value. This commit updates the logic to leverage nullish coalescing operator in these host binding expressions, so `0` is treated as a valid value, thus the `min` and `max` attributes are set correctly.

Partially closes #42267.

@iRealNirmal iRealNirmal force-pushed the forms-validator-fix-minmax branch 2 times, most recently from e89a65b to 734e98f Compare June 2, 2021 13:12
… `0` as a value

Prior to this change the `min` and `max` validator directives would not
set the `min` and `max` attributes on the host element. The problem was
caused by the truthy check in host binding expression that was
calculated as `false` when `0` is used as a value. This commit updates
the logic to leverage nullish coalescing operator in these host binding
expressions, so `0` is treated as a valid value, thus the `min` and
`max` attributes are set correctly.

Partially closes angular#42267
@iRealNirmal iRealNirmal changed the title fix(forms): resolved min and max validator issue fix(forms): the min and max validators should work correctly with 0 as a value Jun 2, 2021
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the feedback @iRealNirmal, the changes look good 👍

I've started tests in Google's codebase and will let you know how it goes.

@AndrewKushnir AndrewKushnir added action: presubmit The PR is in need of a google3 presubmit and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jun 3, 2021
@AndrewKushnir
Copy link
Contributor

@iRealNirmal FYI tests in Google's codebase went well, I'm adding this PR to the merge queue. Thanks for the fix 👍

@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker and removed action: presubmit The PR is in need of a google3 presubmit labels Jun 3, 2021
jessicajaniuk pushed a commit that referenced this pull request Jun 3, 2021
… `0` as a value (#42412)

Prior to this change the `min` and `max` validator directives would not
set the `min` and `max` attributes on the host element. The problem was
caused by the truthy check in host binding expression that was
calculated as `false` when `0` is used as a value. This commit updates
the logic to leverage nullish coalescing operator in these host binding
expressions, so `0` is treated as a valid value, thus the `min` and
`max` attributes are set correctly.

Partially closes #42267

PR Close #42412
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: forms cla: yes target: patch This PR is targeted for the next patch release type: bug/fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression introduced by new min max validators
2 participants