This repository was archived by the owner on Apr 12, 2024. It is now read-only.
Repository navigation
This repository was archived by the owner on Apr 12, 2024. It is now read-only.
Inconsistent handling of invalid initial ngModel values #8949
Description
An ngModelController created with an invalid inital value from scope behaves differently depending on input type:
| Type | invalid model | view | behavior | error | good |
|---|---|---|---|---|---|
| text | {} | [object Object] | calls toString | correctly set | |
| url | 'notanurl' | notanurl | calls toString | correctly set | ✅ |
| number | 'notanumber' | (empty) ❌ | throws if model is not type number |
no errors are set because of exception ❌ | ❌ |
| number+min=5 | 3 | 3 | ^ | correctly set | ✅ |
| date | 'notadate' | (empty) ❌ | expects a date and uses the date filte, else returns nothing | required set | ❌ |
| date | new Date('123456') | 0NaN-NaN-NaN | ^ | $error.date is not set ❌ | ❌ |
I think we should always try to set the $viewValue to something. This is important if you have server-side data that may be invalid.
So number and date should not discard invalid values. If we pass garbage to the validators, the correct errors will be set.
Regarding calling toString on text-based input. I think that's okay as you will rarely set an object to a model when you know it'll be displayed in an input. But we should do "the right thing" and try to format values that are good enough, so in short:
- number should not throw and call toString regardless
- date should return call toString if not a Date object
- date should also get the bug fixed where NaN gets output
I'll think I'll be able to throw a PR together over the weekend.
Activity
caitp commentedon Sep 5, 2014
You won't be able to fix a number of these that you're claiming to be "not good" (otherwise we'd have inconsistencies between user specified values and model-specified values)
Narretz commentedon Sep 5, 2014
I admit, I haven't looked into it. Do you mean because of native validators?
caitp commentedon Sep 5, 2014
You can't set a date or number input's values to something which doesn't fit the date or number parsing rules (I'm pretty sure that's the end result of what we do in setViewValue) http://jsfiddle.net/egedyh1v/
Narretz commentedon Sep 5, 2014
Ah, I see what you mean. However, I think I can improve the handling a little bit.
And the last entry, the date, that looks like a bug in the date filter.
Narretz commentedon Sep 5, 2014
Well, yeah I see what you mean. I think the code needs some comments why we're doing this.
12 remaining items
jeffbcross commentedon Sep 30, 2014
Assigned to @tbosch since he's working on #9218, which should resolve most of these issues.
tbosch commentedon Oct 1, 2014
Yes, we should throw if the model has an invalid type. This would do the following:
input[text],input[url],input[email]: throw if the model value is not a string (to be implemented in a formatter).input[date]: throw if the model value is not a date.Invalid Datesin the model (e.g. created vianew Date('1234567')will not throw but result in an empty input with an errorInvalidDateset.input[number]: throw if the model value is not a number.NaNin the model will not throw but result in an empty input with an errorNaNset.@Narretz Does this sound good to you?
Narretz commentedon Oct 1, 2014
I'm not really a fan of throwing in these cases. Just to be clear, the reason is that we don't want the formatting process to change the model? Otherwise we could at least try to "parse" the model. Or is the parsing part what makes this hard?
Anyway, it's definitely a breaking change for text and date. I expect it'll break quite a few people's apps that rely on a number displaying "okay" in a text input.
tbosch commentedon Oct 1, 2014
Ok, talked again with @IgorMinar and we came up with this:
input[date]andinput[number]if the model value is not a date / number, as we have no good way to convert the model value. We don't add validation errors as the user was not responsible for the incorrect types in the model and would rather only be confused.input[text]we keep the old behavior of just converting the value to a string as too many people rely on this (as @Narretz pointed out).fix(input): correctly handle invalid model values for `input[date/tim…
fix(input): correctly handle invalid model values for `input[date/tim…
fix(input): correctly handle invalid model values for `input[date/tim…
fix(input): correctly handle invalid model values for `input[date/tim…
btford commentedon Oct 1, 2014
👏
iamdey commentedon Nov 7, 2014
woops... In my case I assume the user is responsible of his data and allow him to save invalid values. We were able (angular 1.2) to show him what's wrong each time he open the form.
Well I'm not sure how to reproduce previous behavior.
Narretz commentedon Nov 7, 2014
What is "show him what's wrong each time he open the form" supposed to mean? Please elaborate.
iamdey commentedon Nov 7, 2014
Here is the case, the "form" is a meta query, this query cannot be executed until the parameters are not valid (a date or number depend of the parameter's type).
May be the user wants a draft, so we allow him to save whatever he wants. We use ngModel.$invalid to indicate how to fix his query.
So yes, the server must give incorrect values as it has been saved and yes I want to use native input type date/number.
I'm surprised that angular assume the provided values must be correct. May be there is a UX mistake in my design, but I should not be the only one.
Narretz commentedon Nov 7, 2014
Although I still have problems understanding exactly what you are doing, it seems like ngModelOptions.allowInvalid should solve your problems. You should provide an example of what you're doing if this is not the case.
----- Ursprüngliche Nachricht -----
Von: "David Epely" [email protected]
Gesendet: 07.11.2014 16:38
An: "angular/angular.js" [email protected]
Cc: "Narretz" [email protected]
Betreff: Re: [angular.js] Inconsistent handling of invalid initial ngModelvalues (#8949)
Here is the case, the "form" is a meta query, this query cannot be executed until the parameters are not valid (a date or number depend of the parameter's type).
May be the user wants a draft, so we allow him to save whatever he wants. We use ngModel.$invalid to indicate how to fix his query.
So yes, the server must give incorrect values as it has been saved and yes I want to use native input type date/number.
I'm surprised that angular assume the provided values must be correct. May be there is a UX mistake in my design, but I should not be the only one.
—
Reply to this email directly or view it on GitHub.=