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

fix(md-chips): appendChip disallows identical objects #4479

Closed
wants to merge 3 commits into from
Closed

fix(md-chips): appendChip disallows identical objects #4479

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
be910ab
fix(md-chips): appendChip disallows identical objects
JordanMajd Sep 5, 2015
46be703
perf(md-chips): appendChips object check loop optimized
JordanMajd Sep 10, 2015
62a0542
style(md-chips): adds comments and whitespace to appendChip
JordanMajd Sep 10, 2015
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
24 changes: 24 additions & 0 deletions src/components/chips/chips.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,30 @@ describe('<md-chips>', function() {
expect(scope.appendChip.calls.mostRecent().args[0]).toBe('Apple');
});

it('should disallow identical object chips', function() {
var element = buildChips(CHIP_APPEND_TEMPLATE);
var ctrl = element.controller('mdChips');

ctrl.items = [{name: 'Apple', uppername: 'APPLE'}];

var chipObj = function(chip) {
return {
name: chip,
uppername: chip.toUpperCase()
};
};
scope.appendChip = jasmine.createSpy('appendChip').and.callFake(chipObj);

element.scope().$apply(function() {
ctrl.chipBuffer = 'Apple';
simulateInputEnterKey(ctrl);
});

expect(ctrl.items.length).toBe(1);
expect(scope.appendChip).toHaveBeenCalled();
expect(scope.appendChip.calls.mostRecent().args[0]).toBe('Apple');
});

it('should prevent the default when backspace is pressed', inject(function($mdConstant) {
var element = buildChips(BASIC_CHIP_TEMPLATE);
var ctrl = element.controller('mdChips');
Expand Down
20 changes: 13 additions & 7 deletions src/components/chips/js/chipsController.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,19 @@ MdChipsCtrl.prototype.getAdjacentChipIndex = function(index) {
* call out to the md-on-append method, if provided
* @param newChip
*/
MdChipsCtrl.prototype.appendChip = function(newChip) {
if (this.useOnAppend && this.onAppend) {
newChip = this.onAppend({'$chip': newChip});
}
if (this.items.indexOf(newChip) + 1) return;
this.items.push(newChip);
};
MdChipsCtrl.prototype.appendChip = function(newChip) {
if (this.useOnAppend && this.onAppend) {
newChip = this.onAppend({'$chip': newChip});
}
if(typeof newChip === 'object'){
Copy link
Contributor

Choose a reason for hiding this comment

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

I might use angular.isObject() here.

Also, if you could add some whitespace and a few comments describing what each of the ifs check for, I think it would help readability.

Otherwise, LGTM :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea,

I'm on it.

var identical = this.items.some(function(item){
return angular.equals(newChip, item);
});
if(identical) return;
}
if (this.items.indexOf(newChip) + 1) return;
this.items.push(newChip);
};

/**
* Sets whether to use the md-on-append expression. This expression is
Expand Down