From 74d24453fc8a20faacf6cefe5c55ff8a02aacfa3 Mon Sep 17 00:00:00 2001 From: Michael Prentice Date: Wed, 13 Jun 2018 00:04:06 -0400 Subject: [PATCH] fix(chips): regression where chips model gets out of sync with view (#11310) manually call `ngChange` as it won't be triggered by `ngModel` for chips add `ng-change` test to contact-chips refinements to chips and contact-chips docs remove references to `filter-selected` on contact-chips it was disabled 2 years ago remove use of `angular.lowercase` which is removed in AngularJS 1.7 Fixes #11304. Fixes #11301. --- .../autocomplete/demoBasicUsage/script.js | 2 +- .../autocomplete/demoCustomTemplate/script.js | 2 +- .../autocomplete/demoFloatingLabel/script.js | 2 +- .../autocomplete/demoInsideDialog/script.js | 2 +- src/components/chips/chips.spec.js | 32 +++++++++-- src/components/chips/contact-chips.spec.js | 38 ++++++++++++- .../chips/demoContactChips/index.html | 2 - .../chips/demoContactChips/script.js | 17 +++--- .../chips/demoCustomInputs/script.js | 2 +- src/components/chips/js/chipsController.js | 54 +++++++++++++------ src/components/chips/js/chipsDirective.js | 31 ++++++----- .../chips/js/contactChipsController.js | 4 -- .../chips/js/contactChipsDirective.js | 12 ++--- 13 files changed, 136 insertions(+), 64 deletions(-) diff --git a/src/components/autocomplete/demoBasicUsage/script.js b/src/components/autocomplete/demoBasicUsage/script.js index 35b757d69d0..607f3b33915 100644 --- a/src/components/autocomplete/demoBasicUsage/script.js +++ b/src/components/autocomplete/demoBasicUsage/script.js @@ -74,7 +74,7 @@ * Create filter function for a query string */ function createFilterFor(query) { - var lowercaseQuery = angular.lowercase(query); + var lowercaseQuery = query.toLowerCase(); return function filterFn(state) { return (state.value.indexOf(lowercaseQuery) === 0); diff --git a/src/components/autocomplete/demoCustomTemplate/script.js b/src/components/autocomplete/demoCustomTemplate/script.js index c7ebf7a3b2c..23eec80514c 100644 --- a/src/components/autocomplete/demoCustomTemplate/script.js +++ b/src/components/autocomplete/demoCustomTemplate/script.js @@ -95,7 +95,7 @@ * Create filter function for a query string */ function createFilterFor(query) { - var lowercaseQuery = angular.lowercase(query); + var lowercaseQuery = query.toLowerCase(); return function filterFn(item) { return (item.value.indexOf(lowercaseQuery) === 0); diff --git a/src/components/autocomplete/demoFloatingLabel/script.js b/src/components/autocomplete/demoFloatingLabel/script.js index 37c98d5f0c2..fa37203b72f 100644 --- a/src/components/autocomplete/demoFloatingLabel/script.js +++ b/src/components/autocomplete/demoFloatingLabel/script.js @@ -52,7 +52,7 @@ * Create filter function for a query string */ function createFilterFor(query) { - var lowercaseQuery = angular.lowercase(query); + var lowercaseQuery = query.toLowerCase(); return function filterFn(state) { return (state.value.indexOf(lowercaseQuery) === 0); diff --git a/src/components/autocomplete/demoInsideDialog/script.js b/src/components/autocomplete/demoInsideDialog/script.js index 6ea5899e597..1ce37c4af69 100644 --- a/src/components/autocomplete/demoInsideDialog/script.js +++ b/src/components/autocomplete/demoInsideDialog/script.js @@ -73,7 +73,7 @@ * Create filter function for a query string */ function createFilterFor(query) { - var lowercaseQuery = angular.lowercase(query); + var lowercaseQuery = query.toLowerCase(); return function filterFn(state) { return (state.value.indexOf(lowercaseQuery) === 0); diff --git a/src/components/chips/chips.spec.js b/src/components/chips/chips.spec.js index 2309d13bd77..9f2ec44aa6d 100755 --- a/src/components/chips/chips.spec.js +++ b/src/components/chips/chips.spec.js @@ -39,7 +39,7 @@ describe('', function() { describe('with no overrides', function() { beforeEach(module('material.components.chips', 'material.components.autocomplete')); beforeEach(inject(function($rootScope, _$exceptionHandler_, _$timeout_) { - scope = $rootScope.$new(); + scope = $rootScope.$new(false); scope.items = ['Apple', 'Banana', 'Orange']; $exceptionHandler = _$exceptionHandler_; $timeout = _$timeout_; @@ -177,6 +177,30 @@ describe('', function() { expect(scope.addChip.calls.mostRecent().args[1]).toBe(3); // Index }); + it('should update the view if the add method changes or removes the chip', function() { + var element = buildChips(CHIP_ADD_TEMPLATE); + var ctrl = element.controller('mdChips'); + + scope.addChip = function ($chip, $index) { + if ($chip === 'Grape') { + var grape = scope.items.pop(); + grape += '[' + $index + ']'; + scope.items.push(grape); + } + if ($chip === 'Broccoli') { + scope.items.pop(); + } + }; + + element.scope().$apply(function() { + ctrl.chipBuffer = 'Broccoli'; + simulateInputEnterKey(ctrl); + ctrl.chipBuffer = 'Grape'; + simulateInputEnterKey(ctrl); + }); + + expect(scope.items[3]).toBe('Grape[3]'); + }); it('should call the remove method when removing a chip', function() { var element = buildChips(CHIP_REMOVE_TEMPLATE); @@ -217,16 +241,17 @@ describe('', function() { simulateInputEnterKey(ctrl); }); expect(scope.onModelChange).toHaveBeenCalled(); + expect(scope.onModelChange.calls.count()).toBe(1); expect(scope.onModelChange.calls.mostRecent().args[0].length).toBe(4); element.scope().$apply(function() { ctrl.removeChip(0); }); expect(scope.onModelChange).toHaveBeenCalled(); + expect(scope.onModelChange.calls.count()).toBe(2); expect(scope.onModelChange.calls.mostRecent().args[0].length).toBe(3); }); - it('should call the select method when selecting a chip', function() { var element = buildChips(CHIP_SELECT_TEMPLATE); var ctrl = element.controller('mdChips'); @@ -694,7 +719,8 @@ describe('', function() { input.val(' Test '); // We have to trigger the `change` event, because IE11 does not support - // the `input` event to update the ngModel. An alternative for `input` is to use the `change` event. + // the `input` event to update the ngModel. An alternative for `input` is to use the + // `change` event. input.triggerHandler('change'); expect(ctrl.chipBuffer).toBeTruthy(); diff --git a/src/components/chips/contact-chips.spec.js b/src/components/chips/contact-chips.spec.js index 77c22ee9ead..f692f873084 100644 --- a/src/components/chips/contact-chips.spec.js +++ b/src/components/chips/contact-chips.spec.js @@ -10,13 +10,14 @@ describe('', function() { md-highlight-flags="i"\ md-min-length="1"\ md-chip-append-delay="2000"\ + ng-change="onModelChange(contacts)"\ placeholder="To">\ '; beforeEach(module('material.components.chips')); beforeEach(inject(function($rootScope) { - scope = $rootScope.$new(); + scope = $rootScope.$new(false); var img = 'data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw=='; scope.allContacts = [ { @@ -35,7 +36,7 @@ describe('', function() { ]; scope.contacts = []; - scope.highlightFlags = "i"; + scope.highlightFlags = 'i'; })); var attachedElements = []; @@ -65,6 +66,31 @@ describe('', function() { expect(ctrl.highlightFlags).toEqual('i'); }); + it('should trigger ng-change on chip addition/removal', function() { + var element = buildChips(CONTACT_CHIPS_TEMPLATE); + var ctrl = element.controller('mdContactChips'); + var chipsElement = element.find('md-chips'); + var chipsCtrl = chipsElement.controller('mdChips'); + + scope.onModelChange = jasmine.createSpy('onModelChange'); + + element.scope().$apply(function() { + chipsCtrl.appendChip(scope.allContacts[0]); + }); + expect(scope.onModelChange).toHaveBeenCalled(); + expect(scope.onModelChange.calls.count()).toBe(1); + expect(scope.onModelChange.calls.mostRecent().args[0].length).toBe(1); + expect(scope.contacts.length).toBe(1); + + element.scope().$apply(function() { + chipsCtrl.removeChip(0); + }); + expect(scope.onModelChange).toHaveBeenCalled(); + expect(scope.onModelChange.calls.count()).toBe(2); + expect(scope.onModelChange.calls.mostRecent().args[0].length).toBe(0); + expect(scope.contacts.length).toBe(0); + }); + it('forwards the md-chips-append-delay attribute to the md-chips', function() { var element = buildChips(CONTACT_CHIPS_TEMPLATE); var chipsCtrl = element.find('md-chips').controller('mdChips'); @@ -167,4 +193,12 @@ describe('', function() { return container; } + function simulateInputEnterKey(ctrl) { + var event = {}; + event.preventDefault = jasmine.createSpy('preventDefault'); + inject(function($mdConstant) { + event.keyCode = $mdConstant.KEY_CODE.ENTER; + }); + ctrl.inputKeydown(event); + } }); diff --git a/src/components/chips/demoContactChips/index.html b/src/components/chips/demoContactChips/index.html index f1883034295..49f9b8c839d 100644 --- a/src/components/chips/demoContactChips/index.html +++ b/src/components/chips/demoContactChips/index.html @@ -10,7 +10,6 @@ md-contact-email="email" md-require-match="true" md-highlight-flags="i" - filter-selected="ctrl.filterSelected" placeholder="To"> @@ -43,7 +42,6 @@

Searching asynchronously.

md-contact-email="email" md-require-match="true" md-highlight-flags="i" - filter-selected="ctrl.filterSelected" placeholder="To"> diff --git a/src/components/chips/demoContactChips/script.js b/src/components/chips/demoContactChips/script.js index a558ee3d701..43b9cb4ec26 100644 --- a/src/components/chips/demoContactChips/script.js +++ b/src/components/chips/demoContactChips/script.js @@ -2,7 +2,7 @@ 'use strict'; // If we do not have CryptoJS defined; import it - if (typeof CryptoJS == 'undefined') { + if (typeof CryptoJS === 'undefined') { var cryptoSrc = 'https://cdnjs.cloudflare.com/ajax/libs/crypto-js/3.1.2/rollups/md5.js'; var scriptTag = document.createElement('script'); scriptTag.setAttribute('src', cryptoSrc); @@ -13,7 +13,7 @@ .module('contactChipsDemo', ['ngMaterial']) .controller('ContactChipDemoCtrl', DemoCtrl); - function DemoCtrl ($q, $timeout) { + function DemoCtrl ($q, $timeout, $log) { var self = this; var pendingSearch, cancelSearch = angular.noop; var lastSearch; @@ -21,7 +21,6 @@ self.allContacts = loadContacts(); self.contacts = [self.allContacts[0]]; self.asyncContacts = []; - self.filterSelected = true; self.querySearch = querySearch; self.delayedQuerySearch = delayedQuerySearch; @@ -39,7 +38,7 @@ * Also debounce the queries; since the md-contact-chips does not support this */ function delayedQuerySearch(criteria) { - if ( !pendingSearch || !debounceSearch() ) { + if (!pendingSearch || !debounceSearch()) { cancelSearch(); return pendingSearch = $q(function(resolve, reject) { @@ -77,16 +76,16 @@ * Create filter function for a query string */ function createFilterFor(query) { - var lowercaseQuery = angular.lowercase(query); + var lowercaseQuery = query.toLowerCase(); return function filterFn(contact) { - return (contact._lowername.indexOf(lowercaseQuery) != -1); + return (contact._lowername.indexOf(lowercaseQuery) !== -1); }; } - function onModelChange(model) { - alert('The model has changed'); + function onModelChange(newModel) { + $log.log('The model has changed to ' + JSON.stringify(newModel) + '.'); } function loadContacts() { @@ -117,6 +116,4 @@ }); } } - - })(); diff --git a/src/components/chips/demoCustomInputs/script.js b/src/components/chips/demoCustomInputs/script.js index ffa56f36e2e..b71c31e4aae 100644 --- a/src/components/chips/demoCustomInputs/script.js +++ b/src/components/chips/demoCustomInputs/script.js @@ -44,7 +44,7 @@ * Create filter function for a query string */ function createFilterFor(query) { - var lowercaseQuery = angular.lowercase(query); + var lowercaseQuery = query.toLowerCase(); return function filterFn(vegetable) { return (vegetable._lowername.indexOf(lowercaseQuery) === 0) || diff --git a/src/components/chips/js/chipsController.js b/src/components/chips/js/chipsController.js index c94f2877fda..155ea05fc3d 100644 --- a/src/components/chips/js/chipsController.js +++ b/src/components/chips/js/chipsController.js @@ -21,9 +21,11 @@ angular * @param $element * @param $timeout * @param $mdUtil + * @param $exceptionHandler * @constructor */ -function MdChipsCtrl ($scope, $attrs, $mdConstant, $log, $element, $timeout, $mdUtil) { +function MdChipsCtrl ($scope, $attrs, $mdConstant, $log, $element, $timeout, $mdUtil, + $exceptionHandler) { /** @type {$timeout} **/ this.$timeout = $timeout; @@ -42,6 +44,9 @@ function MdChipsCtrl ($scope, $attrs, $mdConstant, $log, $element, $timeout, $md /** @type {$log} */ this.$log = $log; + /** @type {$exceptionHandler} */ + this.$exceptionHandler = $exceptionHandler; + /** @type {$element} */ this.$element = $element; @@ -142,10 +147,10 @@ function MdChipsCtrl ($scope, $attrs, $mdConstant, $log, $element, $timeout, $md this.contentIds = []; /** - * The index of the chip that should have it's tabindex property set to 0 so it is selectable + * The index of the chip that should have it's `tabindex` property set to `0` so it is selectable * via the keyboard. * - * @type {int} + * @type {number} */ this.ariaTabIndex = null; @@ -326,9 +331,9 @@ MdChipsCtrl.prototype.getCursorPosition = function(element) { * @param chipContents */ MdChipsCtrl.prototype.updateChipContents = function(chipIndex, chipContents){ - if(chipIndex >= 0 && chipIndex < this.items.length) { + if (chipIndex >= 0 && chipIndex < this.items.length) { this.items[chipIndex] = chipContents; - this.updateNgModel(); + this.updateNgModel(true); } }; @@ -454,8 +459,7 @@ MdChipsCtrl.prototype.getAdjacentChipIndex = function(index) { /** * Append the contents of the buffer to the chip list. This method will first * call out to the md-transform-chip method, if provided. - * - * @param newChip + * @param {string} newChip chip buffer contents that will be used to create the new chip */ MdChipsCtrl.prototype.appendChip = function(newChip) { this.shouldFocusLastChip = true; @@ -486,7 +490,7 @@ MdChipsCtrl.prototype.appendChip = function(newChip) { this.updateNgModel(); - // If they provide the md-on-add attribute, notify them of the chip addition + // If the md-on-add attribute is specified, send a chip addition event if (this.useOnAdd && this.onAdd) { this.onAdd({ '$chip': newChip, '$index': index }); } @@ -549,7 +553,8 @@ MdChipsCtrl.prototype.getChipBuffer = function() { this.userInputNgModelCtrl ? this.userInputNgModelCtrl.$viewValue : this.userInputElement[0].value; - // Ensure that the chip buffer is always a string. For example, the input element buffer might be falsy. + // Ensure that the chip buffer is always a string. For example, the input element buffer + // might be falsy. return angular.isString(chipBuffer) ? chipBuffer : ''; }; @@ -577,23 +582,38 @@ MdChipsCtrl.prototype.hasMaxChipsReached = function() { /** * Updates the validity properties for the ngModel. + * + * TODO add the md-max-chips validator to this.ngModelCtrl.validators so that the validation will + * be performed automatically. */ MdChipsCtrl.prototype.validateModel = function() { this.ngModelCtrl.$setValidity('md-max-chips', !this.hasMaxChipsReached()); this.ngModelCtrl.$validate(); // rerun any registered validators }; -MdChipsCtrl.prototype.updateNgModel = function() { - this.ngModelCtrl.$setViewValue(this.items.slice()); - // TODO add the md-max-chips validator to this.ngModelCtrl.validators so that - // the validation will be performed automatically on $viewValue change - this.validateModel(); +/** + * Function to handle updating the model, validation, and change notification when a chip + * is added, removed, or changed. + * @param {boolean=} skipValidation true to skip calling validateModel() + */ +MdChipsCtrl.prototype.updateNgModel = function(skipValidation) { + if (!skipValidation) { + this.validateModel(); + } + // This will trigger ng-change to fire, even in cases where $setViewValue() would not. + angular.forEach(this.ngModelCtrl.$viewChangeListeners, function(listener) { + try { + listener(); + } catch (e) { + this.$exceptionHandler(e); + } + }); }; /** * Removes the chip at the given index. - * @param {number} index - * @param {Event=} event + * @param {number} index of chip to remove + * @param {Event=} event optionally passed to the onRemove callback */ MdChipsCtrl.prototype.removeChip = function(index, event) { var removed = this.items.splice(index, 1); @@ -705,7 +725,7 @@ MdChipsCtrl.prototype.focusChip = function(index) { /** * Configures the required interactions with the ngModel Controller. - * Specifically, set {@code this.items} to the {@code NgModelCtrl#$viewVale}. + * Specifically, set {@code this.items} to the {@code NgModelCtrl#$viewValue}. * @param ngModelCtrl */ MdChipsCtrl.prototype.configureNgModel = function(ngModelCtrl) { diff --git a/src/components/chips/js/chipsDirective.js b/src/components/chips/js/chipsDirective.js index 44d14d8692b..742df538eab 100644 --- a/src/components/chips/js/chipsDirective.js +++ b/src/components/chips/js/chipsDirective.js @@ -73,7 +73,8 @@ * * * In some cases, you have an autocomplete inside of the `md-chips`.
- * When the maximum amount of chips has been reached, you can also disable the autocomplete selection.
+ * When the maximum amount of chips has been reached, you can also disable the autocomplete + * selection.
* Here is an example markup. * * @@ -96,10 +97,11 @@ * * Please refer to the documentation of this option (below) for more information. * - * @param {expression} ng-model Assignable angular expression to be data-bound to the list of chips. - * The expression should evaluate to a `string` or `Object` Array. The type of this array should align - * with the return value of `md-transform-chip`. - * @param {expression=} ng-change AngularJS expression to be executed on chip addition/removal. + * @param {expression} ng-model Assignable AngularJS expression to be data-bound to the list of + * chips. The expression should evaluate to a `string` or `Object` Array. The type of this + * array should align with the return value of `md-transform-chip`. + * @param {expression=} ng-change AngularJS expression to be executed on chip addition, removal, + * or content change. * @param {string=} placeholder Placeholder text that will be forwarded to the input. * @param {string=} secondary-placeholder Placeholder text that will be forwarded to the input, * displayed when there is at least one item in the list @@ -108,18 +110,19 @@ * @param {boolean=} readonly Disables list manipulation (deleting or adding list items), hiding * the input and delete buttons. If no `ng-model` is provided, the chips will automatically be * marked as readonly.

- * When `md-removable` is not defined, the `md-remove` behavior will be overwritten and disabled. - * @param {string=} md-enable-chip-edit Set this to "true" to enable editing of chip contents. The user can - * go into edit mode with pressing "space", "enter", or double clicking on the chip. Chip edit is only - * supported for chips with basic template. + * When `md-removable` is not defined, the `md-remove` behavior will be overwritten and + * disabled. + * @param {string=} md-enable-chip-edit Set this to "true" to enable editing of chip contents. + * The user can go into edit mode with pressing "space", "enter", or double clicking on the + * chip. Chip edit is only supported for chips with basic template. * @param {boolean=} ng-required Whether ng-model is allowed to be empty or not. * @param {number=} md-max-chips The maximum number of chips allowed to add through user input. *

The validation property `md-max-chips` can be used when the max chips * amount is reached. * @param {boolean=} md-add-on-blur When set to true, remaining text inside of the input will * be converted into a new chip on blur. - * @param {expression} md-transform-chip An expression of form `myFunction($chip)` that when called - * expects one of the following return values: + * @param {expression} md-transform-chip An expression of form `myFunction($chip)` that when + * called expects one of the following return values: * - an object representing the `$chip` input string * - `undefined` to simply add the `$chip` input string, or * - `null` to prevent the chip from being appended @@ -282,7 +285,8 @@ deleteButtonLabel: '@', separatorKeys: '=?mdSeparatorKeys', requireMatch: '=?mdRequireMatch', - chipAppendDelayString: '@?mdChipAppendDelay' + chipAppendDelayString: '@?mdChipAppendDelay', + ngChange: '&' } }; @@ -357,7 +361,7 @@ $mdTheming(element); var mdChipsCtrl = controllers[0]; - if(chipTemplate) { + if (chipTemplate) { // Chip editing functionality assumes we are using the default chip template. mdChipsCtrl.enableChipEdit = false; } @@ -382,6 +386,7 @@ // If an `md-on-append` attribute was set, tell the controller to use the expression // when appending chips. // + // TODO: Remove this now that 1.0 is long since released // DEPRECATED: Will remove in official 1.0 release if (attrs.mdOnAppend) mdChipsCtrl.useOnAppendExpression(); diff --git a/src/components/chips/js/contactChipsController.js b/src/components/chips/js/contactChipsController.js index 8ef7000c196..e88aaf97de9 100644 --- a/src/components/chips/js/contactChipsController.js +++ b/src/components/chips/js/contactChipsController.js @@ -2,8 +2,6 @@ angular .module('material.components.chips') .controller('MdContactChipsCtrl', MdContactChipsCtrl); - - /** * Controller for the MdContactChips component * @constructor @@ -16,12 +14,10 @@ function MdContactChipsCtrl () { this.searchText = ''; } - MdContactChipsCtrl.prototype.queryContact = function(searchText) { return this.contactQuery({'$query': searchText}); }; - MdContactChipsCtrl.prototype.itemName = function(item) { return item[this.contactName]; }; diff --git a/src/components/chips/js/contactChipsDirective.js b/src/components/chips/js/contactChipsDirective.js index 081bcb6069c..61efc20c8e3 100644 --- a/src/components/chips/js/contactChipsDirective.js +++ b/src/components/chips/js/contactChipsDirective.js @@ -16,8 +16,10 @@ angular * You may also use the `md-highlight-text` directive along with its parameters to control the * appearance of the matched text inside of the contacts' autocomplete popup. * - * @param {string=|object=} ng-model A model to bind the list of items to - * @param {expression=} ng-change AngularJS expression to be executed on chip addition/removal + * @param {expression} ng-model Assignable AngularJS expression to be data-bound to the list of + * contact chips. The expression should evaluate to an `Object` Array. + * @param {expression=} ng-change AngularJS expression to be executed on chip addition, removal, + * or content change. * @param {string=} placeholder Placeholder text that will be forwarded to the input. * @param {string=} secondary-placeholder Placeholder text that will be forwarded to the input, * displayed when there is at least on item in the list @@ -33,12 +35,6 @@ angular * @param {number=} md-min-length Specifies the minimum length of text before autocomplete will * make suggestions * - * @param {expression=} filter-selected Whether to filter selected contacts from the list of - * suggestions shown in the autocomplete. - * - * ***Note:** This attribute has been removed but may come back.* - * - * * * @usage *