ng1bs4
ng1bs4 copied to clipboard
Add Validations and Error Handling
Implement binding validations in all components.
Adhere @langdonx 's implementation in Pagination component:
...
$onInit() {
// validate bindings
if (typeof this.currentPage !== 'number' || isFinite(this.currentPage) === false) {
this.$log.error('invalid ngbsPagination::currentPage:', this.currentPage, 'expecting a number');
}
...
}
I mentioned this here, but it would be so much easier to implement these everywhere if we had some nice utility functions. What do you think about a validation.service.js
? I'm not sure where it lives (maybe right ./src/library/
), but it looks like this:
export default class {
constructor($log) {
this.$log = $log;
}
validateNumber(obj, component, attribute) {
const property = convertCamelToHyphenated(attribute);
if (obj[property] && (typeof obj[property] !== 'number' || isFinite(obj[property]) === false)) {
this.$log.error(`invalid ${component}::${attribute}: ${JSON.stringify(obj[property])} expecting a number`);
}
}
validateEtc() {}
validateEtc() {}
validateEtc() {}
}
And then it's implemented like this for progress.component.js:
$onChanges(changesObj) {
ValidationService.validateNumber(changesObj, 'ngbs-progress', 'progress-max');
ValidationService.validateNumber(changesObj, 'ngbs-progress', 'progress-value');
ValidationService.validateBoolean(changesObj, 'ngbs-progress', 'progress-animated-progress');
ValidationService.validateBoolean(changesObj, 'ngbs-progress', 'progress-auto-label');
ValidationService.validateBoolean(changesObj, 'ngbs-progress', 'progress-striped');
ValidationService.validateBoolean(changesObj, 'ngbs-progress', 'progress-animated-striped');
ValidationService.validateStringFromArray(changesObj, 'ngbs-progress', 'progress-background', SUPPORTED_BACKGROUNDS);
}
Some thoughts:
- 26 less lines in the component!
- I'm passing
changesObj
to the ValidationService (instead ofthis
) because we don't need to validate all the attributes every any one of them changes. - I switched from passing
'camelCaseAttribute'
to'component-case-attribute'
because a user will be seeing these errors and I imagine it'll be easier to read since that's how they reference them. I don't really know if it's worth the cost in efficiency to be doing that conversion. I imagine for some components$onChanges
might get fired a lot.