jonsamwell/angular-auto-validate

register-custom-form-control not validating via debounce due to missing force flag

Closed this issue · 8 comments

Looking at these lines of code:

setValidationState = debounce.debounce(function () {
     validationManager.validateElement(ngModelCtrl, element);
}, 100);

validateElement normally takes a 3rd parameter to force validation when the particular element is included via register-custom-form-control. But since validateElement is being called from this decorator without a check on the particular element to "force", the element validation doesn't get updated correctly. Specifically, I am using the ui-select angular component and have added register-custom-form-control to it. It seems to be working except in this case. The only time I can get the validation style correct is by pressing my submit button again which causes it to go through the normal validationForm path instead of validateElement.

Any thoughts on how to work around this or to alter the code to fix this?

A solution to this problem could be to not consider a custom form control as the thing that "forces" validation but rather when register-custom-form-control is used, somehow add it to elementTypesToValidate so that it's referred to not as an afterthought...?

I have altered my local code and got this working. I do not have the means to build everything in grunt but I can make a pull request in the next couple days that you can review to merge. Also, there could be some discussion on this.

In a nutshell, I add an addCustomElementToValidate method to validationManager which adds additional node name strings to elementTypesToValidate. I then added a controller to the registerCustomFormControl directive and injected validationManager to it. On this new controller, I added a $scope.registerCustomElement method that gets called from the link function right after frmEl.customHTMLFormControlsCollection.push(element[0]);. Validation is now working on submit and when changing the ngModel. In my ui-select case, I am requiring at least 1 selection in my multi-select. So when I add a selection, errors no longer show and when I remove selections, an error correctly displays. The ui-select directive becomes a div tag when used.

The only drawback here is that the custom element needs to be part of the elementTypesToValidate list AND the customHTMLFormControlsCollection - I'd like to think we can get around this.

Thanks for this - it seems like you have a good handle on the problem! I'll have a think about this one and get back to you but seems like this is a good solution! Thanks!

or replace

                    shouldValidateElement = function (el) {
                        return el && el.length > 0 && elementIsVisible(el) && elementTypesToValidate.indexOf(el[0].nodeName.toLowerCase()) > -1;
                    },

with

                     shouldValidateElement = function (el) {
                        return el && el.length > 0 && elementIsVisible(el) && (elementTypesToValidate.indexOf(el[0].nodeName.toLowerCase()) > -1 || el[0].hasAttribute('register-custom-form-control'));
                    },

I'm running into the same issue. These both sound like good fixes. I'm not super familiar with the code but it seems odd that there is a list of input types that drive the validations. I wonder if long term if there is any better way to track what elements have validations on them. I suppose other libraries require you to add a directive to each of these elements so perhaps there isn't sat the moment.

I think I prefer the second approach. In theory, by adding it to the array as done in the first approach you tell it to remember to validate that element and it validates every element of that type. Additionally it mixes the list of 'stock' input types with custom input types.

The second approach seems simpler and is more in line with the way the patch for validation of custom elements on submit was implemented.

Another thing worth noting is that this property/directive is not documented except outside of comments in the code.

Thanks for the great library and let me know if you guys need any help!

@slupekdev I completely agree with your comments. And I agree that @pjwalmsley's solution makes more sense. Mine was definitely a hack to get something working immediately.

@jonsamwell What are your thoughts? I think the solution listed above should work and is a simple fix that could be applied immediately.

Hi @abobwhite @slupekdev @pjwalmsley !

Sorry I've been slow getting on this! I agree and think @slupekdev's is the best approach and easier to implement. The reason we need a list of types to validate is that a custom control could contain other controls such as buttons which we can safely ignore in the validation phase.

I'll do this fix now and add a few tests to prove the functionality. I'll get on the documentation for it afterwards unless any of you guys want to PR some documentation for it?

Thanks everyone!

Sorry for the delay. Issue fixed in commit 888ecd0 and available on bower V1.15.22

I'll create a new issue for the missing documentation.

Thanks!!!!