-
Notifications
You must be signed in to change notification settings - Fork 25
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
#97 Implemented delayed validation by introducing delayedValidation o… #119
Conversation
src/index.ts
Outdated
!input.classList.contains(this.ValidationInputCssClassName) | ||
) { | ||
// When delayedValidation=true, "input" only takes it back to valid. "Change" can make it invalid. | ||
return; |
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.
This needs to return true
or false
.
Do you mind adding a sample page with delayed validation enabled so it's easy to test the behavior? |
@haacked sure, thanks for looking at this. Fixed the return statement and added a sample. Please have a look when the time allows. We'd love to see it in the package to use it in our app. Also below is a gif demonstrating the behavior of new delayed validation. Idea is, it doesn't bother you while you still typing, but only shows up when you finished and focused out/submitted. And if the field is errored and you are back in it to fix it, it will turn back valid as soon as the data becomes valid. In fact, the original lib at https://jqueryvalidation.org/files/demo/ behaves very similarly: |
Oh yeah, the current behavior is annoying. It seems to me the behavior in this PR should be the default as it seems to align with the jquery validation behavior. @dahlbyk any thoughts? |
@haacked If we choose to make this behavior default, I think it would be better to remove the aspnet-client-validation/src/index.ts Lines 1102 to 1104 in a59c47a
Please let me know what you decide and I can take care of it. cheers, |
@andkorsh I agree with your plan. Let’s make it so. |
…nts. Fixed a bug with data-val-event not being respected
@haacked I have updated the PR now and created a "Immediate Validation" sample page that comparing the new default behavior with the old behavior which can now be achieved using cheers! |
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.
Looks good! I need to test it before I approve it.
if ( | ||
!input.dataset.valEvent && | ||
event && event.type === 'input' && | ||
!input.classList.contains(this.ValidationInputCssClassName) | ||
) { |
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.
Minor nit, but I prefer the logical operators in a multi-line conditional to align on the left. Makes it easier to see all the component conditionals.
if ( | |
!input.dataset.valEvent && | |
event && event.type === 'input' && | |
!input.classList.contains(this.ValidationInputCssClassName) | |
) { | |
if (!input.dataset.valEvent | |
&& event | |
&& event.type === 'input' | |
&& !input.classList.contains(this.ValidationInputCssClassName) | |
) { |
Thank you! And appreciate you creating this great package, hope Microsoft will use it and remove the jquery dependency from identity templates one day. |
@andkorsh As discussed in original issue, is the final result still a drop-in replacement? If not, what should I change in my code to upgrade to these bits? |
It is still a drop in replacement and behavior now aligns more with how the
original library behaves.
cheers
…On Sat, Aug 17, 2024 at 3:59 AM lonix1 ***@***.***> wrote:
@andkorsh <https://github.com/andkorsh> As discussed in original issue,
is the final result still a drop-in replacement? If not, what should I
change in my code to upgrade to these bits?
—
Reply to this email directly, view it on GitHub
<#119 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA5YBM6TIGOX5UAWNAVBEODZR4GGFAVCNFSM6AAAAABMHMCRF2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOJUG44TSOBSGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
#97 Implemented delayed validation by introducing delayedValidation option.
The logic is a little bit more complicated than switching validation event to blur as initially discussed in the issue. This implementation doesn't do any validation while the user typing for the first time (i.e. until the first 'change' or 'submit') but once the validation had failed and user focused back into the input, validation switches back to "input" event to remove the error as soon as the data is valid.