Skip to content
This repository has been archived by the owner on Jul 1, 2020. It is now read-only.

Updated from 1.3.26 to 1.3.27 yesterday - tab out of required field leaves red border. #48

Closed
rluiten opened this issue Jul 16, 2015 · 14 comments

Comments

@rluiten
Copy link

rluiten commented Jul 16, 2015

Just discovered this now.
Loaded our Sign in form, which has validation="required" for both username and password fields.

On page load, click in user name field type a value hit tab or click out of field the blue border turns red not green, as if it has an error but does not display the "Field is required." message. So it think its got an error but not the message. This occurs with 1.3.35, i just rolled back to 1.3.25 does not do it.

Appears to be a regression.
I have not created a working example yet, I hope to create a Plunker after I finish this other task.

On first form load, click in field type, form field goes green, tab out it is adding ng-invalid to classes and it goes red but no message, if i click in the form field again and then tab out again it clears it up and its fine and green. I am currently trying to reproduce in a plunker :) not having much luck yet.

@rluiten
Copy link
Author

rluiten commented Jul 17, 2015

Documenting what I have learned so far.
Some notes for a specific field on the form showing the behaviour.

works fine in v1.3.26 shows problem in v1.3.27
on load v1.3.26

<input class="form-control margin-top-5 ng-pristine ng-untouched
 ng-invalid ng-invalid-validation" type="text" ng-model="vm.username" 
 placeholder="Username" name="username" validation="required" 
 id="username" debounce="10">

click in type something tab out v1.3.26

<input class="form-control margin-top-5 ng-dirty ng-valid-parse ng-touched 
 ng-valid ng-valid-validation" type="text" ng-model="vm.username" 
 placeholder="Username" name="username" validation="required"
 id="username" debounce="10">

all ok.

on load v1.3.27

<input class="form-control margin-top-5 ng-pristine ng-untouched 
 ng-invalid ng-invalid-validation" type="text" ng-model="vm.username" 
 placeholder="Username" name="username" validation="required" 
 id="username" debounce="10">

click in type something tab out v1.3.27 shows red border no error message

<input class="form-control margin-top-5 ng-dirty ng-valid-parse
 ng-invalid ng-invalid-validation ng-touched" type="text"
 ng-model="vm.username" 
 placeholder="Username" name="username" validation="required" 
 id="username" debounce="10">

click in and click out again v1.3.27 now shows green

<input class="form-control margin-top-5 ng-dirty ng-valid-parse 
 ng-touched ng-valid ng-valid-validation" type="text"
 ng-model="vm.username" 
 placeholder="Username" name="username" validation="required" id="username" debounce="10">

I still have not reproduced behaviour in a plunker which is getting frustrating.

@ghiscoding
Copy link
Owner

I doubt it would be the version 1.3.32, there is only some regular expression rule changes for accepting the '.' notation for all the date validators, it's mainly just changes in the angular-rules.js file.
By the way, you can see each and every single changes by clicking on the version number inside the changelog, it will open the commit changes and you can investigate.

I had similar problem to what you described, but I don't remember what it was since I fixed it (or I thought I did).
What are you config? Do you have multiple controllers? Multiple forms? Are you using the Directive or the Service? What is your AngularJS version? Do you have anything set in the controller related to angular-validation, for example any global validation config?

It would really help to replicate the bug, so that I could investigate. You could try modifying the Plunker - Live demo that I have.

I'm almost done with the ControllerAs syntax, I would rather fix this issue if you find it. So I'll wait a little before pushing it.

I see you live in Brisbane, I went to Sydney last year to visit my best friend and I love it there, if only I could get the PR (seems hard though), I would move. Summer is too short in Canada haha.

@rluiten
Copy link
Author

rluiten commented Jul 17, 2015

I updated my notes it wasn't 1.3.32 it was a red herring, it was the transition from 1.3.26 to 1.3.27 looks like.

I have been trying to reproduce in plunker outside our environment no luck yet.
One controller, one form, angular 1.3.13, just using the validation="required" directive.
It is not a very complex case really.

I have another issue that has come up i need to spend the next hour or two on, and hope to get back t his again after that.

@ghiscoding
Copy link
Owner

There was a few changes in 1.3.27 as you can see by clicking on it.

Inside the /src/ folder, there is 4 files that you can use which are unminified files and are meant to debug any issues that might be found in Angular-Validation. Just make sure to load the validation-directive.js before the other 3 files since it's a dependency.

What I would ask you for helping to debug is to update to version 1.3.27 and from your index file, load these 4 source files. Then make sure you still get the problem and then, try overwriting the validation-directive.js by the one found in 1.3.26. I have a feeling that it might be the change in the onBlur, if that doesn't make any difference, try overwriting the validation-common.js and so on... but I'm guessing it's the directive one, the blur caused me problem in the past. I can't do a full reset of the onBlur since it would break the onBlur of the user if he has defined one too.

Would you have an onBlur function by any change on the same field? If you have an ngClick or something else bind to the same input? If so I'd like to know what it is.

@rluiten
Copy link
Author

rluiten commented Jul 17, 2015

Canada nice place to visit, not sure I'd want to live there :).

I have now tried with the 4 file version of 1.3.26 no problem.
I have now tried with the 4 file version of 1.3.27 get the problem fairly regularly, I will point out some times it does not happen, even with the full minimized version some times the red box does not happen as well and things are right it goes green. Just reload the page and try again get the red box on tabbing out of the input field or clicking out of it.

I have now tried 1.3.27 but with the directive from 1.3.26 and do not get the problem, so I do think your suspicion of onBlur seems the likely area.

The field we have it on has not got any click or onblur functions.
The declaration is as follows, nothing tricky going on

<input class="form-control margin-top-5" type="text"
                   ng-model="vm.username" placeholder="Username"
                   name="username" validation="required" id="username" debounce="10" />

@rluiten rluiten changed the title Updated from 1.3.25 to 1.3.35 yesterday - tab out of required field leaves red border. Updated from 1.3.26 to 1.3.27 yesterday - tab out of required field leaves red border. Jul 17, 2015
@ghiscoding
Copy link
Owner

Well at least we know which file it is, but without any ways of reproducing the error I can't really help you on my side. I can guide you but can't change the code myself. So you will have to debug it yourself with version 1.3.27 installed (including the validation-directive.js of 1.3.27)

Try debugging following these steps, test after each one, don't do them all in 1 shot else we can't find the problem

1- try removing the debounce="10" you have on the input, test it, see if it makes a difference.
2- inside the onBlur function, there is a new way of getting the value with event.target.value, can you write another line of code with console.log(event.target.value); to see if you always get the value. If not then let me know.
3- if no luck, try replacing the onBlur handler function like it was in previous version, take the previous code of that onBlur and put it back inside the function attemptToValidate(), test it...
4- I'm assuming you already found the problem with previous steps, but if you didn't then there's only the waitingLimit variable left, you can try to undo the changes of that and see what happen.

Also, do you have jQuery (which version)? and what browser do you test with? have you tried other browser? I usually test with Firefox/Chrome and sometimes IE

@rluiten
Copy link
Author

rluiten commented Jul 19, 2015

Ok its Monday morning, I can’t reach work and the code remotely or I would have looked at this sooner.

Occurs with following…
Chrome 43.0.2357.81 m
Firefox 36.04

Does not occur with IE11.0.9600

jQuery version is v2.1.4
Removing “debounce” with validation v1.3.27 does not appear to fix it

Bit of extra learning, if I type characters and leave field quickly it does not happen, this is for both Chrome and Firefox version mentioned above. [works in ie]

If I type some chars wait a while and leave field I get the red border.
I believe debounce is the delay to internal validation, when I removed it this behaviour became more obvious. I suspect it occurs only if leaving field after the internal delay and its validation has run, i made debounce 5 seconds and the red box on leaving does not happen until i wait for that time frame before leaving.

@rluiten
Copy link
Author

rluiten commented Jul 19, 2015

Moving the 1.3.27 blur handler out of attemptToValiate() causes the problem all the time leaving field – not a surprise at the moment.

Putting the 1.3.26 blur handler into v1.3.27 outside attemptToValiate() calls $setValidity() not attemptToValidate() on next context after blur fixes the problem – not a surprise to me again as blue handler can only set validity nothing else then.

Added the console.log() to 1.3.27, the event.target.value always seems to be set correctly.

It appears the problem is triggered by leaving the field after the debounce delay that causes the red box it seems at the moment.

As a hack in v1.3.35, I added a flag to validation-directive called ‘timerValidationSetValidation’ and set it to false initially, set it to true if the timer $setValidation call occurs, and then in the ‘blur’ handler specifically do nothing if set. This appears to address this issue – not surprising, however this is not a solution as it seems calling attemptToValidate() in the ‘blur’ is causing the wrong state to set if the timer had all ready called $setValidity(). I don't like this hack, so not going anywhere with it for now.

Thanks for all your input so far, it has helped me learn more about not angular but also our current behaviour.

Bit further hackery...

Order of events.
Type something, wait till debounce.
So the $timeout calls $setValidity, at the default waitLimit of 1000
Exit the field, this triggers the blur handler which calls attemptToValidate() which hits the code to invalidate the field explicitly. this is what sets the red box on again.
Then attemptToValidate() exits.
Then the $timeout function executes this time with waitLimit of 0 [because it came via blur handler which passes 0] but it is not resetting validity.

Ok as a test, I changed the called to attemptToValidate() to a delay of 10, and that fixes it so red box is not left behind in both Firefox and Chrome.
I do not call this a good but it is not horrible.
We can run with this work around for now.
Let me know if there is anything else you want me to try.
There is a small chance I could make the page available to you to look at in the near future if you have interest, at the moment it is not deployed or deployable anywhere public.

@ghiscoding
Copy link
Owner

Hello again, thanks for doing some troubleshooting :)
Too bad you can't reproduce it outside of your own code, I would have helped more.

Just a couple of things that I'd like to check with you... When you say you're getting the red border, you mean that the field is suppose to be invalid and you're not getting the error text, meaning you just get 1 of the 2, is that it? Or is it that it's suppose to be valid, but the red border stays?
If you are suppose to get the red border and the text, can you inspect the DOM and see if you get the <span> that is suppose to created (by Angular-Validation). I'd like to know if the <span> get created or not.
When you created a new variable named timerValidationSetValidation, it's probably similar to the one I made named formElmObj.isValidationCancelled, I use that one for knowing if I should attempt to validate or not, the reason I use it is because I can't find the best way to cancel the onBlur, so I use that flag. In the past I was resetting the complete onBlur, but some guys complaining in the past (which I completely understand them) that it was breaking their own onBlur, so I created that isValidationCancelled to go over that problem.

When you say adding a delay of 10, do you mean on this line attemptToValidate(event.target.value, 10); is that it? The 0 is there to validate automatically after the user tab away (onBlur), adding a 10 millisecond isn't that much and so I'm ok with that too. But what this also tells me is that we have a delay problem, that often happens between vanilla javascript (or jQuery) versus AngularJS code (which always happen after vanilla js).

At this point, I'm more interested into the delay of 10, could you give more details...

Thanks a lot for all your help

BTW, It's still Sunday night here in Montreal, Canada..hehe.. oh and I did and maintain this library on my personal time, though I started using it at work too, because well, it's too good..hehe
Oh and 1 last thing, I'm waiting to fix this problem to push the ControllerAs code as well.

@rluiten
Copy link
Author

rluiten commented Jul 20, 2015

I removed timerValidationSetValidation hack I did as the timeout modification is a bit simpler at moment. So going back to the current 1.3.35 version.

The field has validation set to required only.

I load page, and click in it, field shows blue outline for focus.
I type a few characters then when the delay has passed [default 1 second] the outline turns green for valid which is correct.
Now when i leave the field by clicking outside or pressing tab the border turns red and stays red.
The field is valid, but it had been marked invalid again by the code, it has the class for invalid attached ng-invalid ng-invalid-validation
If I click back in field it goes back to green and the next time i leave field it stays green.
No message is before or after in these steps.

In v1.3.35 if i modify, the last parameter to attemptToValidate() to 10 my symptoms go away.

        elm.bind('blur', blurHandler = function(event) {
          // get the form element custom object and use it after
          var formElmObj = commonObj.getFormElementByName(ctrl.$name);
          if (!formElmObj.isValidationCancelled) {
            // validate without delay
            attemptToValidate(event.target.value, 0);  // <<<--- modify to 10
          }else {
            ctrl.$setValidity('validation', true);
          }
        });

In v1.3.35, in file validation-directive.js
When the blur handler runs due to be leaving the field at line 74 calls attemptToValidate(event.target.value, 0).
This then causes the field to be marked invalid in line 125 as part of the execution of attemptToValidate().
The rest of attemptToValidate() runs and that sets up $timeout with waiting limit of 0 starting at line 149.
Then attemptToValidate() exits.
Due to the zero length timeout pretty much the next thing that happens is the body of the $timeout at line 150 the

scope.$evalAsync(ctrl.$setValidity('validation', commonObj.validate(value, true) ));

So far for me when i modifed the waitLimit to 10 in 'blur' handler it gives a bit of a delay before this last line is run, which means it gets to set the validity correctly.

There is some sort of race condition here.

@ghiscoding
Copy link
Owner

Hmm ok I see, the problem in your case might be related to the scope.$evalAsync and adding a delay helps in the fact that it happens after the $digest is finished. You can see a little bit of explanation on this guys website $scope.$evalAsync() vs. $timeout() In AngularJS and in my code, there's an $evalAsync inside a $timeout, I believe the $evalAsync is checked first, then goes the $timeout, but in your situation it seems to not be the case, so adding a delay is making sure it runs after all the $digest is finished.

Unless you find something else, I'm ok with adding the delay of 10 on line 74 (since it's 10ms, no one will ever see a difference anyway), I'll add not only in my Directive but also into my Service as well so they are exactly the same.

BTW, you have 2 $evalAsync in your text, I guess you made a typo error there.. scope.$evalAsync(scope.$evalAsync(...) ))

Bed time for me, I'll look at this tomorrow and push this fix and the ControllerAs, as long as it fixes all your problems. Again thanks a lot for the help, this is what I like about the community of open source :)

@rluiten
Copy link
Author

rluiten commented Jul 20, 2015

Oops, yes to typo :)

@ghiscoding
Copy link
Owner

Added the delay of 10 in latest version, please test it and confirm.

Thanks again for all the feedback :)

@ghiscoding
Copy link
Owner

Hey I answered you in the other issue #52 and might be the same answer for the problem you had here with the red border... You have to make your red border appear only after it's dirty

This is part of my CSS to do that, I always use the ng-dirty

/* invalid & (dirty or touched) => red -- CSS3 only */
.ng-invalid.ng-dirty:not(:focus),
.ng-invalid.ng-touched:not(:focus) {
    border-color: #e74c3c;
}

/* valid & dirty => green */
.ng-valid.ng-dirty.ng-touched {
    border-color: #2ecc71;
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants