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

fix(input): duplicate placeholders and data bindings aria-label #8291

Closed
wants to merge 1 commit into from
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
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