Add component accessibility auditing and fixes (#8679)

This continues #8455 by adding accessibility audits to component integration tests and fixing associated errors. It adds audits to existing tests rather than adding separate ones to facilitate auditing the various permutations a component’s rendering can go through. It also adds linting to ensure audits happen in component tests. This necessitated consolidating test files that were scattered.
hashicorp · Aug 25, 2020 · 5eddb14 · 5eddb14
1 parent b511af9
commit 5eddb14
Show file tree

Hide file tree

Showing 71 changed files with 379 additions and 132 deletions.
diff --git a/ui/app/templates/components/fs/file.hbs b/ui/app/templates/components/fs/file.hbs
@@ -16,7 +16,7 @@
       <button data-test-log-action="tail" class="button is-white is-compact" onclick={{action "gotoTail"}} type="button">Tail</button>
     {{/if}}
     {{#if this.isStreamable}}
-      <button data-test-log-action="toggle-stream" class="button is-white is-compact" onclick={{action "toggleStream"}} type="button">
+      <button data-test-log-action="toggle-stream" class="button is-white is-compact" onclick={{action "toggleStream"}} type="button" title="{{if this.logger.isStreaming "Pause" "Start"}} streaming">
         {{x-icon (if this.logger.isStreaming "media-pause" "media-play") class="is-text"}}
       </button>
     {{/if}}

diff --git a/ui/app/templates/components/multi-select-dropdown.hbs b/ui/app/templates/components/multi-select-dropdown.hbs
@@ -4,7 +4,7 @@
   @onClose={{action (mut this.isOpen) false}} as |dd|
 >
   <dd.Trigger data-test-dropdown-trigger class="dropdown-trigger" {{on "keydown" (action "openOnArrowDown" dd)}}>
-    <div class="dropdown-trigger-label">
+    <div class="dropdown-trigger-label" id="{{this.elementId}}-label">
       {{this.label}}
       {{#if this.selection.length}}
         <span data-test-dropdown-count class="tag is-light">
@@ -15,29 +15,34 @@
     <span class="dropdown-trigger-icon ember-power-select-status-icon"></span>
   </dd.Trigger>
   <dd.Content class="dropdown-options">
-    <ul role="listbox" data-test-dropdown-options>
-      {{#each this.options key="key" as |option|}}
-        <li
-          data-test-dropdown-option={{option.key}}
-          class="dropdown-option"
-          tabindex="0"
-          onkeydown={{action "traverseList" option}}
-        >
-          <label>
-            <input
-              type="checkbox"
-              tabindex="-1"
-              checked={{contains option.key this.selection}}
-              onchange={{action "toggle" option}}
-            />
-            {{option.label}}
-          </label>
-        </li>
-      {{else}}
-        <em data-test-dropdown-empty class="dropdown-empty">
+    {{#if this.options}}
+      <ul role="listbox" aria-labelledby="{{this.elementId}}-label" data-test-dropdown-options>
+        {{#each this.options key="key" as |option|}}
+          <div
+            data-test-dropdown-option={{option.key}}
+            class="dropdown-option"
+            tabindex="0"
+            onkeydown={{action "traverseList" option}}
+          >
+            <label>
+              <input
+                type="checkbox"
+                tabindex="-1"
+                checked={{contains option.key this.selection}}
+                role="option"
+                onchange={{action "toggle" option}}
+              />
+              {{option.label}}
+            </label>
+          </div>
+        {{/each}}
+      </ul>
+    {{else}}
+      <ul aria-labelledby="{{this.elementId}}-label" data-test-dropdown-options>
+        <li data-test-dropdown-empty class="dropdown-empty">
           No options
-        </em>
-      {{/each}}
-    </ul>
+        </li>
+      </ul>
+    {{/if}}
   </dd.Content>
 </BasicDropdown>
diff --git a/ui/tests/.eslintrc.js b/ui/tests/.eslintrc.js
@@ -10,19 +10,36 @@ module.exports = {
   env: {
     embertest: true,
   },
-  overrides: {
-    files: ['acceptance/**/*-test.js'],
-    plugins: ['ember-a11y-testing'],
-    rules: {
-      'ember-a11y-testing/a11y-audit-called': 'error',
+  overrides: [
+    {
+      files: ['acceptance/**/*-test.js'],
+      plugins: ['ember-a11y-testing'],
+      rules: {
+        'ember-a11y-testing/a11y-audit-called': 'error',
+      },
+      settings: {
+        'ember-a11y-testing': {
+          auditModule: {
+            package: 'nomad-ui/tests/helpers/a11y-audit',
+            exportName: 'default',
+          },
+        },
+      },
     },
-    settings: {
-      'ember-a11y-testing': {
-        auditModule: {
-          package: 'nomad-ui/tests/helpers/a11y-audit',
-          exportName: 'default',
+    {
+      files: ['integration/components/**/*-test.js'],
+      plugins: ['ember-a11y-testing'],
+      rules: {
+        'ember-a11y-testing/a11y-audit-called': 'error',
+      },
+      settings: {
+        'ember-a11y-testing': {
+          auditModule: {
+            package: 'nomad-ui/tests/helpers/a11y-audit',
+            exportName: 'componentA11yAudit',
+          },
         },
       },
     },
-  },
+  ],
 };
diff --git a/ui/tests/acceptance/allocation-detail-test.js b/ui/tests/acceptance/allocation-detail-test.js
@@ -49,8 +49,7 @@ module('Acceptance | allocation detail', function(hooks) {
   });
 
   test('it passes an accessibility audit', async function(assert) {
-    await a11yAudit();
-    assert.ok(true, 'a11y audit passes');
+    await a11yAudit(assert);
   });
 
   test('/allocation/:id should name the allocation and link to the corresponding job and node', async function(assert) {

diff --git a/ui/tests/acceptance/application-errors-test.js b/ui/tests/acceptance/application-errors-test.js
@@ -20,9 +20,7 @@ module('Acceptance | application errors ', function(hooks) {
   test('it passes an accessibility audit', async function(assert) {
     server.pretender.get('/v1/nodes', () => [500, {}, null]);
     await ClientsList.visit();
-    await a11yAudit();
-
-    assert.ok(true, 'a11y audit passes');
+    await a11yAudit(assert);
   });
 
   test('transitioning away from an error page resets the global error', async function(assert) {

diff --git a/ui/tests/acceptance/behaviors/fs.js b/ui/tests/acceptance/behaviors/fs.js
@@ -24,17 +24,31 @@ const fileSort = (prop, files) => {
   return dir.sortBy(prop).concat(file.sortBy(prop));
 };
 
-export default function browseFilesystem({ pageObjectVisitPathFunctionName, pageObjectVisitFunctionName, visitSegments, getExpectedPathBase, getTitleComponent, getBreadcrumbComponent, getFilesystemRoot }) {
+export default function browseFilesystem({
+  pageObjectVisitPathFunctionName,
+  pageObjectVisitFunctionName,
+  visitSegments,
+  getExpectedPathBase,
+  getTitleComponent,
+  getBreadcrumbComponent,
+  getFilesystemRoot,
+}) {
   test('it passes an accessibility audit', async function(assert) {
-    await FS[pageObjectVisitFunctionName](visitSegments({allocation: this.allocation, task: this.task }));
-    await a11yAudit();
-    assert.ok(true, 'a11y audit passes');
+    await FS[pageObjectVisitFunctionName](
+      visitSegments({ allocation: this.allocation, task: this.task })
+    );
+    await a11yAudit(assert);
   });
 
   test('visiting filesystem root', async function(assert) {
-    await FS[pageObjectVisitFunctionName](visitSegments({allocation: this.allocation, task: this.task }));
+    await FS[pageObjectVisitFunctionName](
+      visitSegments({ allocation: this.allocation, task: this.task })
+    );
 
-    const pathBaseWithTrailingSlash = getExpectedPathBase({ allocation: this.allocation, task: this.task });
+    const pathBaseWithTrailingSlash = getExpectedPathBase({
+      allocation: this.allocation,
+      task: this.task,
+    });
     const pathBaseWithoutTrailingSlash = pathBaseWithTrailingSlash.slice(0, -1);
 
     assert.equal(currentURL(), pathBaseWithoutTrailingSlash, 'No redirect');
@@ -50,17 +64,32 @@ export default function browseFilesystem({ pageObjectVisitPathFunctionName, page
         pathWithLeadingSlash = `/${filePath}`;
       }
 
-      await FS[pageObjectVisitPathFunctionName]({ ...visitSegments({allocation: this.allocation, task: this.task }), path: filePath });
+      await FS[pageObjectVisitPathFunctionName]({
+        ...visitSegments({ allocation: this.allocation, task: this.task }),
+        path: filePath,
+      });
       assert.equal(
         currentURL(),
-        `${getExpectedPathBase({allocation: this.allocation, task: this.task })}${encodeURIComponent(filePath)}`,
+        `${getExpectedPathBase({
+          allocation: this.allocation,
+          task: this.task,
+        })}${encodeURIComponent(filePath)}`,
         'No redirect'
       );
       assert.equal(
         document.title,
-        `${pathWithLeadingSlash} - ${getTitleComponent({allocation: this.allocation, task: this.task})} - Nomad`
+        `${pathWithLeadingSlash} - ${getTitleComponent({
+          allocation: this.allocation,
+          task: this.task,
+        })} - Nomad`
+      );
+      assert.equal(
+        FS.breadcrumbsText,
+        `${getBreadcrumbComponent({
+          allocation: this.allocation,
+          task: this.task,
+        })} ${filePath.replace(/\//g, ' ')}`.trim()
       );
-      assert.equal(FS.breadcrumbsText, `${getBreadcrumbComponent({allocation: this.allocation, task: this.task})} ${filePath.replace(/\//g, ' ')}`.trim());
     };
 
     await paths.reduce(async (prev, filePath) => {
@@ -73,7 +102,10 @@ export default function browseFilesystem({ pageObjectVisitPathFunctionName, page
     const objects = { allocation: this.allocation, task: this.task };
     await FS[pageObjectVisitPathFunctionName]({ ...visitSegments(objects), path: '/' });
 
-    const sortedFiles = fileSort('name', filesForPath(this.server.schema.allocFiles, getFilesystemRoot(objects)).models);
+    const sortedFiles = fileSort(
+      'name',
+      filesForPath(this.server.schema.allocFiles, getFilesystemRoot(objects)).models
+    );
 
     assert.ok(FS.fileViewer.isHidden);
 
@@ -123,7 +155,10 @@ export default function browseFilesystem({ pageObjectVisitPathFunctionName, page
     );
 
     assert.equal(FS.breadcrumbs.length, 3);
-    assert.equal(FS.breadcrumbsText, `${getBreadcrumbComponent(objects)} ${this.directory.name} ${this.nestedDirectory.name}`);
+    assert.equal(
+      FS.breadcrumbsText,
+      `${getBreadcrumbComponent(objects)} ${this.directory.name} ${this.nestedDirectory.name}`
+    );
     assert.equal(FS.breadcrumbs[2].text, this.nestedDirectory.name);
 
     assert.notOk(
@@ -190,7 +225,10 @@ export default function browseFilesystem({ pageObjectVisitPathFunctionName, page
       ];
     });
 
-    await FS[pageObjectVisitPathFunctionName]({ ...visitSegments({allocation: this.allocation, task: this.task }), path: '/' });
+    await FS[pageObjectVisitPathFunctionName]({
+      ...visitSegments({ allocation: this.allocation, task: this.task }),
+      path: '/',
+    });
 
     assert.deepEqual(FS.directoryEntryNames(), [
       'aaa-big-old-directory',
@@ -275,7 +313,10 @@ export default function browseFilesystem({ pageObjectVisitPathFunctionName, page
 
     await FS[pageObjectVisitPathFunctionName]({ ...visitSegments(objects), path: '/' });
 
-    const sortedFiles = fileSort('name', filesForPath(this.server.schema.allocFiles, getFilesystemRoot(objects)).models);
+    const sortedFiles = fileSort(
+      'name',
+      filesForPath(this.server.schema.allocFiles, getFilesystemRoot(objects)).models
+    );
     const fileRecord = sortedFiles.find(f => !f.isDir);
     const fileIndex = sortedFiles.indexOf(fileRecord);
 
@@ -303,7 +344,10 @@ export default function browseFilesystem({ pageObjectVisitPathFunctionName, page
   });
 
   test('viewing an empty directory', async function(assert) {
-    await FS[pageObjectVisitPathFunctionName]({ ...visitSegments({ allocation: this.allocation, task: this.task }), path: 'empty-directory' });
+    await FS[pageObjectVisitPathFunctionName]({
+      ...visitSegments({ allocation: this.allocation, task: this.task }),
+      path: 'empty-directory',
+    });
 
     assert.ok(FS.isEmptyDirectory);
   });
@@ -313,7 +357,10 @@ export default function browseFilesystem({ pageObjectVisitPathFunctionName, page
       return new Response(500, {}, 'no such file or directory');
     });
 
-    await FS[pageObjectVisitPathFunctionName]({ ...visitSegments({ allocation: this.allocation, task: this.task }), path: '/what-is-this' });
+    await FS[pageObjectVisitPathFunctionName]({
+      ...visitSegments({ allocation: this.allocation, task: this.task }),
+      path: '/what-is-this',
+    });
     assert.equal(FS.error.title, 'Not Found', '500 is interpreted as 404');
 
     await visit('/');
@@ -322,7 +369,10 @@ export default function browseFilesystem({ pageObjectVisitPathFunctionName, page
       return new Response(999);
     });
 
-    await FS[pageObjectVisitPathFunctionName]({ ...visitSegments({ allocation: this.allocation, task: this.task }), path: '/what-is-this' });
+    await FS[pageObjectVisitPathFunctionName]({
+      ...visitSegments({ allocation: this.allocation, task: this.task }),
+      path: '/what-is-this',
+    });
     assert.equal(FS.error.title, 'Error', 'other statuses are passed through');
   });
 
@@ -331,7 +381,10 @@ export default function browseFilesystem({ pageObjectVisitPathFunctionName, page
       return new Response(500, {}, 'no such file or directory');
     });
 
-    await FS[pageObjectVisitPathFunctionName]({ ...visitSegments({ allocation: this.allocation, task: this.task }), path: this.directory.name });
+    await FS[pageObjectVisitPathFunctionName]({
+      ...visitSegments({ allocation: this.allocation, task: this.task }),
+      path: this.directory.name,
+    });
     assert.equal(FS.error.title, 'Not Found', '500 is interpreted as 404');
 
     await visit('/');
@@ -340,7 +393,10 @@ export default function browseFilesystem({ pageObjectVisitPathFunctionName, page
       return new Response(999);
     });
 
-    await FS[pageObjectVisitPathFunctionName]({ ...visitSegments({ allocation: this.allocation, task: this.task }), path: this.directory.name });
+    await FS[pageObjectVisitPathFunctionName]({
+      ...visitSegments({ allocation: this.allocation, task: this.task }),
+      path: this.directory.name,
+    });
     assert.equal(FS.error.title, 'Error', 'other statuses are passed through');
   });
 }
diff --git a/ui/tests/acceptance/client-detail-test.js b/ui/tests/acceptance/client-detail-test.js
@@ -44,8 +44,7 @@ module('Acceptance | client detail', function(hooks) {
 
   test('it passes an accessibility audit', async function(assert) {
     await ClientDetail.visit({ id: node.id });
-    await a11yAudit();
-    assert.ok(true, 'a11y audit passes');
+    await a11yAudit(assert);
   });
 
   test('/clients/:id should have a breadcrumb trail linking back to clients', async function(assert) {

diff --git a/ui/tests/acceptance/client-monitor-test.js b/ui/tests/acceptance/client-monitor-test.js
@@ -28,8 +28,7 @@ module('Acceptance | client monitor', function(hooks) {
 
   test('it passes an accessibility audit', async function(assert) {
     await ClientMonitor.visit({ id: node.id });
-    await a11yAudit();
-    assert.ok(true, 'a11y audit passes');
+    await a11yAudit(assert);
   });
 
   test('/clients/:id/monitor should have a breadcrumb trail linking back to clients', async function(assert) {

diff --git a/ui/tests/acceptance/clients-list-test.js b/ui/tests/acceptance/clients-list-test.js
@@ -21,8 +21,7 @@ module('Acceptance | clients list', function(hooks) {
     server.createList('agent', 1);
 
     await ClientsList.visit();
-    await a11yAudit();
-    assert.ok(true, 'a11y audit passes');
+    await a11yAudit(assert);
   });
 
   test('/clients should list one page of clients', async function(assert) {

diff --git a/ui/tests/acceptance/exec-test.js b/ui/tests/acceptance/exec-test.js
@@ -36,8 +36,7 @@ module('Acceptance | exec', function(hooks) {
 
   test('it passes an accessibility audit', async function(assert) {
     await Exec.visitJob({ job: this.job.id });
-    await a11yAudit();
-    assert.ok(true, 'a11y audit passes');
+    await a11yAudit(assert);
   });
 
   test('/exec/:job should show the region, namespace, and job name', async function(assert) {

diff --git a/ui/tests/acceptance/job-allocations-test.js b/ui/tests/acceptance/job-allocations-test.js
@@ -34,8 +34,7 @@ module('Acceptance | job allocations', function(hooks) {
     allocations = server.schema.allocations.where({ jobId: job.id }).models;
 
     await Allocations.visit({ id: job.id });
-    await a11yAudit();
-    assert.ok(true, 'a11y audit passes');
+    await a11yAudit(assert);
   });
 
   test('lists all allocations for the job', async function(assert) {

diff --git a/ui/tests/acceptance/job-definition-test.js b/ui/tests/acceptance/job-definition-test.js
@@ -21,8 +21,7 @@ module('Acceptance | job definition', function(hooks) {
   });
 
   test('it passes an accessibility audit', async function(assert) {
-    await a11yAudit('scrollable-region-focusable');
-    assert.ok(true, 'a11y audit passes');
+    await a11yAudit(assert, 'scrollable-region-focusable');
   });
 
   test('visiting /jobs/:job_id/definition', async function(assert) {

diff --git a/ui/tests/acceptance/job-deployments-test.js b/ui/tests/acceptance/job-deployments-test.js
@@ -36,8 +36,7 @@ module('Acceptance | job deployments', function(hooks) {
 
   test('it passes an accessibility audit', async function(assert) {
     await Deployments.visit({ id: job.id });
-    await a11yAudit();
-    assert.ok(true, 'a11y audit passes');
+    await a11yAudit(assert);
   });
 
   test('/jobs/:id/deployments should list all job deployments', async function(assert) {