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 committed Oct 14, 2014
1 parent 3f5c47b commit 2ea9e3f
Show file tree
Hide file tree
Showing 16 changed files with 122 additions and 32 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 src/components/buttons/buttons.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ function MaterialButtonDirective(ngHrefDirectives, $materialInkRipple, $material
});

return function postLink(scope, element, attr) {
$materialAria.expect(element, 'aria-label', element.text());
$materialAria.expect(element, 'aria-label', true);
$materialInkRipple.attachButtonBehavior(element);
};
}
Expand Down
1 change: 1 addition & 0 deletions src/components/buttons/buttons.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
describe('material-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/buttons/demo1/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">

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

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

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

<material-button class="material-button-fab material-theme-light-blue">
<material-button class="material-button-fab material-theme-light-blue" aria-label="People">
<material-icon icon="/img/icons/ic_people_24px.svg" style="width: 24px; height: 24px;"></material-icon>
</material-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 @@ -76,8 +76,6 @@ function MaterialCheckboxDirective(inputDirectives, $materialInkRipple, $materia
tAttrs.tabIndex = 0;
tElement.attr('role', tAttrs.type);

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

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

Expand All @@ -90,6 +88,8 @@ function MaterialCheckboxDirective(inputDirectives, $materialInkRipple, $materia
$formatters: []
};

$materialAria.expect(element, '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 @@ -120,7 +120,6 @@ function MaterialCheckboxDirective(inputDirectives, $materialInkRipple, $materia

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('materialCheckbox', 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>' +
'<material-checkbox ng-model="blue">' +
'</material-checkbox>' +
'</div>')($rootScope);

var cbElements = element.find('material-checkbox');
expect($log.warn).toHaveBeenCalled();
}));

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

var cbElements = element.find('material-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 @@ -247,6 +247,6 @@ function MaterialDialogService($timeout, $rootElement, $materialEffects, $animat
dialogContent = element;
}
var defaultText = Util.stringFromTextBody(dialogContent.text(), 3);
$materialAria.expect(element, 'aria-label', defaultText);
$materialAria.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('$materialDialog', function() {

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

beforeEach(inject(function spyOnMaterialEffects($materialEffects, $$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 @@ -252,7 +252,7 @@ function materialRadioButtonDirective($materialAria) {
'aria-checked' : 'false'
});

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

/**
* Build a unique ID for each radio button that will be used with aria-activedescendant.
Expand Down
23 changes: 22 additions & 1 deletion 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 = 'material-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('material-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('<material-radio-group ng-model="color">' +
'<material-radio-button value="blue"></material-radio-button>' +
'<material-radio-button value="green"></material-radio-button>' +
Expand All @@ -50,6 +51,26 @@ describe('radioButton', function() {
expect(element.attr('aria-activedescendant')).not.toEqual(rbElements.eq(0).attr('id'));
}));

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

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

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

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

it('should be operable via arrow keys', inject(function($compile, $rootScope) {
var element = $compile('<material-radio-group ng-model="color">' +
'<material-radio-button value="blue"></material-radio-button>' +
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 @@ -106,6 +106,7 @@ function SliderController(scope, element, attr, $$rAF, $window, $materialEffects
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 = Util.throttle(refreshSliderDimensions, 5000);

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

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

element.attr('tabIndex', 0);
element.attr('role', 'slider');
element.on('keydown', keydownListener);
Expand Down Expand Up @@ -209,7 +211,6 @@ function SliderController(scope, element, attr, $$rAF, $window, $materialEffects
* Refreshing Dimensions
*/
var sliderDimensions = {};
var throttledRefreshDimensions = Util.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('material-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('material-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>' +
'<material-slider min="100" max="104" step="2" ng-model="model"></material-slider>' +
'<material-slider min="0" max="100" ng-model="model2" aria-label="some label"></material-slider>' +
'</div>'
)($rootScope);

var sliders = element.find('material-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('<material-switch>', function() {
var CHECKED_CSS = 'material-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 @@ -200,7 +200,7 @@ function MaterialTabDirective($materialInkRipple, $compile, $materialAria) {
'aria-labelledby': tabId
});

$materialAria.expect(element, 'aria-label', element.text());
$materialAria.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('materialTabs 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('$materialAria', [
'$$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 2ea9e3f

Please sign in to comment.