Refactoring

[jamesbrobb]

If you look at the initial console output of the plunker you can see what's happening

http://plnkr.co/edit/7tWnDtnXfss4v8rkvO0m?p=preview

As each radio button is added to the form, as they all share the same name, they replace one another. So the last radio button added is the one that's retrieved from the formController by form.cheese.

If you select each radio button in order, you can see a side effect of this, as only the last one ('maybe') sets the ngModelController to $dirty.

This issue is fixed by validate-multiple-fields, but i'd like to propose another solution that doesn't require that directive to handle a RadioSelect.

Solution

Firstly rather than using validate-multiple-fields to handle a 'required' group of radio buttons, you can just use the required attribute on each radio button. As they all bind to the same ng-model value, as soon as one is selected and the ng-model value becomes populated, every buttons 'required' validation is satisfied and becomes true.

http://plnkr.co/edit/w0tUBsuxXSNqAkPhNkaB?p=preview

Then to remove the $dirty/$pristine issue, you can wrap the elements in an ngForm of the same name, so that all view/error state is bound to that instead, which is then updated by each input.

http://plnkr.co/edit/WmezNdFd4SU5x44A1uoZ?p=preview

As each radio buttons ngModel is bound to the same property on scope, the resulting data structure still matches the data structure expected by the server.

[jamesbrobb]

This relates to the issue i explained on Friday (#135) about completely removing the 'rejected' error/errors in the validate-multiple-fields directive when validating a required RadioSelect or CheckboxSelectMultiple.

This demonstrates that point, as the 'rejected' error is applied to the ngForm by djangoForms.setErrors but never removed. So the form can never return to a valid state if the 'rejected' error is not explicitly removed from it.

http://plnkr.co/edit/9MqYF7WzZBUN1SJwLDpk?p=preview

Problem

When dealing with an error that relates to an ngForm i don't think that $setValidity should be called on the form directly, to manipulate its state.

djangoForms.setErrors treats an ngForm the same as it treats any field, setting its validity directly. This logic runs on both:

field = form[key];
field.$message = errors[0];
field.$setValidity('rejected', false);
field.$setPristine();

The docs state that the purpose of formController.$setValidity is:

Sets the validity of a form control.

So what's actually occurring here, is that the state of a non-existent control - 'rejected' - is being set to false. Thus causing the form to be invalid.

Solution

By manipulating the error state of the controls instead, you correctly set the ngForms error state. Then once all inputs are interacted with, their 'rejected' error state gets removed and the ngForm is returned to its error free state.

In the plunker, if you follow these steps:

1. Select any option

2. Submit the form

• the rejected state is added

3. Select each option

• the rejected state is removed

http://plnkr.co/edit/OxCv91wfscq1IrVXutoU?p=preview

Now each individual control has it's own 'rejected' state set to false, which causes the 'rejected' error state of the ngForm to become false, so that the error message is displayed. Then as each control is selected, the $viewChangeListener function of each (added by the subfield loop in djangoForms.setErrors) is fired, clearing the error, causing the ngForm to correctly remove it's 'rejected' error once all fields are valid.

Obviously this is not complete, as we need to remove all control 'rejected' errors when any single control is clicked. But i'll come back to that in a minute.

[jamesbrobb]

Problem

Using a $viewChangeListener to remove the 'rejected' error means that the error will only be removed if the input is interacted with. But not if the model data is changed from somewhere else.

http://plnkr.co/edit/Qjf1oLl80r0Fd1nJKBJz?p=preview

1. Select 'Yes'
2. Submit
3. Select 'Set value through scope'

If you look at the console output, you'll see that the $viewChangeListener for 'maybe' has not be fired and you still need to select all 3 radio buttons individually for the 'rejected' error to be removed from the ngForm.

Solution

Instead of adding a $viewChangeListener in djangoForm.setErrors to remove the 'rejected' error, we add a directive to each input that removes the error using the $parsers and $formatters pipelines. The same way angular 1.2 validators (required, min length, maxlength etc) work.

<input name="cheese" ng-model="data.cheese" required djng-rejected type="radio" value="no">

function djngRejected() {
  return {
    restrict: 'A',
    require: '?ngModel',
    link: function(scope, element, attrs, ctrl) {

      if(!ctrl)
        return;

      var validator = function(value) {

        if(ctrl.$error.rejected) {
          ctrl.$message = undefined;
          ctrl.$setValidity('rejected', true);
        }

        return value;
      };

      ctrl.$formatters.push(validator);
      ctrl.$parsers.push(validator);
    }
  }
}

if you follow the same steps as before you'll see that the 'rejected' error is now cleared

http://plnkr.co/edit/SqhRvyDTg4ZR4Y7JJrCw?p=preview

1. Select 'Yes'
2. Submit
3. Select 'Set value through scope'

Due to the way in which ngModel $formatters and $parsers work, the new djng-rejected directive has also solved the previous problem of clearing all rejected errors when a single radio button is selected.

As they are all bound to the same model value on scope - data.cheese - if one radio button is clicked it's 'rejected' error is removed by its $parser, then when the model value changes, the other two automatically fire their $formatters, clearing their rejected error and removing the 'rejected' error from the ngForm.

We've now completely removed the need for the validate-multiple-values directive on a RadioSelect.

[jamesbrobb]

We can go one step further by moving the specific logic to add/remove 'rejected' errors into (namespaced) methods on each ngModel.

djng_forms_module.directive('djngRejected', function() {
    return {
        restrict: 'A',
        require: '?ngModel',
        link: function(scope, element, attrs, ctrl) {

            if(!ctrl || attrs. djngRejected !== '')
                return;

            var validator = function(value) {

                if(ctrl.$error.rejected)
                    ctrl.djngClearRejected()

                return value;
            };

            ctrl.djngClearRejected = function() {
                ctrl.$message = undefined;
                ctrl.$setValidity('rejected', true);
            };

            ctrl.djngAddRejected = function(msg) {
                ctrl.$message = msg;
                ctrl.$setValidity('rejected', false);
                ctrl.$setPristine();
            };

            ctrl.$formatters.push(validator);
            ctrl.$parsers.push(validator);
        }
    }
});

So we're essentially creating an interface for adding and clearing the rejected messages. For example, the ng.django.angular.messages module uses the 1.3+ $validators to manage it's 'rejected' errors. So it could replace the existing ng.django.forms djng-rejected directive with it's own version that adds a $validator and overrides the two methods to add/remove correctly.

djng_forms_module.directive('djngRejected', function() {

    return {
        restrict: 'A',
        require: '?ngModel',
        link: function(scope, element, attrs, ctrl) {

            if(!ctrl || attrs.djngRejected !== '1.3')
                return;

            var _hasMessage = false,
                _value = null;

            ctrl.$validators.rejected = function(value) {

                if(_hasMessage && (_value !== value)) {

                    _hasMessage = false;
                    _value = null;

                    ctrl.$message = undefined;

                }else{

                    _hasMessage = !!ctrl.$message;

                    if(_hasMessage)
                        _value = value; 
                }

                return !_hasMessage;
            }

            ctrl.djngClearRejected = function() {
                if(ctrl.$message) {
                    ctrl.$message = undefined;
                    ctrl.$validate();
                }
            };

            ctrl.djngAddRejected = function(msg) {
                ctrl.$message = msg;
                ctrl.$validate();
            };
        }
    }
});

Now the existing logic for djangoForm.setErrors and validateMultipleFields does not need to be changed/updated to handle the addition and removal of 'rejected' errors for 1.2 and 1.3+ style validation.

validateMultipleFields has also been modified to handle this, so the validate function now correctly clears every inputs ngModel 'rejected' error when a single checkbox is interacted with. Which in turn removes that $error from the parent ngForm.

[jamesbrobb]

Finally, i've refactored the validate-multiple-fields directive to work with these other updates.

Also (going back to my earlier comment about calling $setValidity()directly on a form) instead of setting $setValidity('required', false) on the ngForm that wraps the inputs, it now changes the error state of each input individually, which is then reflected in the error state of the ngForm. In fact, it no longer needs to know about the ngForm at all.

I'm also wondering if this could be refactored further to completely remove the need for the subFields? As using the method of registering each ngModel to the validate-multiple-fields directive from its own directive, they no longer appear necessary.

Unless there are cases where a direct child ngModel may exist that shouldn't be added? But i'm not sure this would ever occur when managing a group of checkboxes.

[jamesbrobb]

That's everything. Sorry for the sligthly verbose explanation, i just wanted to be clear on why i've made the decisions i have when refactoring this logic.