-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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 (input): ng-touched applied to ion-input on blur #12315
Conversation
@@ -229,6 +229,7 @@ export class BaseInput<T> extends Ion implements CommonInput<T> { | |||
this._form && this._form.unsetAsFocused(this); | |||
this._setFocus(false); | |||
this.ionBlur.emit(this); | |||
this._onTouched && this._onTouched(); | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for submitting this PR.
It seems like this is only half of the fix. This will set ng-touched on blur, but it does not prevent touched from being applied on keyup (see #12102, #12700). I believe this same call also needs to be removed from onChange()
on line 257.
I will need to ask around here about that, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kensodemann You're right.
If we look at the Angular implementation this is the expected behavior.
I'll change that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@frontendbeast made an example about it http://plnkr.co/edit/8NtRId4YGbhXTHcWOMfV?p=preview
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After reviewing this with @manucorporat, this change is a little deeper than this. As we have it coded right now, it will have a negative impact on checkboxes and a couple of other inputs. We discussed solutions, which I will try to get around to implementing over the weekend (easier to do than try to explain).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kensodemann 👍
I'll appreciate this fix.
Created PR #12812 which replaces this PR. I will close this one. |
Short description of what this resolves:
Solve the problem of ng-touched is being applied only on keyup
Ionic Version: 3.4.x
Fixes: #12102