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

Commit

Permalink
fix(): Add better warnings to materialAria
Browse files Browse the repository at this point in the history
Closes #366
  • Loading branch information
Marcy Sutton authored and ajoslin committed Oct 17, 2014
1 parent 96c0095 commit 3368c93
Show file tree
Hide file tree
Showing 17 changed files with 123 additions and 34 deletions.
19 changes: 18 additions & 1 deletion config/test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,23 @@ var TestUtil = {
test.after(function() {
angular.element.prototype.focus = focus;
});
}
},

/**
* Create a fake version of $$rAF that does things asynchronously
*/
mockRaf: function() {
module('ng', function($provide) {
$provide.value('$$rAF', mockRaf);

function mockRaf(cb) {
cb();
}
mockRaf.debounce = function(cb) {
return function() {
cb.apply(this, arguments);
};
};
});
}
};
2 changes: 1 addition & 1 deletion docs/app/js/app.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
var DocsApp = angular.module('docsApp', ['ngMaterial', 'ngRoute', 'angularytics', 'ngAria'])
var DocsApp = angular.module('docsApp', ['ngMaterial', 'ngRoute', 'angularytics'])

.config([
'COMPONENTS',
Expand Down
2 changes: 1 addition & 1 deletion src/components/button/button.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ function MdButtonDirective(ngHrefDirectives, $mdInkRipple, $mdAria, $mdUtil ) {
});

return function postLink(scope, element, attr) {
$mdAria.expect(element, 'aria-label', element.text());
$mdAria.expect(element, 'aria-label', true);
$mdInkRipple.attachButtonBehavior(element);
};
}
Expand Down
1 change: 1 addition & 0 deletions src/components/button/button.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
describe('md-button', function() {

beforeEach(TestUtil.mockRaf);
beforeEach(module('material.components.button'));

it('should have inner-anchor with attrs if href attr is given', inject(function($compile, $rootScope) {
Expand Down
8 changes: 4 additions & 4 deletions src/components/button/demoBasicUsage/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,19 @@

<section layout="vertical" layout-sm="horizontal" layout-align="center center">

<md-button class="md-button-fab">
<md-button class="md-button-fab" aria-label="Time">
<md-icon icon="/img/icons/ic_access_time_24px.svg" style="width: 24px; height: 24px;"></md-icon>
</md-button>

<md-button class="md-button-fab">
<md-button class="md-button-fab" aria-label="New document">
<md-icon icon="/img/icons/ic_insert_drive_file_24px.svg" style="width: 24px; height: 24px;"></md-icon>
</md-button>

<md-button class="md-button-fab" disabled>
<md-button class="md-button-fab" disabled aria-label="Comment">
<md-icon icon="/img/icons/ic_comment_24px.svg" style="width: 24px; height: 24px;"></md-icon>
</md-button>

<md-button class="md-button-fab md-theme-light-blue">
<md-button class="md-button-fab md-theme-light-blue" aria-label="Profile">
<md-icon icon="/img/icons/ic_people_24px.svg" style="width: 24px; height: 24px;"></md-icon>
</md-button>

Expand Down
5 changes: 2 additions & 3 deletions src/components/checkbox/checkbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,6 @@ function MdCheckboxDirective(inputDirectives, $mdInkRipple, $mdAria, $mdConstant
tAttrs.tabIndex = 0;
tElement.attr('role', tAttrs.type);

$mdAria.expect(tElement, 'aria-label', tElement.text());

return function postLink(scope, element, attr, ngModelCtrl) {
var checked = false;

Expand All @@ -92,6 +90,8 @@ function MdCheckboxDirective(inputDirectives, $mdInkRipple, $mdAria, $mdConstant
$formatters: []
};

$mdAria.expect(tElement, 'aria-label', true);

// Reuse the original input[type=checkbox] directive from Angular core.
// This is a bit hacky as we need our own event listener and own render
// function.
Expand Down Expand Up @@ -122,7 +122,6 @@ function MdCheckboxDirective(inputDirectives, $mdInkRipple, $mdAria, $mdConstant

function render() {
checked = ngModelCtrl.$viewValue;
// element.attr('aria-checked', checked);
if(checked) {
element.addClass(CHECKED_CSS);
} else {
Expand Down
23 changes: 23 additions & 0 deletions src/components/checkbox/checkbox.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,29 @@ describe('mdCheckbox', function() {

beforeEach(module('material.components.checkbox'));
beforeEach(module('ngAria'));
beforeEach(TestUtil.mockRaf);

it('should warn developers they need a label', inject(function($compile, $rootScope, $log){
spyOn($log, "warn");

var element = $compile('<div>' +
'<md-checkbox ng-model="blue">' +
'</md-checkbox>' +
'</div>')($rootScope);

expect($log.warn).toHaveBeenCalled();
}));

it('should copy text content to aria-label', inject(function($compile, $rootScope){
var element = $compile('<div>' +
'<md-checkbox ng-model="blue">' +
'Some text' +
'</md-checkbox>' +
'</div>')($rootScope);

var cbElements = element.find('md-checkbox');
expect(cbElements.eq(0).attr('aria-label')).toBe('Some text');
}));

it('should set checked css class and aria-checked attributes', inject(function($compile, $rootScope) {
var element = $compile('<div>' +
Expand Down
2 changes: 1 addition & 1 deletion src/components/dialog/dialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,6 @@ function MdDialogService($timeout, $rootElement, $mdEffects, $animate, $mdAria,
dialogContent = element;
}
var defaultText = $mdUtil.stringFromTextBody(dialogContent.text(), 3);
$mdAria.expect(element, 'aria-label', defaultText);
$mdAria.expect(element, 'aria-label', true, defaultText);
}
}
1 change: 1 addition & 0 deletions src/components/dialog/dialog.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
describe('$mdDialog', function() {

beforeEach(TestUtil.mockRaf);
beforeEach(module('material.components.dialog', 'ngAnimateMock'));

beforeEach(inject(function spyOnMdEffects($mdEffects, $$q, $animate) {
Expand Down
2 changes: 1 addition & 1 deletion src/components/radioButton/radioButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ function mdRadioButtonDirective($mdAria, $mdUtil) {
'aria-checked' : 'false'
});

$mdAria.expect(element, 'aria-label', element.text());
$mdAria.expect(element, 'aria-label', true);

/**
* Build a unique ID for each radio button that will be used with aria-activedescendant.
Expand Down
24 changes: 22 additions & 2 deletions src/components/radioButton/radioButton.spec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
describe('radioButton', function() {
var CHECKED_CSS = 'md-checked';

beforeEach(TestUtil.mockRaf);
beforeEach(module('material.components.radioButton'));

it('should set checked css class', inject(function($compile, $rootScope) {
Expand Down Expand Up @@ -31,7 +32,7 @@ describe('radioButton', function() {
expect(rbGroupElement.find('md-radio-button').eq(0).attr('role')).toEqual('radio');
}));

it('should set aria attributes', inject(function($compile, $rootScope) {
it('should set aria states', inject(function($compile, $rootScope) {
var element = $compile('<md-radio-group ng-model="color">' +
'<md-radio-button value="blue"></md-radio-button>' +
'<md-radio-button value="green"></md-radio-button>' +
Expand All @@ -50,12 +51,31 @@ describe('radioButton', function() {
expect(element.attr('aria-activedescendant')).not.toEqual(rbElements.eq(0).attr('id'));
}));

it('should be operable via arrow keys', inject(function($compile, $rootScope, $mdConstant) {
it('should warn developers they need a label', inject(function($compile, $rootScope, $log){
spyOn($log, "warn");
var element = $compile('<md-radio-group ng-model="color">' +
'<md-radio-button value="blue"></md-radio-button>' +
'<md-radio-button value="green"></md-radio-button>' +
'</md-radio-group>')($rootScope);

expect($log.warn).toHaveBeenCalled();
}));

it('should create an aria label from provided text', inject(function($compile, $rootScope) {
var element = $compile('<md-radio-group ng-model="color">' +
'<md-radio-button value="blue">Blue</md-radio-button>' +
'<md-radio-button value="green">Green</md-radio-button>' +
'</md-radio-group>')($rootScope);

var rbElements = element.find('md-radio-button');
expect(rbElements.eq(0).attr('aria-label')).toEqual('Blue');
}));

it('should be operable via arrow keys', inject(function($compile, $rootScope, $mdConstant) {
var element = $compile('<md-radio-group ng-model="color">' +
'<md-radio-button value="blue"></md-radio-button>' +
'<md-radio-button value="green"></md-radio-button>' +
'</md-radio-group>')($rootScope);
$rootScope.$apply(function(){
$rootScope.color = 'blue';
});
Expand Down
5 changes: 3 additions & 2 deletions src/components/slider/slider.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ function SliderController(scope, element, attr, $$rAF, $window, $mdEffects, $mdA
var trackContainer = angular.element(element[0].querySelector('.slider-track-container'));
var activeTrack = angular.element(element[0].querySelector('.slider-track-fill'));
var tickContainer = angular.element(element[0].querySelector('.slider-track-ticks'));
var throttledRefreshDimensions = $mdUtil.throttle(refreshSliderDimensions, 5000);

// Default values, overridable by attrs
attr.min ? attr.$observe('min', updateMin) : updateMin(0);
Expand All @@ -125,7 +126,8 @@ function SliderController(scope, element, attr, $$rAF, $window, $mdEffects, $mdA
updateAriaDisabled(!!attr.disabled);
}

$mdAria.expect(element, 'aria-label');
$mdAria.expect(element, 'aria-label', false);

element.attr('tabIndex', 0);
element.attr('role', 'slider');
element.on('keydown', keydownListener);
Expand Down Expand Up @@ -214,7 +216,6 @@ function SliderController(scope, element, attr, $$rAF, $window, $mdEffects, $mdA
* Refreshing Dimensions
*/
var sliderDimensions = {};
var throttledRefreshDimensions = $mdUtil.throttle(refreshSliderDimensions, 5000);
refreshSliderDimensions();
function refreshSliderDimensions() {
sliderDimensions = trackContainer[0].getBoundingClientRect();
Expand Down
16 changes: 16 additions & 0 deletions src/components/slider/slider.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ describe('md-slider', function() {
};
}

beforeEach(TestUtil.mockRaf);
beforeEach(module('ngAria'));
beforeEach(module('material.components.slider','material.decorators'));

it('should set model on press', inject(function($compile, $rootScope, $timeout) {
Expand Down Expand Up @@ -93,4 +95,18 @@ describe('md-slider', function() {
expect($rootScope.model).toBe(100);
}));

it('should warn developers they need a label', inject(function($compile, $rootScope, $timeout, $log) {
spyOn($log, "warn");

var element = $compile(
'<div>' +
'<md-slider min="100" max="104" step="2" ng-model="model"></md-slider>' +
'<md-slider min="0" max="100" ng-model="model2" aria-label="some label"></md-slider>' +
'</div>'
)($rootScope);

var sliders = element.find('md-slider');
expect($log.warn).toHaveBeenCalledWith(sliders[0]);
expect($log.warn).not.toHaveBeenCalledWith(sliders[1]);
}));
});
1 change: 1 addition & 0 deletions src/components/switch/switch.spec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
describe('<md-switch>', function() {
var CHECKED_CSS = 'md-checked';

beforeEach(TestUtil.mockRaf);
beforeEach(module('ngAria'));
beforeEach(module('material.components.switch'));

Expand Down
2 changes: 1 addition & 1 deletion src/components/tabs/js/tabItemDirective.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ function MdTabDirective($mdInkRipple, $compile, $mdAria, $mdUtil, $mdConstant) {
'aria-labelledby': tabId
});

$mdAria.expect(element, 'aria-label', element.text());
$mdAria.expect(element, 'aria-label', true);
}

};
Expand Down
1 change: 1 addition & 0 deletions src/components/tabs/tabs.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
describe('mdTabs directive', function() {

beforeEach(TestUtil.mockRaf);
beforeEach(module('material.components.tabs', 'material.decorators', 'material.services.aria'));

describe('controller', function(){
Expand Down
43 changes: 26 additions & 17 deletions src/services/aria/aria.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
angular.module('material.services.aria', [])

.service('$mdAria', [
'$$rAF',
'$log',
AriaService
]);

function AriaService($log) {
var messageTemplate = 'ARIA: Attribute "%s", required for accessibility, is missing on "%s"!';
function AriaService($$rAF, $log) {
var messageTemplate = 'ARIA: Attribute "%s", required for accessibility, is missing on "%s"';
var defaultValueTemplate = 'Default value was set: %s="%s".';

return {
Expand All @@ -17,23 +18,31 @@ function AriaService($log) {
* Check if expected ARIA has been specified on the target element
* @param element
* @param attrName
* @param defaultValue
* @param copyElementText
* @param {optional} defaultValue
*/
function expectAttribute(element, attrName, defaultValue) {

var node = element[0];
if (!node.hasAttribute(attrName)) {
var hasDefault = angular.isDefined(defaultValue);

if (hasDefault) {
defaultValue = String(defaultValue).trim();
// $log.warn(messageTemplate + ' ' + defaultValueTemplate,
// attrName, getTagString(node), attrName, defaultValue);
element.attr(attrName, defaultValue);
} else {
// $log.warn(messageTemplate, attrName, getTagString(node));
function expectAttribute(element, attrName, copyElementText, defaultValue) {

$$rAF(function(){

var node = element[0];
if (!node.hasAttribute(attrName)) {

var hasDefault;
if(copyElementText === true){
if(!defaultValue) defaultValue = element.text().trim();
hasDefault = angular.isDefined(defaultValue) && defaultValue.length;
}

if (hasDefault) {
defaultValue = String(defaultValue).trim();
element.attr(attrName, defaultValue);
} else {
$log.warn(messageTemplate, attrName, node);
$log.warn(node);
}
}
}
});
}


Expand Down

0 comments on commit 3368c93

Please sign in to comment.