Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

feat(ngModel) Allow running the formatters without a change to the modelValue #10764

Closed
wants to merge 1 commit into from
+72 −15
Closed

feat(ngModel) Allow running the formatters without a change to the modelValue #10764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 36 additions & 15 deletions src/ng/directive/ngModel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,41 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
}
};

function formatValue(modelValue) {
var formatters = ctrl.$formatters,
idx = formatters.length;

var viewValue = modelValue;
while (idx--) {
viewValue = formatters[idx](viewValue);
}

return viewValue;
}

/**
* @ngdoc method
* @name ngModel.NgModelController#$setModelValue
*
*/
this.$setModelValue = function(modelValue, fromModel) {
var previousModelValue = ctrl.$modelValue;
ctrl.$modelValue = ctrl.$$rawModelValue = modelValue;

if (!fromModel && previousModelValue !== ctrl.$modelValue) {
this.$$writeModelToScope();
}

var viewValue = formatValue(this.$modelValue);

if (this.$viewValue !== viewValue) {
this.$viewValue = ctrl.$$lastCommittedViewValue = viewValue;
this.$render();

ctrl.$$runValidators(undefined, modelValue, ctrl.$viewValue, noop);
}
};

// model -> value
// Note: we cannot use a normal scope.$watch as we want to detect the following:
// 1. scope value is 'a'
Expand All @@ -813,21 +848,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
// if scope model value and ngModel value are out of sync
// TODO(perf): why not move this to the action fn?
if (modelValue !== ctrl.$modelValue) {
ctrl.$modelValue = ctrl.$$rawModelValue = modelValue;

var formatters = ctrl.$formatters,
idx = formatters.length;

var viewValue = modelValue;
while (idx--) {
viewValue = formatters[idx](viewValue);
}
if (ctrl.$viewValue !== viewValue) {
ctrl.$viewValue = ctrl.$$lastCommittedViewValue = viewValue;
ctrl.$render();

ctrl.$$runValidators(undefined, modelValue, viewValue, noop);
}
ctrl.$setModelValue(modelValue, true);
}

return modelValue;
Expand Down
36 changes: 36 additions & 0 deletions test/ng/directive/ngModelSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,42 @@ describe('ngModel', function() {
});


describe('$setModelValue', function() {

it('should set the value to $modelValue', function() {
ctrl.$setModelValue(10);
expect(ctrl.$modelValue).toBe(10);
});

it('should update the value on the scope', inject(function($compile) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually looking at this again I don't like that we can set the $modelValue to a value that is different to the scope.

For your use case @realityking you should always call this method as ngModel.$setModelValue(ngModel.$modelValue) and providing a different value should be documented as not supported.
This would remove the need for this test and the associated behaviour.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the follow up.

This sounds like a bad API (you can call this function only with this value, nothing else). To be honest, given this comment, I'd be more comfortable returning to $runFormatters (maybe called $format to be more inline with the other methods in ngModelController)

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I guess you are right.
If we had ngModel.$formatModel, then I suspect we would need to have a complementary ngModel.$parseView? Since changes to the formatters might well include changes to the parsers too? In which case one would want the new viewValue to be parsed and update the model....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds like a decent idea, even if only for API symmetry reasons. (I believe you can already gain the effect by calling '$setViewValue` with the current view value)

I'll try to get it done over the weekend.

var element = $compile('<form name="form"><input name="field" ng-model="val" /></form>')(scope);

var input = element.children().eq(0);
ctrl = input.controller('ngModel');

scope.val = 11;
scope.$digest();
ctrl.$setModelValue(22);
scope.$digest();
expect(scope.val).toBe(22);

dealoc(element);
}));

it('should $render only if value changed', function() {
spyOn(ctrl, '$render');

ctrl.$setModelValue(3);
expect(ctrl.$render).toHaveBeenCalledOnce();
ctrl.$render.reset();

ctrl.$formatters.push(function() {return 3;});
ctrl.$setModelValue(5);
expect(ctrl.$render).not.toHaveBeenCalled();
});
});


describe('model -> view', function() {

it('should set the value to $modelValue', function() {
Expand Down