From 303b381ba1c9e5ff41d24c5600e86ecea2d3fbc5 Mon Sep 17 00:00:00 2001 From: Shahar Talmi Date: Mon, 14 Apr 2014 18:43:07 +0300 Subject: [PATCH 1/3] refactor(inputSpec): move digest into compileInput helper --- test/ng/directive/inputSpec.js | 38 ++-------------------------------- 1 file changed, 2 insertions(+), 36 deletions(-) diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index f3b06c495797..74d9bb002596 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -420,6 +420,7 @@ describe('input', function() { formElm = jqLite('
'); formElm.append(inputElm); $compile(formElm)(scope); + scope.$digest(); } beforeEach(inject(function($injector, _$sniffer_, _$browser_) { @@ -915,7 +916,6 @@ describe('input', function() { compileInput( ''); - scope.$digest(); changeInputValueTo('a'); expect(inputElm.val()).toBe('a'); @@ -929,7 +929,6 @@ describe('input', function() { compileInput( ''); - scope.$digest(); changeInputValueTo('a'); expect(inputElm.val()).toBe('a'); @@ -965,7 +964,6 @@ describe('input', function() { it('should report error on assignment error', function() { expect(function() { compileInput(''); - scope.$digest(); }).toThrowMinErr("$parse", "syntax", "Syntax Error: Token '''' is an unexpected token at column 7 of the expression [throw ''] starting at ['']."); }); @@ -996,7 +994,6 @@ describe('input', function() { it('should validate in-lined pattern', function() { compileInput(''); - scope.$digest(); changeInputValueTo('x000-00-0000x'); expect(inputElm).toBeInvalid(); @@ -1017,7 +1014,6 @@ describe('input', function() { it('should validate in-lined pattern with modifiers', function() { compileInput(''); - scope.$digest(); changeInputValueTo('aB'); expect(inputElm).toBeValid(); @@ -1028,9 +1024,8 @@ describe('input', function() { it('should validate pattern from scope', function() { - compileInput(''); scope.regexp = /^\d\d\d-\d\d-\d\d\d\d$/; - scope.$digest(); + compileInput(''); changeInputValueTo('x000-00-0000x'); expect(inputElm).toBeInvalid(); @@ -1153,7 +1148,6 @@ describe('input', function() { it('should come up blank when no value specified', function() { compileInput(''); - scope.$digest(); expect(inputElm.val()).toBe(''); scope.$apply(function() { @@ -1181,7 +1175,6 @@ describe('input', function() { describe('min', function (){ beforeEach(function (){ compileInput(''); - scope.$digest(); }); it('should invalidate', function (){ @@ -1202,7 +1195,6 @@ describe('input', function() { describe('max', function(){ beforeEach(function (){ compileInput(''); - scope.$digest(); }); it('should validate', function (){ @@ -1279,7 +1271,6 @@ describe('input', function() { it('should come up blank when no value specified', function() { compileInput(''); - scope.$digest(); expect(inputElm.val()).toBe(''); scope.$apply(function() { @@ -1306,7 +1297,6 @@ describe('input', function() { describe('min', function (){ beforeEach(function (){ compileInput(''); - scope.$digest(); }); it('should invalidate', function (){ @@ -1327,7 +1317,6 @@ describe('input', function() { describe('max', function(){ beforeEach(function (){ compileInput(''); - scope.$digest(); }); it('should validate', function (){ @@ -1403,7 +1392,6 @@ describe('input', function() { it('should come up blank when no value specified', function() { compileInput(''); - scope.$digest(); expect(inputElm.val()).toBe(''); scope.$apply(function() { @@ -1430,7 +1418,6 @@ describe('input', function() { describe('min', function (){ beforeEach(function (){ compileInput(''); - scope.$digest(); }); it('should invalidate', function (){ @@ -1451,7 +1438,6 @@ describe('input', function() { describe('max', function (){ beforeEach(function (){ compileInput(''); - scope.$digest(); }); it('should invalidate', function (){ @@ -1472,7 +1458,6 @@ describe('input', function() { it('should validate even if max value changes on-the-fly', function(done) { scope.max = '2013-01-01T01:02'; compileInput(''); - scope.$digest(); changeInputValueTo('2014-01-01T12:34'); expect(inputElm).toBeInvalid(); @@ -1487,7 +1472,6 @@ describe('input', function() { it('should validate even if min value changes on-the-fly', function(done) { scope.min = '2013-01-01T01:02'; compileInput(''); - scope.$digest(); changeInputValueTo('2010-01-01T12:34'); expect(inputElm).toBeInvalid(); @@ -1557,7 +1541,6 @@ describe('input', function() { it('should come up blank when no value specified', function() { compileInput(''); - scope.$digest(); expect(inputElm.val()).toBe(''); scope.$apply(function() { @@ -1584,7 +1567,6 @@ describe('input', function() { describe('min', function (){ beforeEach(function (){ compileInput(''); - scope.$digest(); }); it('should invalidate', function (){ @@ -1605,7 +1587,6 @@ describe('input', function() { describe('max', function (){ beforeEach(function (){ compileInput(''); - scope.$digest(); }); it('should invalidate', function (){ @@ -1626,7 +1607,6 @@ describe('input', function() { it('should validate even if max value changes on-the-fly', function(done) { scope.max = '21:02'; compileInput(''); - scope.$digest(); changeInputValueTo('22:34'); expect(inputElm).toBeInvalid(); @@ -1641,7 +1621,6 @@ describe('input', function() { it('should validate even if min value changes on-the-fly', function(done) { scope.min = '08:45'; compileInput(''); - scope.$digest(); changeInputValueTo('06:15'); expect(inputElm).toBeInvalid(); @@ -1711,7 +1690,6 @@ describe('input', function() { it('should come up blank when no value specified', function() { compileInput(''); - scope.$digest(); expect(inputElm.val()).toBe(''); scope.$apply(function() { @@ -1738,7 +1716,6 @@ describe('input', function() { describe('min', function (){ beforeEach(function (){ compileInput(''); - scope.$digest(); }); it('should invalidate', function (){ @@ -1759,7 +1736,6 @@ describe('input', function() { describe('max', function (){ beforeEach(function (){ compileInput(''); - scope.$digest(); }); it('should invalidate', function (){ @@ -1780,7 +1756,6 @@ describe('input', function() { it('should validate even if max value changes on-the-fly', function(done) { scope.max = '2013-01-01'; compileInput(''); - scope.$digest(); changeInputValueTo('2014-01-01'); expect(inputElm).toBeInvalid(); @@ -1795,7 +1770,6 @@ describe('input', function() { it('should validate even if min value changes on-the-fly', function(done) { scope.min = '2013-01-01'; compileInput(''); - scope.$digest(); changeInputValueTo('2010-01-01'); expect(inputElm).toBeInvalid(); @@ -1846,7 +1820,6 @@ describe('input', function() { it('should come up blank when no value specified', function() { compileInput(''); - scope.$digest(); expect(inputElm.val()).toBe(''); scope.$apply(function() { @@ -1875,7 +1848,6 @@ describe('input', function() { it('should validate', function() { compileInput(''); - scope.$digest(); changeInputValueTo('1'); expect(inputElm).toBeInvalid(); @@ -1891,7 +1863,6 @@ describe('input', function() { it('should validate even if min value changes on-the-fly', function(done) { scope.min = 10; compileInput(''); - scope.$digest(); changeInputValueTo('5'); expect(inputElm).toBeInvalid(); @@ -1909,7 +1880,6 @@ describe('input', function() { it('should validate', function() { compileInput(''); - scope.$digest(); changeInputValueTo('20'); expect(inputElm).toBeInvalid(); @@ -1925,7 +1895,6 @@ describe('input', function() { it('should validate even if max value changes on-the-fly', function(done) { scope.max = 10; compileInput(''); - scope.$digest(); changeInputValueTo('5'); expect(inputElm).toBeValid(); @@ -2352,7 +2321,6 @@ describe('input', function() { it('should set $invalid when model undefined', function() { compileInput(''); - scope.$digest(); expect(inputElm).toBeInvalid(); }); @@ -2406,7 +2374,6 @@ describe('input', function() { compileInput(''); scope.changeFn = jasmine.createSpy('changeFn'); - scope.$digest(); expect(scope.changeFn).not.toHaveBeenCalled(); browserTrigger(inputElm, 'click'); @@ -2433,7 +2400,6 @@ describe('input', function() { compileInput('' + '' + ''); - scope.$digest(); browserTrigger(inputElm[0], 'click'); expect(scope.selected).toBe(true); From ffb4b13a218d735bf54c9420d44b0dc959db824d Mon Sep 17 00:00:00 2001 From: Shahar Talmi Date: Fri, 9 May 2014 02:20:19 +0300 Subject: [PATCH 2/3] refactor(ngModelOptions): move all logic into ngModelController move responsibility for pending and debouncing model updates into ngModeController by letting input dierctives pass all view updates to $setViewValue, where they will remain pending until an updateOn trigger occurs and debounced. introducing a new api ($commitViewValue) which allows to flush pending or debounced updates so that they will take place immidiately. BREAKING CHANGE: NgModelController.$cancelUpdate was renamed to NgModelController.$rollbackViewValue To migrate the code follow the example below: Before: $scope.resetWithCancel = function (e) { if (e.keyCode == 27) { $scope.myForm.myInput1.$cancelUpdate(); $scope.myValue = ''; } }; After: $scope.resetWithCancel = function (e) { if (e.keyCode == 27) { $scope.myForm.myInput1.$rollbackViewValue(); $scope.myValue = ''; } } --- src/ng/directive/input.js | 172 ++++++++++++++++++--------------- test/ng/directive/inputSpec.js | 74 +++++++++++--- 2 files changed, 155 insertions(+), 91 deletions(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index 3ee3cf0fb5f0..0c7664e1d02e 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -16,7 +16,7 @@ var DATETIMELOCAL_REGEXP = /^(\d{4})-(\d\d)-(\d\d)T(\d\d):(\d\d)$/; var WEEK_REGEXP = /^(\d{4})-W(\d\d)$/; var MONTH_REGEXP = /^(\d{4})-(\d\d)$/; var TIME_REGEXP = /^(\d\d):(\d\d)$/; -var DEFAULT_REGEXP = /(\b|^)default(\b|$)/; +var DEFAULT_REGEXP = /(\s+|^)default(\s+|$)/; var inputType = { @@ -934,51 +934,42 @@ function textInputType(scope, element, attr, ctrl, $sniffer, $browser) { } }; - // Allow adding/overriding bound events - if (ctrl.$options && ctrl.$options.updateOn) { - // bind to user-defined events - element.on(ctrl.$options.updateOn, listener); - } - - // setup default events if requested - if (!ctrl.$options || ctrl.$options.updateOnDefault) { - // if the browser does support "input" event, we are fine - except on IE9 which doesn't fire the - // input event on backspace, delete or cut - if ($sniffer.hasEvent('input')) { - element.on('input', listener); - } else { - var timeout; - - var deferListener = function(ev) { - if (!timeout) { - timeout = $browser.defer(function() { - listener(ev); - timeout = null; - }); - } - }; + // if the browser does support "input" event, we are fine - except on IE9 which doesn't fire the + // input event on backspace, delete or cut + if ($sniffer.hasEvent('input')) { + element.on('input', listener); + } else { + var timeout; + + var deferListener = function(ev) { + if (!timeout) { + timeout = $browser.defer(function() { + listener(ev); + timeout = null; + }); + } + }; - element.on('keydown', function(event) { - var key = event.keyCode; + element.on('keydown', function(event) { + var key = event.keyCode; - // ignore - // command modifiers arrows - if (key === 91 || (15 < key && key < 19) || (37 <= key && key <= 40)) return; + // ignore + // command modifiers arrows + if (key === 91 || (15 < key && key < 19) || (37 <= key && key <= 40)) return; - deferListener(event); - }); + deferListener(event); + }); - // if user modifies input value using context menu in IE, we need "paste" and "cut" events to catch it - if ($sniffer.hasEvent('paste')) { - element.on('paste cut', deferListener); - } + // if user modifies input value using context menu in IE, we need "paste" and "cut" events to catch it + if ($sniffer.hasEvent('paste')) { + element.on('paste cut', deferListener); } - - // if user paste into input using mouse on older browser - // or form autocomplete on newer browser, we need "change" event to catch it - element.on('change', listener); } + // if user paste into input using mouse on older browser + // or form autocomplete on newer browser, we need "change" event to catch it + element.on('change', listener); + ctrl.$render = function() { element.val(ctrl.$isEmpty(ctrl.$viewValue) ? '' : ctrl.$viewValue); }; @@ -1221,15 +1212,7 @@ function radioInputType(scope, element, attr, ctrl) { } }; - // Allow adding/overriding bound events - if (ctrl.$options && ctrl.$options.updateOn) { - // bind to user-defined events - element.on(ctrl.$options.updateOn, listener); - } - - if (!ctrl.$options || ctrl.$options.updateOnDefault) { - element.on('click', listener); - } + element.on('click', listener); ctrl.$render = function() { var value = attr.value; @@ -1252,15 +1235,7 @@ function checkboxInputType(scope, element, attr, ctrl) { }); }; - // Allow adding/overriding bound events - if (ctrl.$options && ctrl.$options.updateOn) { - // bind to user-defined events - element.on(ctrl.$options.updateOn, listener); - } - - if (!ctrl.$options || ctrl.$options.updateOnDefault) { - element.on('click', listener); - } + element.on('click', listener); ctrl.$render = function() { element[0].checked = ctrl.$viewValue; @@ -1704,22 +1679,22 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ /** * @ngdoc method - * @name ngModel.NgModelController#$cancelUpdate + * @name ngModel.NgModelController#$rollbackViewValue * * @description - * Cancel an update and reset the input element's value to prevent an update to the `$viewValue`, + * Cancel an update and reset the input element's value to prevent an update to the `$modelValue`, * which may be caused by a pending debounced event or because the input is waiting for a some * future event. * * If you have an input that uses `ng-model-options` to set up debounced events or events such - * as blur you can have a situation where there is a period when the value of the input element - * is out of synch with the ngModel's `$viewValue`. + * as blur you can have a situation where there is a period when the `$viewValue` + * is out of synch with the ngModel's `$modelValue`. * * In this case, you can run into difficulties if you try to update the ngModel's `$modelValue` * programmatically before these debounced/future events have resolved/occurred, because Angular's * dirty checking mechanism is not able to tell whether the model has actually changed or not. * - * The `$cancelUpdate()` method should be called before programmatically changing the model of an + * The `$rollbackViewValue()` method should be called before programmatically changing the model of an * input which may have such events pending. This is important in order to make sure that the * input field will be updated with the new model value and any pending operations are cancelled. * @@ -1730,7 +1705,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ * .controller('CancelUpdateCtrl', function($scope) { * $scope.resetWithCancel = function (e) { * if (e.keyCode == 27) { - * $scope.myForm.myInput1.$cancelUpdate(); + * $scope.myForm.myInput1.$rollbackViewValue(); * $scope.myValue = ''; * } * }; @@ -1749,11 +1724,11 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ *

Now see what happens if you start typing then press the Escape key

* *
- *

With $cancelUpdate()

+ *

With $rollbackViewValue()

*
* myValue: "{{ myValue }}" * - *

Without $cancelUpdate()

+ *

Without $rollbackViewValue()

*
* myValue: "{{ myValue }}" *
@@ -1761,14 +1736,27 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ * * */ - this.$cancelUpdate = function() { + this.$rollbackViewValue = function() { $timeout.cancel(pendingDebounce); + ctrl.$viewValue = ctrl.$$lastCommittedViewValue; ctrl.$render(); }; - // update the view value - this.$$realSetViewValue = function(value) { - ctrl.$viewValue = value; + /** + * @ngdoc method + * @name ngModel.NgModelController#$commitViewValue + * + * @description + * Commit a pending update to the `$modelValue`. + * + * Updates may be pending by a debounced event or because the input is waiting for a some future + * event defined in `ng-model-options`. this method is rarely needed as `NgModelController` + * usually handles calling this in response to input events. + */ + this.$commitViewValue = function() { + var value = ctrl.$viewValue; + ctrl.$$lastCommittedViewValue = value; + $timeout.cancel(pendingDebounce); // change to dirty if (ctrl.$pristine) { @@ -1813,6 +1801,9 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ * * Lastly, all the registered change listeners, in the `$viewChangeListeners` list, are called. * + * In case the {@link ng.directive:ngModelOptions ngModelOptions} directive is used with `updateOn` + * and the `default` trigger is not listed, all those actions will remain pending until one of the + * `updateOn` events is triggered on the DOM element. * All these actions will be debounced if the {@link ng.directive:ngModelOptions ngModelOptions} * directive is used with a custom debounce for this particular event. * @@ -1822,6 +1813,13 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ * @param {string} trigger Event that triggered the update. */ this.$setViewValue = function(value, trigger) { + ctrl.$viewValue = value; + if (!ctrl.$options || ctrl.$options.updateOnDefault) { + ctrl.$$debounceViewValueCommit(trigger); + } + }; + + this.$$debounceViewValueCommit = function(trigger) { var debounceDelay = 0, options = ctrl.$options, debounce; @@ -1840,10 +1838,10 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ $timeout.cancel(pendingDebounce); if (debounceDelay) { pendingDebounce = $timeout(function() { - ctrl.$$realSetViewValue(value); + ctrl.$commitViewValue(); }, debounceDelay); } else { - ctrl.$$realSetViewValue(value); + ctrl.$commitViewValue(); } }; @@ -1863,7 +1861,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ } if (ctrl.$viewValue !== value) { - ctrl.$viewValue = value; + ctrl.$viewValue = ctrl.$$lastCommittedViewValue = value; ctrl.$render(); } } @@ -2001,6 +1999,16 @@ var ngModelDirective = function() { scope.$on('$destroy', function() { formCtrl.$removeControl(modelCtrl); }); + }, + post: function(scope, element, attr, ctrls) { + var modelCtrl = ctrls[0]; + if (modelCtrl.$options && modelCtrl.$options.updateOn) { + element.on(modelCtrl.$options.updateOn, function(ev) { + scope.$apply(function() { + modelCtrl.$$debounceViewValueCommit(ev && ev.type); + }); + }); + } } } }; @@ -2279,14 +2287,18 @@ var ngValueDirective = function() { * * Given the nature of `ngModelOptions`, the value displayed inside input fields in the view might * be different than the value in the actual model. This means that if you update the model you - * should also invoke {@link ngModel.NgModelController `$cancelUpdate`} on the relevant input field in + * should also invoke {@link ngModel.NgModelController `$rollbackViewValue`} on the relevant input field in * order to make sure it is synchronized with the model and that any debounced action is canceled. * - * The easiest way to reference the control's {@link ngModel.NgModelController `$cancelUpdate`} + * The easiest way to reference the control's {@link ngModel.NgModelController `$rollbackViewValue`} * method is by making sure the input is placed inside a form that has a `name` attribute. This is * important because `form` controllers are published to the related scope under the name in their * `name` attribute. * + * Any pending changes will take place immediately when an enclosing form is submitted via the + * `submit` event. Note that `ngClick` events will occur before the model is updated. Use `ngSubmit` + * to have access to the updated model. + * * @param {Object} ngModelOptions options to apply to the current model. Valid keys are: * - `updateOn`: string specifying which event should be the input bound to. You can set several * events using an space delimited list. There is a special event called `default` that @@ -2324,7 +2336,7 @@ var ngValueDirective = function() { $scope.cancel = function (e) { if (e.keyCode == 27) { - $scope.userForm.userName.$cancelUpdate(); + $scope.userForm.userName.$rollbackViewValue(); } }; } @@ -2342,7 +2354,7 @@ var ngValueDirective = function() { expect(model.getText()).toEqual('say hello'); }); - it('should $cancelUpdate when model changes', function() { + it('should $rollbackViewValue when model changes', function() { input.sendKeys(' hello'); expect(input.getAttribute('value')).toEqual('say hello'); input.sendKeys(protractor.Key.ESCAPE); @@ -2364,7 +2376,7 @@ var ngValueDirective = function() { -
+ 
user.name =
@@ -2382,13 +2394,13 @@ var ngModelOptionsDirective = function() { var that = this; this.$options = $scope.$eval($attrs.ngModelOptions); // Allow adding/overriding bound events - if (this.$options.updateOn) { + if (this.$options.updateOn !== undefined) { this.$options.updateOnDefault = false; // extract "default" pseudo-event from list of events that can trigger a model update - this.$options.updateOn = this.$options.updateOn.replace(DEFAULT_REGEXP, function() { + this.$options.updateOn = trim(this.$options.updateOn.replace(DEFAULT_REGEXP, function() { that.$options.updateOnDefault = true; return ' '; - }); + })); } else { this.$options.updateOnDefault = true; } diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index 74d9bb002596..e1d7ff2c9e4e 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -885,11 +885,12 @@ describe('input', function() { 'ng-model-options="{ debounce: 10000, updateOn: \'blur\' }" >' + ''+ '')(scope); + scope.$digest(); - var input = doc.find('input').eq(0); - input.val('a'); + inputElm = doc.find('input').eq(0); + changeInputValueTo('a'); expect(scope.name).toEqual(undefined); - browserTrigger(input, 'blur'); + browserTrigger(inputElm, 'blur'); expect(scope.name).toBe(undefined); $timeout.flush(2000); expect(scope.name).toBe(undefined); @@ -898,7 +899,58 @@ describe('input', function() { dealoc(doc); })); + it('should flush debounced events when calling $commitViewValue directly', function() { + compileInput( + ''); + + changeInputValueTo('a'); + expect(scope.name).toEqual(undefined); + scope.form.alias.$commitViewValue(); + expect(scope.name).toEqual('a'); + }); + + it('should cancel debounced events when calling $commitViewValue', inject(function($timeout) { + compileInput( + ''); + + changeInputValueTo('a'); + scope.form.alias.$commitViewValue(); + expect(scope.name).toEqual('a'); + + scope.form.alias.$setPristine(); + $timeout.flush(1000); + expect(scope.form.alias.$pristine).toBeTruthy(); + })); + + it('should reset input val if rollbackViewValue called during pending update', function() { + compileInput( + ''); + + changeInputValueTo('a'); + expect(inputElm.val()).toBe('a'); + scope.form.alias.$rollbackViewValue(); + expect(inputElm.val()).toBe(''); + browserTrigger(inputElm, 'blur'); + expect(inputElm.val()).toBe(''); + }); + it('should allow canceling pending updates', inject(function($timeout) { + compileInput( + ''); + + changeInputValueTo('a'); + expect(scope.name).toEqual(undefined); + scope.form.alias.$rollbackViewValue(); + expect(scope.name).toEqual(undefined); + browserTrigger(inputElm, 'blur'); + expect(scope.name).toEqual(undefined); + })); + + it('should allow canceling debounced updates', inject(function($timeout) { compileInput( ''); @@ -906,33 +958,33 @@ describe('input', function() { changeInputValueTo('a'); expect(scope.name).toEqual(undefined); $timeout.flush(2000); - scope.form.alias.$cancelUpdate(); + scope.form.alias.$rollbackViewValue(); expect(scope.name).toEqual(undefined); $timeout.flush(10000); expect(scope.name).toEqual(undefined); })); - it('should reset input val if cancelUpdate called during pending update', function() { + it('should handle model updates correctly even if rollbackViewValue is not invoked', function() { compileInput( ''); changeInputValueTo('a'); - expect(inputElm.val()).toBe('a'); - scope.form.alias.$cancelUpdate(); - expect(inputElm.val()).toBe(''); + scope.$apply(function() { + scope.name = 'b'; + }); browserTrigger(inputElm, 'blur'); - expect(inputElm.val()).toBe(''); + expect(scope.name).toBe('b'); }); - it('should reset input val if cancelUpdate called during debounce', inject(function($timeout) { + it('should reset input val if rollbackViewValue called during debounce', inject(function($timeout) { compileInput( ''); changeInputValueTo('a'); expect(inputElm.val()).toBe('a'); - scope.form.alias.$cancelUpdate(); + scope.form.alias.$rollbackViewValue(); expect(inputElm.val()).toBe(''); $timeout.flush(3000); expect(inputElm.val()).toBe(''); From 479ae4a519fde7ef6edbbb1fefaef73c348d48a6 Mon Sep 17 00:00:00 2001 From: Shahar Talmi Date: Fri, 9 May 2014 02:52:37 +0300 Subject: [PATCH 3/3] feat(ngFormController): commit view value on submit use the new NgModelController.$commitViewValue method in order to make sure that all pending and debounced updates are flushed immediately in case the enclosing for is submitted. Closes #7017 --- src/ng/directive/form.js | 31 ++++++++++++++++++--- test/ng/directive/formSpec.js | 51 +++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/src/ng/directive/form.js b/src/ng/directive/form.js index 23fa8d8b91e8..0369eb4eb2e8 100644 --- a/src/ng/directive/form.js +++ b/src/ng/directive/form.js @@ -74,6 +74,23 @@ function FormController(element, attrs, $scope, $animate) { $animate.addClass(element, (isValid ? VALID_CLASS : INVALID_CLASS) + validationErrorKey); } + /** + * @ngdoc method + * @name form.FormController#$commitViewValue + * + * @description + * Commit all form controls pending updates to the `$modelValue`. + * + * Updates may be pending by a debounced event or because the input is waiting for a some future + * event defined in `ng-model-options`. This method is rarely needed as `NgModelController` + * usually handles calling this in response to input events. + */ + form.$commitViewValue = function() { + forEach(controls, function(control) { + control.$commitViewValue(); + }); + }; + /** * @ngdoc method * @name form.FormController#$addControl @@ -286,6 +303,10 @@ function FormController(element, attrs, $scope, $animate) { * hitting enter in any of the input fields will trigger the click handler on the *first* button or * input[type=submit] (`ngClick`) *and* a submit handler on the enclosing form (`ngSubmit`) * + * Any pending `ngModelOptions` changes will take place immediately when an enclosing form is + * submitted. Note that `ngClick` events will occur before the model is updated. Use `ngSubmit` + * to have access to the updated model. + * * @param {string=} name Name of the form. If specified, the form controller will be published into * related scope, under this name. * @@ -381,19 +402,23 @@ var formDirectiveFactory = function(isNgForm) { // IE 9 is not affected because it doesn't fire a submit event and try to do a full // page reload if the form was destroyed by submission of the form via a click handler // on a button in the form. Looks like an IE9 specific bug. - var preventDefaultListener = function(event) { + var handleFormSubmission = function(event) { + scope.$apply(function() { + controller.$commitViewValue(); + }); + event.preventDefault ? event.preventDefault() : event.returnValue = false; // IE }; - addEventListenerFn(formElement[0], 'submit', preventDefaultListener); + addEventListenerFn(formElement[0], 'submit', handleFormSubmission); // unregister the preventDefault listener so that we don't not leak memory but in a // way that will achieve the prevention of the default action. formElement.on('$destroy', function() { $timeout(function() { - removeEventListenerFn(formElement[0], 'submit', preventDefaultListener); + removeEventListenerFn(formElement[0], 'submit', handleFormSubmission); }, 0, false); }); } diff --git a/test/ng/directive/formSpec.js b/test/ng/directive/formSpec.js index b9f0ded777b9..03829da12d07 100644 --- a/test/ng/directive/formSpec.js +++ b/test/ng/directive/formSpec.js @@ -154,6 +154,57 @@ describe('form', function() { }).toThrowMinErr('ng', 'badname'); }); + describe('triggering commit value on submit', function () { + it('should trigger update on form submit', function() { + var form = $compile( + '
' + + '' + + '
')(scope); + scope.$digest(); + + var inputElm = form.find('input').eq(0); + changeInputValue(inputElm, 'a'); + expect(scope.name).toEqual(undefined); + browserTrigger(form, 'submit'); + expect(scope.name).toEqual('a'); + dealoc(form); + }); + + it('should trigger update on form submit with nested forms', function() { + var form = $compile( + '
' + + '
' + + '' + + '
' + + '
')(scope); + scope.$digest(); + + var inputElm = form.find('input').eq(0); + changeInputValue(inputElm, 'a'); + expect(scope.name).toEqual(undefined); + browserTrigger(form, 'submit'); + expect(scope.name).toEqual('a'); + dealoc(form); + }); + + it('should trigger update before ng-submit is invoked', function() { + var form = $compile( + '
' + + '' + + '
')(scope); + scope.$digest(); + + var inputElm = form.find('input').eq(0); + changeInputValue(inputElm, 'a'); + scope.submit = jasmine.createSpy('submit').andCallFake(function() { + expect(scope.name).toEqual('a'); + }); + browserTrigger(form, 'submit'); + expect(scope.submit).toHaveBeenCalled(); + dealoc(form); + }); + }); describe('preventing default submission', function() {