Skip to content

Commit

Permalink
Required type validation only worked for the last checkbox.
Browse files Browse the repository at this point in the history
  • Loading branch information
Caleb Kniffen committed Jun 8, 2015
1 parent f938531 commit 08d4fbe
Showing 1 changed file with 35 additions and 0 deletions.
35 changes: 35 additions & 0 deletions src/types/multiCheckbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ export default ngModule => {
valueProp: c.string.optional
})
},
defaultOptions: {

This comment has been minimized.

Copy link
@ckniffen

ckniffen Jun 8, 2015

Owner

This was done to prevent ngModelAttributesTemplateManipulator from adding ng-required or required attributes.

This comment has been minimized.

Copy link
@kentcdodds

kentcdodds Jun 9, 2015

Hmmm... What if we just place required="false" and ng-required="to.required" in the template ourselves? The manipulator will not add those attributes if they already exist.

This comment has been minimized.

Copy link
@ckniffen

ckniffen Jun 11, 2015

Owner

I looked into the spec and it says "The values "true" and "false" are not allowed on boolean attributes. To represent a false value, the attribute has to be omitted altogether."

http://www.w3.org/html/wg/drafts/html/master/infrastructure.html#boolean-attribute

So unfortunately this will be required.

This comment has been minimized.

Copy link
@kentcdodds

kentcdodds Jun 11, 2015

Ah yes, forgot about that. Neat...

ngModelAttrs:{
required: {
attribute: '',
bound: ''
}
}
},
apiCheckInstance: c,
controller: /* @ngInject */ function($scope) {
const to = $scope.to;
Expand All @@ -23,15 +31,42 @@ export default ngModule => {
change: setModel
};

function checkValidity(expressionValue){
var valid = angular.isArray($scope.model[opts.key]) &&
$scope.model[opts.key].length > 0 &&
expressionValue;

$scope.fc.$setValidity('required', valid);
}

function setModel() {
$scope.model[opts.key] = [];
angular.forEach($scope.multiCheckbox.checked, (checkbox, index) => {
if (checkbox) {
$scope.model[opts.key].push(to.options[index][to.valueProp || 'value']);
}
});

// Must make sure we mark as touched because only the last checkbox due to a bug in angular.
$scope.fc.$setTouched();
checkValidity(true);
}

if(opts.expressionProperties && opts.expressionProperties.required){

This comment has been minimized.

Copy link
@ckniffen

ckniffen Jun 8, 2015

Owner

Make new validity checker work with expression properties.

$scope.$watch($scope.options.expressionProperties.required, function(newValue){
checkValidity(newValue);
});
}

if($scope.to.required){
var unwatchFormControl = $scope.$watch('fc', function(newValue){
if(!newValue){ return; }
checkValidity(true);
unwatchFormControl;
});
}
}
});
}

};

1 comment on commit 08d4fbe

@kentcdodds
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. I would be curious about the required and ng-required attributes as a means to improve the "hack." Also, if you're keen, I'm a big fan of Newspaper Code Structure :-) You've mostly got it.

Thanks for working on this!

Please sign in to comment.