Merge pull request #1740 from hypothesis/tag-editor-a11y-tests

Improve a11y testing for AutocompleteList and TagEditor

src/sidebar/components/autocomplete-list.js

@@ -53,19 +53,25 @@ export default function AutocompleteList({
   }, [activeItem, itemPrefixId, list, listFormatter, onSelectItem]);
 
   const props = id ? { id } : {}; // only add the id if its passed
+  const isHidden = list.length === 0 || !open;
   return (
-    <div className="autocomplete-list">
-      {list.length > 0 && open && (
-        <ul
-          className="autocomplete-list__items"
-          tabIndex="-1"
-          aria-label="Suggestions"
-          role="listbox"
-          {...props}
-        >
-          {items}
-        </ul>
+    <div
+      className={classnames(
+        {
+          'is-hidden': isHidden,
+        },
+        'autocomplete-list'
       )}
+    >
+      <ul
+        className="autocomplete-list__items"
+        tabIndex="-1"
+        aria-label="Suggestions"
+        role="listbox"
+        {...props}
+      >
+        {items}
+      </ul>
     </div>
   );
 }

src/sidebar/components/tag-editor.js

@@ -109,6 +109,7 @@ function TagEditor({ onEditTags, tags: tagsService, tagList }) {
     }
     updateTags([...tagList, value]);
     setSuggestionsListOpen(false);
+    setActiveItem(-1);
 
     inputEl.current.value = '';
     inputEl.current.focus();
@@ -248,6 +249,7 @@ function TagEditor({ onEditTags, tags: tagsService, tagList }) {
           className="tag-editor__input"
           type="text"
           autoComplete="off"
+          aria-label="Add tag field"
           aria-autocomplete="list"
           aria-activedescendant={activeDescendant}
           aria-controls={`${tagEditorId}-autocomplete-list`}

src/sidebar/components/test/autocomplete-list-test.js

@@ -4,6 +4,7 @@ import { createElement } from 'preact';
 import AutocompleteList from '../autocomplete-list';
 import { $imports } from '../autocomplete-list';
 
+import { checkAccessibility } from '../../../test-util/accessibility';
 import mockImportedComponents from '../../../test-util/mock-imported-components';
 
 describe('AutocompleteList', function() {
@@ -33,12 +34,12 @@ describe('AutocompleteList', function() {
 
   it('does not render the list when `open` is false', () => {
     const wrapper = createComponent();
-    assert.isFalse(wrapper.find('.autocomplete-list__items').exists());
+    assert.isTrue(wrapper.find('.autocomplete-list').hasClass('is-hidden'));
   });
 
   it('does not render the list when `list` is empty', () => {
     const wrapper = createComponent({ open: true, list: [] });
-    assert.isFalse(wrapper.find('.autocomplete-list__items').exists());
+    assert.isTrue(wrapper.find('.autocomplete-list').hasClass('is-hidden'));
   });
 
   it('sets unique keys to the <li> items', () => {
@@ -160,4 +161,22 @@ describe('AutocompleteList', function() {
         .prop('aria-selected')
     );
   });
+
+  it(
+    'should pass a11y checks',
+    checkAccessibility([
+      {
+        name: 'list open',
+        content: () => {
+          return createComponent({ open: true });
+        },
+      },
+      {
+        name: 'list open, first item selected',
+        content: () => {
+          return createComponent({ open: true, activeItem: 1 });
+        },
+      },
+    ])
+  );
 });

src/sidebar/components/test/tag-editor-test.js

@@ -2,6 +2,7 @@ import { mount } from 'enzyme';
 import { createElement } from 'preact';
 import { act } from 'preact/test-utils';
 
+import AutocompleteList from '../autocomplete-list';
 import TagEditor from '../tag-editor';
 import { $imports } from '../tag-editor';
 
@@ -112,70 +113,6 @@ describe('TagEditor', function() {
     assert.isTrue(fakeTagsService.filter.calledWith('tag3'));
   });
 
-  describe('accessibility attributes and ids', () => {
-    it('creates multiple <TagEditor> components with unique autocomplete-list `id` props', () => {
-      const wrapper1 = createComponent();
-      const wrapper2 = createComponent();
-      assert.notEqual(
-        wrapper1.find('AutocompleteList').prop('id'),
-        wrapper2.find('AutocompleteList').prop('id')
-      );
-    });
-
-    it('sets the <AutocompleteList> `id` prop to the same value as the `aria-owns` attribute', () => {
-      const wrapper = createComponent();
-      wrapper.find('AutocompleteList');
-
-      assert.equal(
-        wrapper.find('.tag-editor__combobox-wrapper').prop('aria-owns'),
-        wrapper.find('AutocompleteList').prop('id')
-      );
-    });
-
-    it('sets `aria-expanded` value to match open state', () => {
-      const wrapper = createComponent();
-      wrapper.find('input').instance().value = 'non-empty'; // to open list
-      typeInput(wrapper);
-      assert.equal(
-        wrapper.find('.tag-editor__combobox-wrapper').prop('aria-expanded'),
-        'true'
-      );
-      selectOption(wrapper, 'tag4');
-      wrapper.update();
-      assert.equal(
-        wrapper.find('.tag-editor__combobox-wrapper').prop('aria-expanded'),
-        'false'
-      );
-    });
-
-    it('sets the <AutocompleteList> `activeItem` prop to match the selected item index', () => {
-      function checkAttributes(wrapper) {
-        const activeDescendant = wrapper
-          .find('input')
-          .prop('aria-activedescendant');
-        const itemPrefixId = wrapper
-          .find('AutocompleteList')
-          .prop('itemPrefixId');
-        const activeDescendantIndex = activeDescendant.split(itemPrefixId);
-        assert.equal(
-          activeDescendantIndex[1],
-          wrapper.find('AutocompleteList').prop('activeItem')
-        );
-      }
-
-      const wrapper = createComponent();
-      wrapper.find('input').instance().value = 'non-empty';
-      typeInput(wrapper);
-      // initial aria-activedescendant value is "" when index is -1
-      assert.equal(wrapper.find('input').prop('aria-activedescendant'), '');
-      // 2 suggestions: ['tag3', 'tag4'];
-      navigateDown(wrapper); // press down once
-      checkAttributes(wrapper);
-      navigateDown(wrapper); // press down again once
-      checkAttributes(wrapper);
-    });
-  });
-
   describe('suggestions open / close', () => {
     it('closes the suggestions when selecting a tag from autocomplete-list', () => {
       const wrapper = createComponent();
@@ -271,6 +208,8 @@ describe('TagEditor', function() {
       assert.isTrue(fakeOnEditTags.calledWith({ tags: tagList }));
       // hides the suggestions
       assert.equal(wrapper.find('AutocompleteList').prop('open'), false);
+      // removes the selected index
+      assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), -1);
       // assert the input value is cleared out
       assert.equal(wrapper.find('input').instance().value, '');
       // input element should have focus
@@ -408,11 +347,107 @@ describe('TagEditor', function() {
     });
   });
 
-  // FIXME-A11Y
-  it.skip(
-    'should pass a11y checks',
-    checkAccessibility({
-      content: () => createComponent(),
-    })
-  );
+  describe('accessibility attributes and ids', () => {
+    it('creates multiple <TagEditor> components with unique autocomplete-list `id` props', () => {
+      const wrapper1 = createComponent();
+      const wrapper2 = createComponent();
+      assert.notEqual(
+        wrapper1.find('AutocompleteList').prop('id'),
+        wrapper2.find('AutocompleteList').prop('id')
+      );
+    });
+
+    it('sets the <AutocompleteList> `id` prop to the same value as the `aria-owns` attribute', () => {
+      const wrapper = createComponent();
+      wrapper.find('AutocompleteList');
+
+      assert.equal(
+        wrapper.find('.tag-editor__combobox-wrapper').prop('aria-owns'),
+        wrapper.find('AutocompleteList').prop('id')
+      );
+    });
+
+    it('sets `aria-expanded` value to match open state', () => {
+      const wrapper = createComponent();
+      wrapper.find('input').instance().value = 'non-empty'; // to open list
+      typeInput(wrapper);
+      assert.equal(
+        wrapper.find('.tag-editor__combobox-wrapper').prop('aria-expanded'),
+        'true'
+      );
+      selectOption(wrapper, 'tag4');
+      wrapper.update();
+      assert.equal(
+        wrapper.find('.tag-editor__combobox-wrapper').prop('aria-expanded'),
+        'false'
+      );
+    });
+
+    it('sets the <AutocompleteList> `activeItem` prop to match the selected item index', () => {
+      function checkAttributes(wrapper) {
+        const activeDescendant = wrapper
+          .find('input')
+          .prop('aria-activedescendant');
+        const itemPrefixId = wrapper
+          .find('AutocompleteList')
+          .prop('itemPrefixId');
+        const activeDescendantIndex = activeDescendant.split(itemPrefixId);
+        assert.equal(
+          activeDescendantIndex[1],
+          wrapper.find('AutocompleteList').prop('activeItem')
+        );
+      }
+
+      const wrapper = createComponent();
+      wrapper.find('input').instance().value = 'non-empty';
+      typeInput(wrapper);
+      // initial aria-activedescendant value is "" when index is -1
+      assert.equal(wrapper.find('input').prop('aria-activedescendant'), '');
+      // 2 suggestions: ['tag3', 'tag4'];
+      navigateDown(wrapper); // press down once
+      checkAttributes(wrapper);
+      navigateDown(wrapper); // press down again once
+      checkAttributes(wrapper);
+    });
+  });
+
+  describe('accessibility validation', () => {
+    beforeEach(function() {
+      // create a full dom tree for a11y testing
+      $imports.$mock({
+        './autocomplete-list': AutocompleteList,
+      });
+    });
+
+    it(
+      'should pass a11y checks',
+      checkAccessibility([
+        {
+          name: 'suggestions open',
+          content: () => {
+            const wrapper = createComponent();
+            wrapper.find('input').instance().value = 'non-empty';
+            typeInput(wrapper);
+            return wrapper;
+          },
+        },
+        {
+          name: 'suggestions open, first item selected',
+          content: () => {
+            const wrapper = createComponent();
+            wrapper.find('input').instance().value = 'non-empty';
+            typeInput(wrapper);
+            navigateDown(wrapper);
+            return wrapper;
+          },
+        },
+        {
+          name: 'suggestions closed',
+          content: () => {
+            return createComponent();
+          },
+        },
+      ])
+    );
+  });
 });

src/styles/sidebar/components/autocomplete-list.scss

@@ -2,6 +2,9 @@
 
 .autocomplete-list {
   position: relative;
+  &.is-hidden {
+    display: none;
+  }
 }
 
 .autocomplete-list__items {

src/styles/variables.scss

@@ -20,7 +20,7 @@ $grey-4: #a6a6a6;
 // minus blue tint.
 $grey-semi: #9c9c9c;
 
-$grey-5: #767676;
+$grey-5: #737373;
 
 // Interim color variable for migration purposes, as the step between `$grey-5`
 // and `$grey-6` is large. Represents `base-mid` in proposed future palette,