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

fix(chips): use empty chip buffer if not a string #9885

Merged
merged 1 commit into from
Nov 16, 2016
Merged

fix(chips): use empty chip buffer if not a string #9885

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
13 changes: 13 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 @@ -1224,6 +1224,19 @@ describe('<md-chips>', function() {
expect(scope.items.length).toBe(4);
expect(scope.items[3]).toBe('Grape');
}));

it('should use an empty string if ngModel value is falsy', inject(function($timeout) {
var element = buildChips(NG_MODEL_TEMPLATE);
var ctrl = element.controller('mdChips');

$timeout.flush();

var ngModelCtrl = ctrl.userInputNgModelCtrl;

expect(ngModelCtrl.$viewValue).toBeFalsy();
expect(ctrl.getChipBuffer()).toBe('');
}));

});

describe('without ngModel', function() {
Expand Down
11 changes: 7 additions & 4 deletions src/components/chips/js/chipsController.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,12 +383,15 @@ MdChipsCtrl.prototype.useOnSelectExpression = function() {
* model of an {@code md-autocomplete}, or, through some magic, the model
* bound to any inpput or text area element found within a
* {@code md-input-container} element.
* @return {Object|string}
* @return {string}
*/
MdChipsCtrl.prototype.getChipBuffer = function() {
return !this.userInputElement ? this.chipBuffer :
this.userInputNgModelCtrl ? this.userInputNgModelCtrl.$viewValue :
this.userInputElement[0].value;
var chipBuffer = !this.userInputElement ? this.chipBuffer :
Copy link
Member

@crisbeto crisbeto Oct 25, 2016

Choose a reason for hiding this comment

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

Seems like this code was here from before, but it might be a good chance to get rid of the nested ternary expression since it's a little hard to read/follow. Also the this.userInputElement[0].value can be reduced to this.userInputElement.val().

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.
return angular.isString(chipBuffer) ? chipBuffer : '';
Copy link
Contributor

@topherfangio topherfangio Oct 24, 2016

Choose a reason for hiding this comment

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

Is it ever possible for chipBuffer to be something like 3 instead of the string '3'? If so, this may fail?

Copy link
Member Author

@devversion devversion Oct 24, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, it could happen if the developer uses a custom input element or for example the md-autocomplete (and does have some custom ng-models)

This is kind of a safety check to ensure that there will be not Runtime errors ever.

Copy link
Contributor

@topherfangio topherfangio Oct 24, 2016

Choose a reason for hiding this comment

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

Indeed; I'm just wondering if we could alter this to cast the input to a string instead of returning an empty string.

So, perhaps something like:

return angular.isDefined(chipBuffer) ? chipBuffer + '' : ''

This will ensure that it is always a string, or the empty string, but should handle more cases. Thoughts?

Copy link
Member Author

@devversion devversion Oct 24, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I thought a lot about this, and figured that just returning an empty string is the safest & cleanest approach IMO

I think casting would open more questions than it would solve, because then users might get weird chips like [Object], rather than not showing any chip.

I used the isString to avoid objects being used as truthy chip buffers.

};

/**
Expand Down