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

Validation errors aren't diplayed on form load #35

Closed
igortg opened this issue May 14, 2015 · 10 comments
Closed

Validation errors aren't diplayed on form load #35

igortg opened this issue May 14, 2015 · 10 comments

Comments

@igortg
Copy link

igortg commented May 14, 2015

I have an application where form submissions aren't done explicit by the user, but done "under the hood" while user switch between forms. In this way, user could leave a view with an invalid form data: the data will not be posted, but it's kept alive on the $rootScope.

The problem is when user go back to the invalid form: validation errors are not displayed unless the field is "touched".

I solve the problem by implementing a directive that watch the form $validationSummary attribute, and call checkFormValidity when $validationSummary is filled. See below

app.directive('validateOnload', ['validationService', function (validationService) {
    return {
        restrict: 'A',
        link: function (scope, element, attrs) {
            var validator = new validationService()

            var deregisterCheckForm = scope.$watch('form.$validationSummary', function () {
                if (scope.form.$validationSummary) {
                    validator.checkFormValidity(scope.form)
                    deregisterCheckForm()
                }
            })
        }
    }
}])

I wonder if this behaviour shouldn't be added to the library as an option. What do you think?

@ghiscoding
Copy link
Owner

Hmm I see where you are getting at but the main problem I see is that Angular-Validation was designed with the concept of not bothering the user with invalid error message unless you call the checkFormValidity() as you found out. Perhaps what could be possible is to implement your watcher function in the Service as a new watcher and possibly pre-defined is as Global Options, I was actually looking to add more options to the globalOptions (it seems alone with just the debounce as globalOptions haha), so that would be a nice addition :)

I see it possibly like the following:

// new function inside validationService
// the True argument would mean watch it or False to unwatch it
function watchValidationSummaryChanges(true) { 
    // your watcher
} 

The you can use the GlobalOptions of the Directive

myApp.controller('Ctrl', function ($scope) {
  $scope.$validationOptions = { 
    debounce: 1500,
    watchValidationSummaryChanges: true  // the name is probably too long, please find another name
  }; // set the debounce globally
});

or from the Service

myValidation
    .setGlobalOptions({ debounce: 1500, scope: $scope, watchValidationSummaryChanges: true })

That could be a nice addition, as long as we stick with default being not watching since I really prefer the fact that the user is not bother with a ton of error messages right from the start (unless he click on the submit button which we can then call checkFormValidity()).

It shouldn't be too hard to look at editing the code if you wish, inside the Service validation-service.js file, there is already a watcher in the Service which you can refer too for your changes, look at the function removeWatcher() for reference, it should be fairly easy to understand and then add your public function to the validationService.prototype.watchValidationSummaryChanges on top just before the return validationService;

So yes I would be happy to accept code change, any type of improvement or new feature is always welcome. We can add it in the Wiki after that. Thanks :)

@igortg
Copy link
Author

igortg commented May 14, 2015

Nice,

I'll find some time to work on that this next week. Thks!

@ghiscoding
Copy link
Owner

Cool I will probably push some code changes this weekend, I am working on adding a full suite of End-to-End tests with Protractor and with them running I have done couple of minor tweaks to my code. Next week would be a good timing for latest code.

Thanks again!

ghiscoding added a commit that referenced this issue May 30, 2015
- Added enhancement #35 - PreValidate the Form, display all errors on
page load.
@ghiscoding
Copy link
Owner

I believe this is what you wanted to do... check the PreValidate Form (on page load) new functionality in v1.3.26

Let me know if this what you wanted and is working for you.
Thanks :)

@igortg
Copy link
Author

igortg commented Jun 2, 2015

Nice! Works perfectly.

I saw that you "touch" all the elements if preValidateFormElements == true. This is way better. My solution was somewhat hackish and since it relies on a $watch, errors were shown with some inconvenient delay after the form was loaded.

Thanks

@igortg
Copy link
Author

igortg commented Jun 2, 2015

Ghis,

I think there is a minor problem with the PreValidate Form implementation. It's easy to reproduce:

  1. Enable preValidateFormElements using scope validationOption attribute
  2. Type an invalid value for a numeric field (error message is shown)
  3. Type a valid value for the same field (error message is hidden)
  4. Leave the field (change focus)
  5. Focus on the field again. Last error message will reappear (but the data is still valid)

I could reproduce it by just enabling preValidateFormElements in the sample available on Plunker

@igortg
Copy link
Author

igortg commented Jun 2, 2015

One more issue:

$scope.$validationOptions = {
    preValidateFormElements: false
}

Does not work as expected, since you aren't checking for truthness. You just check for $validationOptions.hasOwnProperty('preValidateFormElements')

Cheers

@ghiscoding ghiscoding reopened this Jun 2, 2015
@ghiscoding
Copy link
Owner

Hello, I actually found some implementation issues after adding a Protractor test, I made some changes but I did not push the changes yet.

On line 524 to 531 of validation-common.js, you can replace it with this

// if user is pre-validating all form elements, display error right away
if (!!self.validatorAttrs.preValidateFormElements || (!!self.scope.$validationOptions && !!self.scope.$validationOptions.preValidateFormElements)) {
  // make the element as it was touched for CSS, only works in AngularJS 1.3+
  if (!!formElmObj && typeof self.ctrl.$setTouched === "function") {
    formElmObj.ctrl.$setTouched();
  }
  // only display errors on page load, when elements are not yet dirty
  if(self.ctrl.$dirty === false) {
    updateErrorMsg(message, { isSubmitted: true, isValid: isFieldValid, obj: formElmObj });
  }
}

Let me know if it gives you better results. Thanks

ghiscoding added a commit that referenced this issue Jun 3, 2015
- Added possibility to use own isolated scope (issue #37 and #26).
- Fixed an implementation issue found from last revision (issue #35).
- Fixed email validation (issue #38).
- Fixed a performance issue found with onBlur which would run as much
validations as there was characters inside the input when onBlur was
called (abcdef => would make 6 validations) and this was extremely
costly with a Remote validation call.
- Update the code of Remote validation to also accept the "As" alias
"remote:vm.customRemoteValidation".
- Finally added and updated a few Protractor tests to cover all of the
above and more.
@ghiscoding
Copy link
Owner

I believe that it's all fixed now, I modified my main demo page, if you go in the "2 Forms" section, you will see it effective and then start typing and focus in or out will work as intended. I made the first implementation a little too quick, now this time I added a Protractor test (I use the "2 Forms" page) so I cover it all. I updated the Plunker Live Demo also, so you can see it there too.

So please use the version 1.3.27
Let me know if you find any other errors
Thanks

@igortg
Copy link
Author

igortg commented Jun 5, 2015

Worked just fine now.

Thanks

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

No branches or pull requests

2 participants