Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

fix(input): ignore min/max if they are empty on all input types #12785

Closed
wants to merge 3 commits into from

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented Sep 8, 2015

When the min/max attributes are empty (i.e. attrs.min/max === ''), there should be no min/max validation applied (i.e. all values should be valid wrt min/max). This works correctly for input[number], but not for date input family (input[date/datetime-local/time/week/month]).

In the case on input[number], an empty string for attrs.min/max is translated to undefined for minVal/maxVal and a check for isUndefined(minVal/maxVal) ensures that no min/max validation takes place.
For the data input family, an empty string for attrs.min/max is translated to NaN (by parseDate()), so an additional check (for isNaN(minVal/maxVal)) is required.

Fixes #12363

When the min/max attributes are empty (i.e. `attrs.min/max === ''`), there
should be no min/max validation applied (i.e. all values should be valid
wrt min/max). This works correctly for `input[number]`, but not for date
input family (`input[date/datetime-local/time/week/month]`).

In the case on `input[number]`, an empty string for `attrs.min/max` is
translated to `undefined` for `minVal/maxVal` and a check for
`isUndefined(minVal/maxVal)` ensures that no min/max validation takes
place.
For the data input family, an empty string for `attrs.min/max` is
translated to `NaN` (by `parseDate()`), so an additional check (for
`isNaN(minVal/maxVal)`) is required.

Fixes angular#12363
@Narretz
Copy link
Contributor

Narretz commented Sep 8, 2015

Couldn't we check the emptiness inside parseObservedDateValue and return undefined in that case, too? It'd be one check less for the actual validator to make.

@Narretz Narretz added this to the 1.4.6 milestone Sep 8, 2015
@gkalpak
Copy link
Member Author

gkalpak commented Sep 8, 2015

Good point, @Narretz. Updated.

@Narretz
Copy link
Contributor

Narretz commented Sep 9, 2015

LGTM after squashing the commits!

@gkalpak
Copy link
Member Author

gkalpak commented Sep 9, 2015

Thx @Narretz ! (I just added the extra commits for "reviewability".)

Squashing and merging...

@gkalpak gkalpak closed this in 544001f Sep 9, 2015
@gkalpak gkalpak deleted the fix-input-empty-min-max branch September 9, 2015 13:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants