Require icon accessibility attributes (#679)

* feat(icon): throw error for inaccessible icons Previously, the a11y-title and a11y-hiddden attributes were added and the description attribute was deprecated. This change removes the description attribute and throws an error if an icon is used without proper accessibility attributes. * fix: remove deprecated attribute from tests There was a default description of "" for icons but now that has been removed. * fix(dropdown-menu-nav): add a11y-hidden to icon Because there will always be a category in the button text and the icon is purely decorative, it should be hidden from screen readers. * hotfix(icon): fix typo in test description Co-authored-by: Mat Harris <mat.harris@ithaka.org> * chore: add changeset --------- Co-authored-by: Mat Harris <mat.harris@ithaka.org>
ithaka · Feb 8, 2024 · a1ade7a · a1ade7a
1 parent 60f2a09
commit a1ade7a
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 53 deletions.
diff --git a/.changeset/thirty-carrots-suffer.md b/.changeset/thirty-carrots-suffer.md
@@ -0,0 +1,5 @@
+---
+'@ithaka/pharos': patch
+---
+
+Require all icons to have a label or be explicitly hidden
diff --git a/packages/pharos/src/components/alert/pharos-alert.test.ts b/packages/pharos/src/components/alert/pharos-alert.test.ts
@@ -39,7 +39,6 @@ describe('pharos-alert', () => {
           class="alert__icon"
           data-pharos-component="PharosIcon"
           a11y-hidden="true"
-          description=""
           name="info-inverse"
         >
         </pharos-icon>

diff --git a/packages/pharos/src/components/dropdown-menu-nav/pharos-dropdown-menu-nav-category.ts b/packages/pharos/src/components/dropdown-menu-nav/pharos-dropdown-menu-nav-category.ts
@@ -52,7 +52,7 @@ export class PharosDropdownMenuNavCategory extends ScopedRegistryMixin(FocusMixi
       })}"
     >
       <slot name="category"></slot>
-      <pharos-icon name="chevron-down"></pharos-icon>
+      <pharos-icon name="chevron-down" a11y-hidden="true"></pharos-icon>
     </button>`;
   }
 }
diff --git a/packages/pharos/src/components/icon/pharos-icon.test.ts b/packages/pharos/src/components/icon/pharos-icon.test.ts
@@ -7,17 +7,19 @@ describe('pharos-icon', () => {
   let component: PharosIcon;
 
   beforeEach(async () => {
-    component = await fixture(html`<test-pharos-icon name="base"></test-pharos-icon>`);
+    component = await fixture(
+      html`<test-pharos-icon name="base" a11y-title="base-icon"></test-pharos-icon>`
+    );
   });
 
   it('is accessible', async () => {
     await expect(component).to.be.accessible();
   });
 
   it('throws an error for an invalid icon name', async () => {
-    component = await fixture(html` <test-pharos-icon name="fake"></test-pharos-icon> `).catch(
-      (e) => e
-    );
+    component = await fixture(
+      html` <test-pharos-icon name="fake" a11y-title="fake-icon"></test-pharos-icon> `
+    ).catch((e) => e);
     expect('Could not get icon named "fake"').to.be.thrown;
   });
 
@@ -46,26 +48,6 @@ describe('pharos-icon', () => {
     expect(svg?.getAttribute('aria-hidden')).to.equal('true');
   });
 
-  it('adds aria-hidden when a11y-hidden is not provided and there is no title or description', async () => {
-    await component.updateComplete;
-    const svg = component.renderRoot.querySelector('svg');
-    expect(svg?.getAttribute('aria-hidden')).to.equal('true');
-  });
-
-  it('does not add aria-hidden if there is a title', async () => {
-    component.a11yTitle = 'some-label';
-    await component.updateComplete;
-    const svg = component.renderRoot.querySelector('svg');
-    expect(svg?.getAttribute('aria-hidden')).not.to.exist;
-  });
-
-  it('does not add aria-hidden if there is a description', async () => {
-    component.description = 'some-label';
-    await component.updateComplete;
-    const svg = component.renderRoot.querySelector('svg');
-    expect(svg?.getAttribute('aria-hidden')).not.to.exist;
-  });
-
   it('sets the svg title properly when a11y-title is set', async () => {
     const labelText = 'This is a test title';
     component.a11yTitle = labelText;
@@ -74,11 +56,12 @@ describe('pharos-icon', () => {
     expect(title).to.contain.text(labelText);
   });
 
-  it('sets the svg title properly when description is set', async () => {
-    const labelText = 'This is a test title';
-    component.description = labelText;
-    await component.updateComplete;
-    const title = component.renderRoot.querySelector('svg>title');
-    expect(title).to.contain.text(labelText);
+  it('throws an error when neither a11y-title or a11y-hidden are set', async () => {
+    component = await fixture(html` <test-pharos-icon name="base"></test-pharos-icon> `).catch(
+      (e) => e
+    );
+    expect(
+      'All icons must have an accessible title (a11y-title) or be marked as hidden to assistive technology (a11y-hidden).'
+    ).to.be.thrown;
   });
 });
diff --git a/packages/pharos/src/components/icon/pharos-icon.ts b/packages/pharos/src/components/icon/pharos-icon.ts
@@ -27,14 +27,6 @@ export class PharosIcon extends PharosElement {
   @property({ type: String, reflect: true })
   public name?: IconName;
 
-  /**
-   * A description of what the icon represents
-   * @attr description
-   * @deprecated Please use a11yTitle instead.
-   */
-  @property({ type: String, reflect: true })
-  public description = '';
-
   /**
    * Indicates the title to apply as the accessible name of the icon.
    * @attr a11y-title
@@ -55,9 +47,10 @@ export class PharosIcon extends PharosElement {
 
   protected override update(changedProperties: PropertyValues): void {
     super.update && super.update(changedProperties);
-    if (this.description.length) {
-      console.warn(
-        "The 'description' attribute of pharos-icon is deprecated and will be removed in the next major release. Please use a11y-title or mark the icon as decorative by using a11y-hidden instead."
+
+    if (!this.a11yHidden && !this.a11yTitle) {
+      throw new Error(
+        `All icons must have an accessible title (a11y-title) or be marked as hidden to assistive technology (a11y-hidden).`
       );
     }
   }
@@ -84,25 +77,19 @@ export class PharosIcon extends PharosElement {
 
   protected override render(): TemplateResult {
     const size = this._getIconSize();
-    const accessibilityLabel = this.a11yTitle || this.description;
-    let hideIcon = this.a11yHidden;
-    // Check accessibilityLabel length for backwards compatibility until description is removed
-    if (hideIcon !== 'true' && accessibilityLabel.length === 0) {
-      hideIcon = 'true';
-    }
     return html`
       <svg
         xmlns="http://www.w3.org/2000/svg"
         version="1.1"
         viewBox="0 0 ${size} ${size}"
         class="icon"
         role="img"
-        aria-hidden=${hideIcon || nothing}
+        aria-hidden=${this.a11yHidden || nothing}
         height="${size}"
         width="${size}"
         focusable="false"
       >
-        <title>${accessibilityLabel}</title>
+        <title>${this.a11yTitle}</title>
         ${unsafeSVG(this._svg)}
       </svg>
     `;

diff --git a/packages/pharos/src/components/text-input/pharos-text-input.test.ts b/packages/pharos/src/components/text-input/pharos-text-input.test.ts
@@ -199,7 +199,6 @@ describe('pharos-text-input', () => {
           class="input__icon"
           data-pharos-component="PharosIcon"
           a11y-hidden="true"
-          description=""
           name="exclamation"
         >
         </pharos-icon>
@@ -234,7 +233,6 @@ describe('pharos-text-input', () => {
           class="input__icon"
           data-pharos-component="PharosIcon"
           a11y-hidden="true"
-          description=""
           name="checkmark"
         >
         </pharos-icon>