Skip to content

Commit

Permalink
fix(material/select): trim aria-labelledby
Browse files Browse the repository at this point in the history
If the form field doesn't have a label, we can end up with an `aria-labelledby` which has a leading
space. This appears to be flagged as invalid by some a11y tools.

Fixes angular#22192.
  • Loading branch information
crisbeto committed Mar 16, 2021
1 parent 662b751 commit fc88abc
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 9 deletions.
26 changes: 25 additions & 1 deletion src/material-experimental/mdc-select/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,17 @@ describe('MDC-based MatSelect', () => {
expect(select.getAttribute('aria-labelledby')).toBe(`${labelId} ${valueId}`);
}));

it('should trim the trigger aria-labelledby when there is no label', fakeAsync(() => {
fixture.componentInstance.hasLabel = false;
fixture.detectChanges();
flush();
fixture.detectChanges();

// Note that we assert that there are no spaces around the value.
const valueId = fixture.nativeElement.querySelector('.mat-mdc-select-value').id;
expect(select.getAttribute('aria-labelledby')).toBe(`${valueId}`);
}));

it('should set the tabindex of the select to 0 by default', fakeAsync(() => {
expect(select.getAttribute('tabindex')).toEqual('0');
}));
Expand Down Expand Up @@ -976,6 +987,18 @@ describe('MDC-based MatSelect', () => {
expect(panel.getAttribute('aria-labelledby')).toBe(`${labelId} myLabelId`);
}));

it('should trim the custom panel aria-labelledby when there is no label', fakeAsync(() => {
fixture.componentInstance.hasLabel = false;
fixture.componentInstance.ariaLabelledby = 'myLabelId';
fixture.componentInstance.select.open();
fixture.detectChanges();
flush();

// Note that we assert that there are no spaces around the value.
const panel = document.querySelector('.mat-mdc-select-panel')!;
expect(panel.getAttribute('aria-labelledby')).toBe(`myLabelId`);
}));

it('should clear aria-labelledby from the panel if an aria-label is set', fakeAsync(() => {
fixture.componentInstance.ariaLabel = 'My label';
fixture.componentInstance.select.open();
Expand Down Expand Up @@ -3835,7 +3858,7 @@ describe('MDC-based MatSelect', () => {
template: `
<div [style.height.px]="heightAbove"></div>
<mat-form-field>
<mat-label>Select a food</mat-label>
<mat-label *ngIf="hasLabel">Select a food</mat-label>
<mat-select placeholder="Food" [formControl]="control" [required]="isRequired"
[tabIndex]="tabIndexOverride" [aria-label]="ariaLabel" [aria-labelledby]="ariaLabelledby"
[panelClass]="panelClass" [disableRipple]="disableRipple"
Expand Down Expand Up @@ -3863,6 +3886,7 @@ class BasicSelect {
isRequired: boolean;
heightAbove = 0;
heightBelow = 0;
hasLabel = true;
tabIndexOverride: number;
ariaLabel: string;
ariaLabelledby: string;
Expand Down
23 changes: 23 additions & 0 deletions src/material/select/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,16 @@ describe('MatSelect', () => {
expect(select.getAttribute('aria-labelledby')).toBe(`${labelId} ${valueId}`);
}));

it('should trim the trigger aria-labelledby when there is no label', fakeAsync(() => {
// Reset the `placeholder` which also controls the label of the form field.
fixture.componentInstance.select.placeholder = '';
fixture.detectChanges();

// Note that we assert that there are no spaces around the value.
const valueId = fixture.nativeElement.querySelector('.mat-select-value').id;
expect(select.getAttribute('aria-labelledby')).toBe(`${valueId}`);
}));

it('should set the tabindex of the select to 0 by default', fakeAsync(() => {
expect(select.getAttribute('tabindex')).toEqual('0');
}));
Expand Down Expand Up @@ -979,6 +989,19 @@ describe('MatSelect', () => {
expect(panel.getAttribute('aria-labelledby')).toBe(`${labelId} myLabelId`);
}));

it('should trim the custom panel aria-labelledby when there is no label', fakeAsync(() => {
// Reset the `placeholder` which also controls the label of the form field.
fixture.componentInstance.select.placeholder = '';
fixture.componentInstance.ariaLabelledby = 'myLabelId';
fixture.componentInstance.select.open();
fixture.detectChanges();
flush();

// Note that we assert that there are no spaces around the value.
const panel = document.querySelector('.mat-select-panel')!;
expect(panel.getAttribute('aria-labelledby')).toBe(`myLabelId`);
}));

it('should clear aria-labelledby from the panel if an aria-label is set', fakeAsync(() => {
fixture.componentInstance.ariaLabel = 'My label';
fixture.componentInstance.select.open();
Expand Down
13 changes: 5 additions & 8 deletions src/material/select/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1004,8 +1004,9 @@ export abstract class _MatSelectBase<C> extends _MatSelectMixinBase implements A
return null;
}

const labelId = this._getLabelId();
return this.ariaLabelledby ? labelId + ' ' + this.ariaLabelledby : labelId;
const labelId = this._parentFormField?.getLabelId();
const labelExpression = (labelId ? labelId + ' ' : '');
return this.ariaLabelledby ? labelExpression + this.ariaLabelledby : labelId;
}

/** Determines the `aria-activedescendant` to be set on the host. */
Expand All @@ -1017,18 +1018,14 @@ export abstract class _MatSelectBase<C> extends _MatSelectMixinBase implements A
return null;
}

/** Gets the ID of the element that is labelling the select. */
private _getLabelId(): string {
return this._parentFormField?.getLabelId() || '';
}

/** Gets the aria-labelledby of the select component trigger. */
private _getTriggerAriaLabelledby(): string | null {
if (this.ariaLabel) {
return null;
}

let value = this._getLabelId() + ' ' + this._valueId;
const labelId = this._parentFormField?.getLabelId();
let value = (labelId ? labelId + ' ' : '') + this._valueId;

if (this.ariaLabelledby) {
value += ' ' + this.ariaLabelledby;
Expand Down

0 comments on commit fc88abc

Please sign in to comment.