fix(icon): remove automatic aria labelling and add a11y guidance

jelbourn · jelbourn · commit 38b5e5d10a41 · 2017-05-19T10:11:46.000-07:00
The automatic application of aria-label for md-icon, in hindsight, was
not a great idea. Neither the ligature string nor the SVG filename are
likely to be meaningful descriptions. Even in cases where they *are*,
the icon may be used in such a way that the application of a label is
still the wrong thing. On top of that, adding `role="img"` is usually
not the right approach for an icon, as that role typically refers to
*content* images, rather than icons that are part of the UI.

Ultimately, it is up to the application developer to add the appropriate
meaning for icons based on how they're used. To this end, we add
guidance to the documentation for what to do in different situations.
diff --git a/src/lib/icon/icon.md b/src/lib/icon/icon.md
@@ -15,8 +15,8 @@ Some fonts are designed to show icons by using
 "home" as a home image. To use a ligature icon, put its text in the content of the `md-icon`
 component.
 
-By default the
-[Material icons font](http://google.github.io/material-design-icons/#icon-font-for-the-web) is used.
+By default, `<md-icon>` expects the
+[Material icons font](http://google.github.io/material-design-icons/#icon-font-for-the-web).
 (You will still need to include the HTML to load the font and its CSS, as described in the link).
 You can specify a different font by setting the `fontSet` input to either the CSS class to apply to
 use the desired font, or to an alias previously registered with
@@ -80,9 +80,32 @@ match the current theme's colors using the `color` attribute. This can be change
 
 ### Accessibility
 
-If an `aria-label` attribute is set on the `md-icon` element, its value will be used as-is. If not,
-the md-icon component will attempt to set the aria-label value from one of these sources:
-* The `alt` attribute
-* The `fontIcon` input
-* The name of the icon from the `svgIcon` input (not including any namespace)
-* The text content of the component (for ligature icons)
+Similar to an `<img>` element, an icon alone does not convey any useful information for a
+screen-reader user. The user of `<md-icon>` must provide additional information pertaining to how
+the icon is used.
+
+In thinking about accessibility, it is useful to place icon use into one of three categories:
+1. **Decorative**: the icon conveys no real semantic meaning and is purely cosmetic.
+2. **Interactive**: a user will click or otherwise interact with the icon to perform some action.
+3. **Indicator**: the icon is not interactive, but it conveys some information, such as a status.
+
+#### Decorative icons
+When the icon is puely cosmetic and conveys no real semantic meaning, the `<md-icon>` element
+should be marked with `aria-hidden="true"`.
+
+#### Interactive icons
+Icons alone are not interactive elements for screen-reader users; when the user would interact with
+some icon on the page, a more appropriate  element should "own" the interaction:
+* The `<md-icon>` element should be a child of a `<button>` or `<a>` element.
+* The `<md-icon>` element should be marked with `aria-hidden="true"`.
+* The parent `<button>` or `<a>` should either have a meaningful label provided either through
+direct text content, `aria-label`, or `aria-labelledby`.
+
+#### Indicator icons
+When the presence of an icon communicates some information to the user, that information must also
+be made available to screen-readers. The most straightforward way to do this is to
+1. Mark the `<md-icon>` as `aria-hidden="true"`
+2. Add a `<span>` as an adjacent sibling to the `<md-icon>` element with text that conveys the same
+information as the icon.
+3. Add the `cdk-visually-hidden` class to the `<span>`. This will make the message invisible
+on-screen but still available to screen-reader users.
diff --git a/src/lib/icon/icon.spec.ts b/src/lib/icon/icon.spec.ts
@@ -41,7 +41,6 @@ describe('MdIcon', () => {
       declarations: [
         MdIconColorTestApp,
         MdIconLigatureTestApp,
-        MdIconLigatureWithAriaBindingTestApp,
         MdIconCustomFontCssTestApp,
         MdIconFromSvgNameTestApp,
       ],
@@ -121,15 +120,12 @@ describe('MdIcon', () => {
       fixture.detectChanges();
       svgElement = verifyAndGetSingleSvgChild(mdIconElement);
       verifyPathChildElement(svgElement, 'woof');
-      // The aria label should be taken from the icon name.
-      expect(mdIconElement.getAttribute('aria-label')).toBe('fido');
 
       // Change the icon, and the SVG element should be replaced.
       testComponent.iconName = 'fluffy';
       fixture.detectChanges();
       svgElement = verifyAndGetSingleSvgChild(mdIconElement);
       verifyPathChildElement(svgElement, 'meow');
-      expect(mdIconElement.getAttribute('aria-label')).toBe('fluffy');
 
       expect(httpRequestUrls).toEqual(['dog.svg', 'cat.svg']);
       // Using an icon from a previously loaded URL should not cause another HTTP request.
@@ -181,8 +177,6 @@ describe('MdIcon', () => {
       expect(svgChild.tagName.toLowerCase()).toBe('g');
       expect(svgChild.getAttribute('id')).toBe('pig');
       verifyPathChildElement(svgChild, 'oink');
-      // The aria label should be taken from the icon name (without the icon set portion).
-      expect(mdIconElement.getAttribute('aria-label')).toBe('pig');
 
       // Change the icon, and the SVG element should be replaced.
       testComponent.iconName = 'farm:cow';
@@ -193,7 +187,6 @@ describe('MdIcon', () => {
       expect(svgChild.tagName.toLowerCase()).toBe('g');
       expect(svgChild.getAttribute('id')).toBe('cow');
       verifyPathChildElement(svgChild, 'moo');
-      expect(mdIconElement.getAttribute('aria-label')).toBe('cow');
     });
 
     it('should allow multiple icon sets in a namespace', () => {
@@ -218,8 +211,6 @@ describe('MdIcon', () => {
       expect(svgChild.getAttribute('id')).toBe('pig');
       expect(svgChild.childNodes.length).toBe(1);
       verifyPathChildElement(svgChild, 'oink');
-      // The aria label should be taken from the icon name (without the namespace).
-      expect(mdIconElement.getAttribute('aria-label')).toBe('pig');
 
       // Both icon sets registered in the 'farm' namespace should have been fetched.
       expect(httpRequestUrls.sort()).toEqual(['farm-set-1.svg', 'farm-set-2.svg']);
@@ -236,7 +227,6 @@ describe('MdIcon', () => {
       expect(svgChild.getAttribute('id')).toBe('cow');
       expect(svgChild.childNodes.length).toBe(1);
       verifyPathChildElement(svgChild, 'moo moo');
-      expect(mdIconElement.getAttribute('aria-label')).toBe('cow');
       expect(httpRequestUrls.sort()).toEqual(['farm-set-1.svg', 'farm-set-2.svg']);
     });
 
@@ -255,7 +245,6 @@ describe('MdIcon', () => {
       // directly and not wrapped in an outer <svg> tag like the <g> elements in other sets.
       svgElement = verifyAndGetSingleSvgChild(mdIconElement);
       verifyPathChildElement(svgElement, 'left');
-      expect(mdIconElement.getAttribute('aria-label')).toBe('left-arrow');
     });
 
     it('should return unmodified copies of icons from icon sets', () => {
@@ -302,89 +291,16 @@ describe('MdIcon', () => {
       testComponent.fontIcon = 'house';
       fixture.detectChanges();
       expect(sortedClassNames(mdIconElement)).toEqual(['font1', 'house', 'mat-icon']);
-      expect(mdIconElement.getAttribute('aria-label')).toBe('house');
 
       testComponent.fontSet = 'f2';
       testComponent.fontIcon = 'igloo';
       fixture.detectChanges();
       expect(sortedClassNames(mdIconElement)).toEqual(['f2', 'igloo', 'mat-icon']);
-      expect(mdIconElement.getAttribute('aria-label')).toBe('igloo');
 
       testComponent.fontSet = 'f3';
       testComponent.fontIcon = 'tent';
       fixture.detectChanges();
       expect(sortedClassNames(mdIconElement)).toEqual(['f3', 'mat-icon', 'tent']);
-      expect(mdIconElement.getAttribute('aria-label')).toBe('tent');
-    });
-  });
-
-  describe('aria label', () => {
-    it('should set aria label from text content if not specified', () => {
-      let fixture = TestBed.createComponent(MdIconLigatureTestApp);
-
-      const testComponent = fixture.componentInstance;
-      const mdIconElement = fixture.debugElement.nativeElement.querySelector('md-icon');
-      testComponent.iconName = 'home';
-
-      fixture.detectChanges();
-      expect(mdIconElement.getAttribute('aria-label')).toBe('home');
-
-      testComponent.iconName = 'hand';
-      fixture.detectChanges();
-      expect(mdIconElement.getAttribute('aria-label')).toBe('hand');
-    });
-
-    it('should not set aria label unless it actually changed', () => {
-      let fixture = TestBed.createComponent(MdIconLigatureTestApp);
-
-      const testComponent = fixture.componentInstance;
-      const mdIconElement = fixture.debugElement.nativeElement.querySelector('md-icon');
-      testComponent.iconName = 'home';
-
-      fixture.detectChanges();
-      expect(mdIconElement.getAttribute('aria-label')).toBe('home');
-
-      mdIconElement.removeAttribute('aria-label');
-      fixture.detectChanges();
-      expect(mdIconElement.getAttribute('aria-label')).toBeFalsy();
-    });
-
-    it('should use alt tag if aria label is not specified', () => {
-      let fixture = TestBed.createComponent(MdIconLigatureWithAriaBindingTestApp);
-
-      const testComponent = fixture.componentInstance;
-      const mdIconElement = fixture.debugElement.nativeElement.querySelector('md-icon');
-      testComponent.iconName = 'home';
-      testComponent.altText = 'castle';
-      fixture.detectChanges();
-      expect(mdIconElement.getAttribute('aria-label')).toBe('castle');
-
-      testComponent.ariaLabel = 'house';
-      fixture.detectChanges();
-      expect(mdIconElement.getAttribute('aria-label')).toBe('house');
-    });
-
-    it('should use provided aria label rather than icon name', () => {
-      let fixture = TestBed.createComponent(MdIconLigatureWithAriaBindingTestApp);
-
-      const testComponent = fixture.componentInstance;
-      const mdIconElement = fixture.debugElement.nativeElement.querySelector('md-icon');
-      testComponent.iconName = 'home';
-      testComponent.ariaLabel = 'house';
-      fixture.detectChanges();
-      expect(mdIconElement.getAttribute('aria-label')).toBe('house');
-    });
-
-    it('should use provided aria label rather than font icon', () => {
-      let fixture = TestBed.createComponent(MdIconCustomFontCssTestApp);
-
-      const testComponent = fixture.componentInstance;
-      const mdIconElement = fixture.debugElement.nativeElement.querySelector('md-icon');
-      testComponent.fontSet = 'f1';
-      testComponent.fontIcon = 'house';
-      testComponent.ariaLabel = 'home';
-      fixture.detectChanges();
-      expect(mdIconElement.getAttribute('aria-label')).toBe('home');
     });
   });
 
@@ -431,35 +347,24 @@ describe('MdIcon without HttpModule', () => {
 /** Test components that contain an MdIcon. */
 @Component({template: `<md-icon>{{iconName}}</md-icon>`})
 class MdIconLigatureTestApp {
-  ariaLabel: string = null;
   iconName = '';
 }
 
 @Component({template: `<md-icon [color]="iconColor">{{iconName}}</md-icon>`})
 class MdIconColorTestApp {
-  ariaLabel: string = null;
   iconName = '';
   iconColor = 'primary';
 }
 
-@Component({template: `<md-icon [aria-label]="ariaLabel" [alt]="altText">{{iconName}}</md-icon>`})
-class MdIconLigatureWithAriaBindingTestApp {
-  altText: string = '';
-  ariaLabel: string = null;
-  iconName = '';
-}
-
 @Component({
-  template: `<md-icon [fontSet]="fontSet" [fontIcon]="fontIcon" [aria-label]="ariaLabel"></md-icon>`
+  template: `<md-icon [fontSet]="fontSet" [fontIcon]="fontIcon"></md-icon>`
 })
 class MdIconCustomFontCssTestApp {
-  ariaLabel: string = null;
   fontSet = '';
   fontIcon = '';
 }
 
-@Component({template: `<md-icon [svgIcon]="iconName" [aria-label]="ariaLabel"></md-icon>`})
+@Component({template: `<md-icon [svgIcon]="iconName"></md-icon>`})
 class MdIconFromSvgNameTestApp {
-  ariaLabel: string = null;
   iconName = '';
 }
diff --git a/src/lib/icon/icon.ts b/src/lib/icon/icon.ts
@@ -53,12 +53,12 @@ import {MdIconRegistry} from './icon-registry';
   styleUrls: ['icon.css'],
   host: {
     'role': 'img',
-    '[class.mat-icon]': 'true',
+    'class': 'mat-icon'
   },
   encapsulation: ViewEncapsulation.None,
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class MdIcon implements OnChanges, OnInit, AfterViewChecked {
+export class MdIcon implements OnChanges, OnInit {
   private _color: string;
 
   /** Name of the icon in the SVG icon set. */
@@ -70,20 +70,13 @@ export class MdIcon implements OnChanges, OnInit, AfterViewChecked {
   /** Name of an icon within a font set. */
   @Input() fontIcon: string;
 
-  /** Alt label to be used for accessibility. */
-  @Input() alt: string;
-
-  /** Screenreader label for the icon. */
-  @Input('aria-label') hostAriaLabel: string = '';
-
   /** Color of the icon. */
   @Input()
   get color(): string { return this._color; }
   set color(value: string) { this._updateColor(value); }
 
   private _previousFontSetClass: string;
   private _previousFontIconClass: string;
-  private _previousAriaLabel: string;
 
   constructor(
       private _elementRef: ElementRef,
@@ -135,7 +128,7 @@ export class MdIcon implements OnChanges, OnInit, AfterViewChecked {
     }
   }
 
-  ngOnChanges(changes: { [propertyName: string]: SimpleChange }) {
+  ngOnChanges(changes: {[propertyName: string]: SimpleChange}) {
     const changedInputs = Object.keys(changes);
     // Only update the inline SVG icon if the inputs changed, to avoid unnecessary DOM operations.
     if (changedInputs.indexOf('svgIcon') != -1 || changedInputs.indexOf('svgSrc') != -1) {
@@ -149,7 +142,6 @@ export class MdIcon implements OnChanges, OnInit, AfterViewChecked {
     if (this._usingFontIcon()) {
       this._updateFontIconClasses();
     }
-    this._updateAriaLabel();
   }
 
   ngOnInit() {
@@ -160,43 +152,6 @@ export class MdIcon implements OnChanges, OnInit, AfterViewChecked {
     }
   }
 
-  ngAfterViewChecked() {
-    // Update aria label here because it may depend on the projected text content.
-    // (e.g. <md-icon>home</md-icon> should use 'home').
-    this._updateAriaLabel();
-  }
-
-  private _updateAriaLabel() {
-      const ariaLabel = this._getAriaLabel();
-      if (ariaLabel && ariaLabel !== this._previousAriaLabel) {
-        this._previousAriaLabel = ariaLabel;
-        this._renderer.setAttribute(this._elementRef.nativeElement, 'aria-label', ariaLabel);
-      }
-  }
-
-  private _getAriaLabel() {
-    // If the parent provided an aria-label attribute value, use it as-is. Otherwise look for a
-    // reasonable value from the alt attribute, font icon name, SVG icon name, or (for ligatures)
-    // the text content of the directive.
-    const label =
-        this.hostAriaLabel ||
-        this.alt ||
-        this.fontIcon ||
-        this._splitIconName(this.svgIcon)[1];
-    if (label) {
-      return label;
-    }
-    // The "content" of an SVG icon is not a useful label.
-    if (this._usingFontIcon()) {
-      const text = this._elementRef.nativeElement.textContent;
-      if (text) {
-        return text;
-      }
-    }
-    // TODO: Warn here in dev mode.
-    return null;
-  }
-
   private _usingFontIcon(): boolean {
     return !this.svgIcon;
   }
