-
Notifications
You must be signed in to change notification settings - Fork 27.4k
feat(ngModel): add $overrideModelOptions
support
#15415
feat(ngModel): add $overrideModelOptions
support
#15415
Conversation
caa180a
to
c4debc4
Compare
src/ng/directive/ngModel.js
Outdated
* | ||
* The previous `ModelOptions` value will not be modified. Instead, the | ||
* new `ModelOptions` object will inherit from the previous value of | ||
* `NgModelController.$options`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC, this is the only place we publicly document NgModelController.$options
. I don't think it is a good idea (especially considering we don't specify what its type is or what the API of this ModelOptions
type is).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I'll fix this
test/ng/directive/ngModelSpec.js
Outdated
ctrl.$overrideModelOptions({ debounce: 1000, updateOn: 'blur' }); | ||
expect(ctrl.$options.getOption('debounce')).toEqual(1000); | ||
expect(ctrl.$options.getOption('updateOn')).toEqual('blur'); | ||
expect(ctrl.$options.getOption('updateOnDefault')).toBe(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
test/ng/directive/ngModelSpec.js
Outdated
it('should inherit from a parent model options', inject(function($compile, $rootScope) { | ||
var element = $compile( | ||
'<form name="form" ng-model-options="{debounce: 1000, updateOn: \'blur\'}">' + | ||
'<input ng-model="value" name="input">' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Inconsistent indentation 😁
test/ng/directive/ngModelSpec.js
Outdated
'<input ng-model="value" name="input">' + | ||
'</form>')($rootScope); | ||
var ctrl = $rootScope.form.input; | ||
ctrl.$overrideModelOptions({ debounce: 2000, '*': '$inherit' }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Ideally, I would repeat the same test without the '*': '$inherit'
part (just in case), but this should not block landing the PR.
c4debc4
to
d2bde12
Compare
This change allows developers to modify the model options for an `ngModel` directive programmatically. Closes angular#15415
$overrideModelOptions
support
@petebacondarwin This has been approved and can be merged. I assigned you as the creator. |
d2bde12
to
2b00e88
Compare
This change allows developers to modify the model options for an `ngModel` directive programmatically. Closes #15415
This change allows developers to modify the model options for an `ngModel` directive programmatically. Closes angular#15415
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
feature
What is the current behavior? (You can also link to an open issue here)
no obvious way to programmatically change model options for an ngModel directive
What is the new behavior (if this is a feature change)?
Adds
$overrideModelOptions
methodDoes this PR introduce a breaking change?
No
Please check if the PR fulfills these requirements
Other information: