Skip to content

Commit

Permalink
fix(input): duplicate placeholders and data bindings aria-label
Browse files Browse the repository at this point in the history
* Fixes the element label being duplicated, if it has a data binding in the placeholder attribute. This was caused by Angular re-adding the placeholder attribute, even though it was removed by the Material placeholder directive.
* Fixes the aria-label getting the raw data binding (e.g. `aria-label="{{expression}}"`), when it's gets taken from a dynamic placeholder.

Fixes angular#8251.
Fixes angular#8377.
  • Loading branch information
crisbeto committed Jun 3, 2016
1 parent c6c5d48 commit 21695be
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 27 deletions.
44 changes: 30 additions & 14 deletions src/components/input/input.js
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ function inputTextareaDirective($mdUtil, $window, $mdAria, $timeout, $mdGesture)
element.after(errorsSpacer);

if (!containerCtrl.label) {
$mdAria.expect(element, 'aria-label', element.attr('placeholder'));
$mdAria.expect(element, 'aria-label', attr.placeholder);
}

element.addClass('md-input');
Expand Down Expand Up @@ -660,15 +660,22 @@ function mdMaxlengthDirective($animate, $mdUtil) {
}
}

function placeholderDirective($log) {
function placeholderDirective($compile) {
return {
restrict: 'A',
require: '^^?mdInputContainer',
priority: 200,
link: postLink
link: {
// Note that we need to do this in the pre-link, as opposed to the post link, if we want to
// support data bindings in the placeholder. This is necessary, because we have a case where
// we transfer the placeholder value to the `<label>` and we remove it from the original `<input>`.
// If we did this in the post-link, Angular would have set up the observers already and would be
// re-adding the attribute, even though we removed it from the element.
pre: preLink
}
};

function postLink(scope, element, attr, inputContainer) {
function preLink(scope, element, attr, inputContainer) {
// If there is no input container, just return
if (!inputContainer) return;

Expand All @@ -682,16 +689,25 @@ function placeholderDirective($log) {
return;
}

// Otherwise, grab/remove the placeholder
var placeholderText = attr.placeholder;
element.removeAttr('placeholder');

// And add the placeholder text as a separate label
if (inputContainer.input && inputContainer.input[0].nodeName != 'MD-SELECT') {
var placeholder = '<label ng-click="delegateClick()">' + placeholderText + '</label>';

inputContainer.element.addClass('md-icon-float');
inputContainer.element.prepend(placeholder);
// md-select handles placeholders on it's own
if (element[0].nodeName != 'MD-SELECT') {
// Move the placeholder expression to the label
var newLabel = angular.element('<label ng-click="delegateClick()">' + attr.placeholder + '</label>');

// Note that we unset it via `attr`, in order to get Angular
// to remove any observers that it might have set up. Otherwise
// the attribute will be added on the next digest.
attr.$set('placeholder', null);

// We need to compile the label manually in case it has any bindings.
// A gotcha here is that we first add the element to the DOM and we compile
// it later. This is necessary, because if we compile the element beforehand,
// it won't be able to find the `mdInputContainer` controller.
inputContainer.element
.addClass('md-icon-float')
.prepend(newLabel);

$compile(newLabel)(scope);
}
}
}
Expand Down
65 changes: 52 additions & 13 deletions src/components/input/input.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ describe('md-input-container directive', function() {
expect(el.find('label').length).toBe(1);
});

it('should ignore placeholder when a label element is present', inject(function($rootScope, $compile) {
it('should ignore placeholder when a label element is present', function() {
var el = $compile(
'<md-input-container>' +
' <label>Hello</label>' +
Expand All @@ -408,19 +408,58 @@ describe('md-input-container directive', function() {
expect(el.find('input')[0].hasAttribute('placeholder')).toBe(true);
expect(label).toBeTruthy();
expect(label.textContent).toEqual('Hello');
}));
});

it('should transfer the placeholder data binding to the newly-created label', function() {
var el = $compile(
'<md-input-container>' +
' <input ng-model="foo" placeholder="{{placeholder}}" />' +
'</md-input-container>'
)(pageScope);

var label = el[0].querySelector('label');
var input = el[0].querySelector('input');

it('should put an aria-label on the input when no label is present', function() {
pageScope.placeholder = 'foo';
pageScope.$digest();

expect(label).toBeTruthy();

expect(input.hasAttribute('placeholder')).toBe(false);
expect(label.textContent).toEqual('foo');

pageScope.placeholder = 'bar';
pageScope.$digest();

// We should check again to make sure that Angular didn't
// re-add the placeholder attribute and cause double labels.
expect(input.hasAttribute('placeholder')).toBe(false);
expect(label.textContent).toEqual('bar');
});

it('should put an aria-label on the input when no label is present', inject(function($timeout) {
var el = $compile('<form name="form">' +
' <md-input-container md-no-float>' +
' <input placeholder="baz" md-maxlength="max" ng-model="foo" name="foo">' +
' <input placeholder="baz" ng-model="foo" name="foo">' +
' </md-input-container>' +
'</form>')(pageScope);

pageScope.$apply();
// Flushes the $mdUtil.nextTick
$timeout.flush();

var input = el.find('input');
expect(input.attr('aria-label')).toBe('baz');
}));

it('should evaluate the placeholder expression before setting the aria-label', function() {
pageScope.placeholder = 'baz';
var el = $compile('<form name="form">' +
' <md-input-container md-no-float>' +
' <input placeholder="{{placeholder}}" ng-model="foo" name="foo">' +
' </md-input-container>' +
'</form>')(pageScope);

expect(el.find('input').attr('aria-label')).toBe('baz');
});

it('should put the container in "has value" state when input has a static value', function() {
Expand All @@ -437,40 +476,40 @@ describe('md-input-container directive', function() {
expect(element.hasClass('md-input-has-value')).toBe(true);
});

it('adds the md-auto-hide class to messages without a visiblity directive', inject(function() {
it('adds the md-auto-hide class to messages without a visiblity directive', function() {
var el = compile(
'<md-input-container><input ng-model="foo">' +
' <div ng-messages></div>' +
'</md-input-container>'
);

expect(el[0].querySelector("[ng-messages]").classList.contains('md-auto-hide')).toBe(true);
}));
});

it('does not add the md-auto-hide class with md-auto-hide="false" on the messages', inject(function() {
it('does not add the md-auto-hide class with md-auto-hide="false" on the messages', function() {
var el = compile(
'<md-input-container><input ng-model="foo">' +
' <div ng-messages md-auto-hide="false">Test Message</div>' +
'</md-input-container>'
);

expect(el[0].querySelector("[ng-messages]").classList.contains('md-auto-hide')).toBe(false);
}));
});

var visibilityDirectives = ['ng-if', 'ng-show', 'ng-hide'];
visibilityDirectives.forEach(function(vdir) {
it('does not add the md-auto-hide class with ' + vdir + ' on the messages', inject(function() {
it('does not add the md-auto-hide class with ' + vdir + ' on the messages', function() {
var el = compile(
'<md-input-container><input ng-model="foo">' +
' <div ng-messages ' + vdir + '="true">Test Message</div>' +
'</md-input-container>'
);

expect(el[0].querySelector("[ng-messages]").classList.contains('md-auto-hide')).toBe(false);
}));
});
});

it('does not add the md-auto-hide class with ngSwitch on the messages', inject(function() {
it('does not add the md-auto-hide class with ngSwitch on the messages', function() {
pageScope.switchVal = 1;

var el = compile(
Expand All @@ -483,7 +522,7 @@ describe('md-input-container directive', function() {
);

expect(el[0].querySelector("[ng-messages]").classList.contains('md-auto-hide')).toBe(false);
}));
});

it('should set the animation class on the ngMessage properly', inject(function() {
var element = compile(
Expand Down

0 comments on commit 21695be

Please sign in to comment.