Default to using tagName div with role of region and updating asserti…

…on and change to warning to reflect intended use in comments
ember-a11y · Nov 8, 2017 · bebc185 · bebc185
1 parent bb7baa8
commit bebc185
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 16 deletions.
diff --git a/addon/components/a11y-landmark.js b/addon/components/a11y-landmark.js
@@ -8,7 +8,7 @@ const LANDMARK_NAVIGATION_ROLE = {
     main: 'main',
     footer: 'contentinfo',
     form: 'form',
-    div: 'div'
+    div: 'region'
 };
 
 const VALID_LANDMARK_ROLES = [
@@ -46,10 +46,10 @@ export default Ember.Component.extend({
      * nav (navigation)
      * div (application, document, region or any of the previous)
      */
-    tagName: null,
+    tagName: 'div',
 
     /*should only be set if ('div' or 'form') is being used as a tagName, otherwise we don't need it.
-     * valid values: 
+     * valid values:
      * banner
      * navigation
      * aside
@@ -62,8 +62,8 @@ export default Ember.Component.extend({
      */
     landmarkRole: null,
 
-    /*we should set an aria-role when either a native element is not used, or the native element does not have the body element as its parent. 
-     * since nothing is going to be the direct child of the body in an Ember app, we don't have to check for that. 
+    /*we should set an aria-role when either a native element is not used, or the native element does not have the body element as its parent.
+     * since nothing is going to be the direct child of the body in an Ember app, we don't have to check for that.
      */
     ariaRole: Ember.computed('tagName', 'landmarkRole', function() {
         const landmark = this.get('tagName');
@@ -74,8 +74,11 @@ export default Ember.Component.extend({
                 return 'search';
             } else if (landmark != 'form' && landmarkRole === 'search') {
                 Ember.assert('This is not a valid combination. Use the form element for a search.');
+            } else if (landmark === 'div') {
+                this._validateLandmarkRole(landmarkRole);
+                return landmarkRole;
             } else {
-                Ember.assert('Cannot set both "tagName" and "landMarkRole." Use one or the other.');
+                Ember.assert('Only "div" or "form" can be used with "landMarkRole." Use one or the other.');
             }
         } else if (landmarkRole) {
             this._validateLandmarkRole(landmarkRole);
@@ -84,7 +87,10 @@ export default Ember.Component.extend({
             this._validateTagName(landmark);
             return LANDMARK_NAVIGATION_ROLE[landmark];
         } else {
-            Ember.assert('Must specify either tagName or landmarkRole');
+            Ember.warn('Should specify either tagName or landmarkRole', {
+                id: 'ember-a11y.no-tagName-or-landmarkRole'
+            });
+            return 'region';
         }
     }),
 
@@ -108,4 +114,4 @@ export default Ember.Component.extend({
 
     // add support for an aria-label, since a landmark element can have one defined.
     ariaLabel: null
-});
+});
diff --git a/tests/unit/components/a11y-landmark-test.js b/tests/unit/components/a11y-landmark-test.js
@@ -76,7 +76,7 @@ test('it fails to determine ariaRole when landmarkRole is invalid', function(ass
 });
 
 test('it fails to determine ariaRole when tagName and landmarkRole are both provided', function(assert) {
-    const expectedErrorMessage = 'Cannot set both \"tagName\" and \"landMarkRole.\" Use one or the other.';
+    const expectedErrorMessage = 'Only "div" or "form" can be used with "landMarkRole." Use one or the other.';
 
     assert.throws(() => {
         const component = this.subject({
@@ -87,11 +87,22 @@ test('it fails to determine ariaRole when tagName and landmarkRole are both prov
     }, buildEmberAssertion(expectedErrorMessage));
 });
 
-test('it fails to determine ariaRole when neither tagName nor landmarkRole are provided', function(assert) {
-    const expectedErrorMessage = 'Must specify either tagName or landmarkRole';
+test('it defaults to div[role="region"] when neither tagName nor landmarkRole are provided', function(assert) {
+    const component = this.subject({});
 
-    assert.throws(() => {
-        const component = this.subject();
-        component.get('ariaRole');
-    }, buildEmberAssertion(expectedErrorMessage));
-});
+    const ariaRole = component.get('ariaRole');
+    assert.equal(ariaRole, 'region');
+
+    const tagName = component.get('tagName');
+    assert.equal(tagName, 'div');
+});
+
+test('it sets tagName correctly when tagName is "form" and landmarkRole is "search"', function(assert) {
+    const component = this.subject({
+        tagName: 'form',
+        landmarkRole: 'search'
+    });
+    const tagName = component.get('tagName');
+
+    assert.equal(tagName, 'form');
+});