fix(input): re-validate when partially editing date-family inputs #13886
Conversation
| var validity = this[VALIDITY_STATE_PROPERTY] || {}; | ||
| var origBadInput = validity.badInput; | ||
| var origTypeMismatch = validity.typeMismatch; | ||
| timeout = $browser.defer(function() { |
There was a problem hiding this comment.
When a keydown event fires the input has not yet changed. This might not be necessary for the other PARTIAL_VALIDATION_EVENTS though. The other option is using keyup but then changes won't be detected if the key is held for a while. Basically the same logic used for the no-input-event case (IE).
|
Definitely simpler. |
|
That is a good point... but I think this is still more change detection then validation logic, it just happens to detect those changes using the |
|
The tests are currently failing on all the non-chrome browsers. Anyone have an opinion for how to fix that? Just put a giant |
| @@ -1918,6 +1918,83 @@ describe('input', function() { | |||
| }); | |||
| }); | |||
|
|
|||
| ['month', 'week', 'time', 'date', 'datetime-local'].forEach(function(inputType) { | |||
| if (jqLite('<input type="' + inputType + '">').attr('type') !== inputType) { | |||
There was a problem hiding this comment.
This fails to detect the supporting browsers. You should use .prop() instead.
The type attribute will always have the requested type, but the type property will be set to text if the browser doesn't support the type.
|
@jbedard, they are expected to fail on browsers that don't support the input type, but the code fails to properly detect support. See https://github.com/angular/angular.js/pull/13886/files#r51847302. |
|
BTW, at the moment only Chrome, Opera and Edge support date inputs. |
jbedard
force-pushed
the
12207-partialdate
branch
from
b27f88a
to
e27bff3
Compare
| // check for validity changes on various DOM events. | ||
| if (PARTIAL_VALIDATION_TYPES[type] && ctrl.$$hasNativeValidators && type === attr.type) { | ||
| element.on(PARTIAL_VALIDATION_EVENTS, function(ev) { | ||
| if (!timeout) { |
There was a problem hiding this comment.
The fact that we are reusing the same timeout with the else brach above, is not good I think.
Since the callbacks are conditionally calling listener, there might be a timeout registered, so we don't register another one, but that other timeout's callback may end up not calling listener (while our callback would).
There was a problem hiding this comment.
I've had that concern as well for a while now with keydown vs cut/paste events in the IE case (no input event).
But in this case, with current browser date-like input support, I don't think there will be any issues. I don't think there is any browser that will not have the input event but will have native date-like inputs.
I do think this should change at some point though, even if just for the IE case...
There was a problem hiding this comment.
True. Having separate timeouts would still be "cleaner", but even the shared timeout isn't indeed expected to create any issues.
|
A couple of minor comments (that could be ignored as well) 😃 |
jbedard
force-pushed
the
12207-partialdate
branch
from
e27bff3
to
ab031d6
Compare
This checks for changes to the badInput/typeMismatch validity flags on keydown/wheel/mousedown events to try to mark date-like inputs as $dirty when they have been partially entered but are still incomplete/invalid.
This has a couple advantages/disadvantages compared to #12902 explained at #12902 (comment).
Fixes #12207