Skip to content

Commit 76a562c

Browse files
committed
Fixes from code review in #371. Reworked the default destroyStrategy to be picked up directly from globalOptions, instead of looking in formDefaults. Properly handles for the difference between the destroyStrategy declared as undefined and completely undeclared, both at the globalOptions level and the form field definition level.
1 parent e73bfe2 commit 76a562c

File tree

2 files changed

+36
-22
lines changed

2 files changed

+36
-22
lines changed

docs/index.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ attribute which should be placed along side `sf-schema`.
193193
| formDefaults | an object that will be used as a default for all form definitions |
194194
| validationMessage | an object or a function that will be used as default validation message for all fields. See [Validation Messages](#validation-messages) for details. |
195195
| setSchemaDefaults | boolean, set to false an no defaults from the schema will be set on the model. |
196+
| destroyStrategy | the default strategy to use for cleaning the model when a form element is removed. see [destroyStrategy](#destroyStrategy) below |
196197
197198
*formDefaults* is mostly useful for setting global [ngModelOptions](#ngmodeloptions)
198199
i.e. changing the entire form to validate on blur.
@@ -831,7 +832,7 @@ will update the model to set the field value to undefined. This can be overridde
831832
on a field to one of null, empty string (""), undefined, or "retain". Any other value will be ignored and the default
832833
behavior will apply. The empty string option only applies to fields that have a type of string; using the empty string
833834
with other field types will just be set to the default destroyStrategy. If you'd like to set the destroyStrategy for
834-
an entire form, add it to the formDefaults in the [globalOptions](#global-options)
835+
an entire form, add it to the [globalOptions](#global-options)
835836
836837
837838

src/directives/schema-validate.js

+34-21
Original file line numberDiff line numberDiff line change
@@ -103,62 +103,75 @@ angular.module('schemaForm').directive('schemaValidate', ['sfValidator', 'sfSele
103103
});
104104

105105

106-
var DEFAULT_DESTROY_STRATEGY;
107-
if (scope.options && scope.options.formDefaults) {
108-
var formDefaultDestroyStrategy = scope.options.formDefaults.destroyStrategy;
109-
var isValidFormDefaultDestroyStrategy = (formDefaultDestroyStrategy === undefined ||
110-
formDefaultDestroyStrategy === '' ||
111-
formDefaultDestroyStrategy === null ||
112-
formDefaultDestroyStrategy === 'retain');
113-
if (isValidFormDefaultDestroyStrategy) {
114-
DEFAULT_DESTROY_STRATEGY = formDefaultDestroyStrategy;
115-
}
116-
else {
117-
console.warn('Unrecognized formDefaults.destroyStrategy: \'%s\'. Used undefined instead.',
118-
formDefaultDestroyStrategy);
119-
DEFAULT_DESTROY_STRATEGY = undefined;
106+
var DEFAULT_DESTROY_STRATEGY = getGlobalOptionsDestroyStrategy();
107+
108+
function getGlobalOptionsDestroyStrategy() {
109+
var defaultStrategy = undefined;
110+
if (scope.options && scope.options.hasOwnProperty('destroyStrategy')) {
111+
var globalOptionsDestroyStrategy = scope.options.destroyStrategy;
112+
var isValidFormDefaultDestroyStrategy = (globalOptionsDestroyStrategy === undefined ||
113+
globalOptionsDestroyStrategy === '' ||
114+
globalOptionsDestroyStrategy === null ||
115+
globalOptionsDestroyStrategy === 'retain');
116+
if (isValidFormDefaultDestroyStrategy) {
117+
defaultStrategy = globalOptionsDestroyStrategy;
118+
}
119+
else {
120+
console.warn('Unrecognized globalOptions destroyStrategy: %s \'%s\'. Used undefined instead.',
121+
typeof globalOptionsDestroyStrategy, globalOptionsDestroyStrategy);
122+
}
120123
}
124+
return defaultStrategy;
121125
}
122126

123127
// Clean up the model when the corresponding form field is $destroy-ed.
124-
// Default behavior can be supplied as a formDefault, and behavior can be overridden in the form definition.
128+
// Default behavior can be supplied as a globalOption, and behavior can be overridden in the form definition.
125129
scope.$on('$destroy', function() {
126130
var form = getForm();
127-
var destroyStrategy = form.destroyStrategy; // Either set in form definition, or as part of formDefaults.
131+
132+
// Either set in form definition, or as part of globalOptions.
133+
var destroyStrategy =
134+
!form.hasOwnProperty('destroyStrategy') ? DEFAULT_DESTROY_STRATEGY : form.destroyStrategy;
128135
var schemaType = getSchemaType();
129136

130137
if (destroyStrategy && destroyStrategy !== 'retain' ) {
131138
// Don't recognize the strategy, so give a warning.
132-
console.warn('Unrecognized destroyStrategy: \'%s\'. Used default instead.', destroyStrategy);
139+
console.warn('%s has defined unrecognized destroyStrategy: \'%s\'. Used default instead.',
140+
attrs.name, destroyStrategy);
133141
destroyStrategy = DEFAULT_DESTROY_STRATEGY;
134142
}
135143
else if (schemaType !== 'string' && destroyStrategy === '') {
136144
// Only 'string' type fields can have an empty string value as a valid option.
137-
console.warn('Attempted to use empty string destroyStrategy on non-string form type. Used default instead.');
145+
console.warn('%s attempted to use empty string destroyStrategy on non-string form type. ' +
146+
'Used default instead.', attrs.name);
138147
destroyStrategy = DEFAULT_DESTROY_STRATEGY;
139148
}
140149

141150
if (destroyStrategy === 'retain') {
142151
return; // Valid option to avoid destroying data in the model.
143152
}
153+
console.log('result %s', destroyStrategy);
144154

145155
destroyUsingStrategy(destroyStrategy);
146156

147157
function destroyUsingStrategy(strategy) {
148-
var strategyIsDefined = (strategy === null || strategy === '' || typeof strategy == undefined);
158+
console.log('Destroy called with %s', strategy);
159+
var strategyIsDefined = (strategy === null || strategy === '' || strategy === undefined);
149160
if (!strategyIsDefined){
150161
strategy = DEFAULT_DESTROY_STRATEGY;
151162
}
152163
sfUnselect(scope.form.key, scope.model, strategy);
153164
}
154165

155166
function getSchemaType() {
167+
var sType;
156168
if (form.schema) {
157-
schemaType = form.schema.type;
169+
sType = form.schema.type;
158170
}
159171
else {
160-
schemaType = null;
172+
sType = null;
161173
}
174+
return sType;
162175
}
163176
});
164177

0 commit comments

Comments
 (0)