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

fix(md-chips placeholder): correct placeholder/secondary logic #6535

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
42 changes: 42 additions & 0 deletions src/components/chips/chips.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,48 @@ describe('<md-chips>', function() {
expect(element.find('md-chips-wrap').hasClass('md-focused')).toBe(false);
}));

describe('placeholder', function() {

it('should put placeholder text in the input element when chips exist but there is no secondary-placeholder text', inject(function() {
var template =
'<md-chips ng-model="items" placeholder="placeholder text"></md-chips>';
var element = buildChips(template);
var ctrl = element.controller('mdChips');
var input = element.find('input')[0];

expect(scope.items.length).toBeGreaterThan(0);
expect(input.placeholder).toBe('placeholder text');
}));

it('should put placeholder text in the input element when there are no chips', inject(function() {
var ctrl, element, input, template;

scope.items = [];
template =
'<md-chips ng-model="items" placeholder="placeholder text" ' +
'secondary-placeholder="secondary-placeholder text"></md-chips>';
element = buildChips(template);
ctrl = element.controller('mdChips');
input = element.find('input')[0];

expect(scope.items.length).toBe(0);
expect(input.placeholder).toBe('placeholder text');
}));

it('should put secondary-placeholder text in the input element when there is at least one chip', inject(function() {
var template =
'<md-chips ng-model="items" placeholder="placeholder text" ' +
'secondary-placeholder="secondary-placeholder text"></md-chips>';
var element = buildChips(template);
var ctrl = element.controller('mdChips');
var input = element.find('input')[0];

expect(scope.items.length).toBeGreaterThan(0);
expect(input.placeholder).toBe('secondary-placeholder text');
}));

});

});

describe('custom inputs', function() {
Expand Down
2 changes: 1 addition & 1 deletion src/components/chips/js/chipsController.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ MdChipsCtrl.prototype.getPlaceholder = function() {
// Allow `secondary-placeholder` to be blank.
var useSecondary = (this.items.length &&
(this.secondaryPlaceholder == '' || this.secondaryPlaceholder));
return useSecondary ? this.placeholder : this.secondaryPlaceholder;
return useSecondary ? this.secondaryPlaceholder : this.placeholder;
Copy link
Contributor

Choose a reason for hiding this comment

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

@topherfangio - this logic looks wrong. Can you check.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope, our existing logic has been wrong the whole time. It currently says

if use secondary
    use regular placeholder
else
    use secondary placeholder

which is totally backwards. This fixes it. See #2770 for Tyler's comment about it being backwards.

};

/**
Expand Down