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

Commit

Permalink
fix(chips): regression where chips model gets out of sync with view (#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
Splaktar authored and jelbourn committed Jun 13, 2018
1 parent a85f038 commit 74d2445
Show file tree
Hide file tree
Showing 13 changed files with 136 additions and 64 deletions.
2 changes: 1 addition & 1 deletion src/components/autocomplete/demoBasicUsage/script.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/components/autocomplete/demoCustomTemplate/script.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/components/autocomplete/demoFloatingLabel/script.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/components/autocomplete/demoInsideDialog/script.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
32 changes: 29 additions & 3 deletions src/components/chips/chips.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe('<md-chips>', 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_;
Expand Down Expand Up @@ -177,6 +177,30 @@ describe('<md-chips>', 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);
Expand Down Expand Up @@ -217,16 +241,17 @@ describe('<md-chips>', 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');
Expand Down Expand Up @@ -694,7 +719,8 @@ describe('<md-chips>', 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();
Expand Down
38 changes: 36 additions & 2 deletions src/components/chips/contact-chips.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@ describe('<md-contact-chips>', function() {
md-highlight-flags="i"\
md-min-length="1"\
md-chip-append-delay="2000"\
ng-change="onModelChange(contacts)"\
placeholder="To">\
</md-contact-chips>';

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 = [
{
Expand All @@ -35,7 +36,7 @@ describe('<md-contact-chips>', function() {
];
scope.contacts = [];

scope.highlightFlags = "i";
scope.highlightFlags = 'i';
}));

var attachedElements = [];
Expand Down Expand Up @@ -65,6 +66,31 @@ describe('<md-contact-chips>', 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');
Expand Down Expand Up @@ -167,4 +193,12 @@ describe('<md-contact-chips>', 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);
}
});
2 changes: 0 additions & 2 deletions src/components/chips/demoContactChips/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
md-contact-email="email"
md-require-match="true"
md-highlight-flags="i"
filter-selected="ctrl.filterSelected"
placeholder="To">
</md-contact-chips>

Expand Down Expand Up @@ -43,7 +42,6 @@ <h2 class="md-title">Searching asynchronously.</h2>
md-contact-email="email"
md-require-match="true"
md-highlight-flags="i"
filter-selected="ctrl.filterSelected"
placeholder="To">
</md-contact-chips>
</md-content>
Expand Down
17 changes: 7 additions & 10 deletions src/components/chips/demoContactChips/script.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -13,15 +13,14 @@
.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;

self.allContacts = loadContacts();
self.contacts = [self.allContacts[0]];
self.asyncContacts = [];
self.filterSelected = true;

self.querySearch = querySearch;
self.delayedQuerySearch = delayedQuerySearch;
Expand All @@ -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) {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -117,6 +116,4 @@
});
}
}


})();
2 changes: 1 addition & 1 deletion src/components/chips/demoCustomInputs/script.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) ||
Expand Down
54 changes: 37 additions & 17 deletions src/components/chips/js/chipsController.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}
};

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 });
}
Expand Down Expand Up @@ -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 : '';
};

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit 74d2445

Please sign in to comment.