Skip to content

Commit b959e18

Browse files
tinayuangaotinayuangao
committed
fix(button-toggle): use native button and aria-pressed for button-toggle (#10990)
1 parent 93b5e89 commit b959e18

File tree

4 files changed

+75
-61
lines changed

4 files changed

+75
-61
lines changed

‎src/lib/button-toggle/button-toggle.html‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,19 @@
1-
<label [attr.for]="inputId" class="mat-button-toggle-label" #label>
2-
<input #input class="mat-button-toggle-input cdk-visually-hidden"
3-
[type]="_type"
4-
[id]="inputId"
5-
[checked]="checked"
6-
[disabled]="disabled || null"
7-
[attr.name]="name"
8-
[attr.aria-label]="ariaLabel"
9-
[attr.aria-labelledby]="ariaLabelledby"
10-
(change)="_onInputChange($event)"
11-
(click)="_onInputClick($event)">
12-
1+
<button #button class="mat-button-toggle-button"
2+
type="button"
3+
[id]="buttonId"
4+
[attr.aria-pressed]="checked"
5+
[disabled]="disabled || null"
6+
[attr.name]="name || null"
7+
[attr.aria-label]="ariaLabel"
8+
[attr.aria-labelledby]="ariaLabelledby"
9+
(click)="_onButtonClick($event)">
1310
<div class="mat-button-toggle-label-content">
1411
<ng-content></ng-content>
1512
</div>
16-
</label>
17-
<div class="mat-button-toggle-focus-overlay"></div>
13+
</button>
1814

15+
<div class="mat-button-toggle-focus-overlay"></div>
1916
<div class="mat-button-toggle-ripple" matRipple
20-
[matRippleTrigger]="label"
17+
[matRippleTrigger]="button"
2118
[matRippleDisabled]="this.disableRipple || this.disabled">
2219
</div>

‎src/lib/button-toggle/button-toggle.scss‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,3 +100,13 @@ $mat-button-toggle-border-radius: 2px !default;
100100
// Pointer events can be safely disabled because the ripple trigger element is the label element.
101101
pointer-events: none;
102102
}
103+
104+
.mat-button-toggle-button {
105+
border: 0;
106+
background: none;
107+
color: inherit;
108+
padding: inherit;
109+
margin: inherit;
110+
font: inherit;
111+
outline: none;
112+
}

‎src/lib/button-toggle/button-toggle.spec.ts‎

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ describe('MatButtonToggle with forms', () => {
9393
buttonToggleDebugElements = fixture.debugElement.queryAll(By.directive(MatButtonToggle));
9494
buttonToggleInstances = buttonToggleDebugElements.map(debugEl => debugEl.componentInstance);
9595
buttonToggleLabels = buttonToggleDebugElements.map(
96-
debugEl => debugEl.query(By.css('label')).nativeElement);
96+
debugEl => debugEl.query(By.css('button')).nativeElement);
9797

9898
fixture.detectChanges();
9999
}));
@@ -244,7 +244,7 @@ describe('MatButtonToggle without forms', () => {
244244
buttonToggleNativeElements = buttonToggleDebugElements
245245
.map(debugEl => debugEl.nativeElement);
246246

247-
buttonToggleLabelElements = fixture.debugElement.queryAll(By.css('label'))
247+
buttonToggleLabelElements = fixture.debugElement.queryAll(By.css('button'))
248248
.map(debugEl => debugEl.nativeElement);
249249

250250
buttonToggleInstances = buttonToggleDebugElements.map(debugEl => debugEl.componentInstance);
@@ -339,7 +339,7 @@ describe('MatButtonToggle without forms', () => {
339339
buttonToggleLabelElements[0].click();
340340
fixture.detectChanges();
341341
tick();
342-
expect(changeSpy).toHaveBeenCalled();
342+
expect(changeSpy).toHaveBeenCalledTimes(1);
343343

344344
buttonToggleLabelElements[0].click();
345345
fixture.detectChanges();
@@ -447,7 +447,7 @@ describe('MatButtonToggle without forms', () => {
447447
buttonToggleDebugElements = fixture.debugElement.queryAll(By.directive(MatButtonToggle));
448448
buttonToggleNativeElements = buttonToggleDebugElements
449449
.map(debugEl => debugEl.nativeElement);
450-
buttonToggleLabelElements = fixture.debugElement.queryAll(By.css('label'))
450+
buttonToggleLabelElements = fixture.debugElement.queryAll(By.css('button'))
451451
.map(debugEl => debugEl.nativeElement);
452452
buttonToggleInstances = buttonToggleDebugElements.map(debugEl => debugEl.componentInstance);
453453
}));
@@ -463,7 +463,7 @@ describe('MatButtonToggle without forms', () => {
463463
it('should check a button toggle when clicked', () => {
464464
expect(buttonToggleInstances.every(buttonToggle => !buttonToggle.checked)).toBe(true);
465465

466-
let nativeCheckboxLabel = buttonToggleDebugElements[0].query(By.css('label')).nativeElement;
466+
let nativeCheckboxLabel = buttonToggleDebugElements[0].query(By.css('button')).nativeElement;
467467

468468
nativeCheckboxLabel.click();
469469

@@ -487,9 +487,9 @@ describe('MatButtonToggle without forms', () => {
487487
});
488488

489489
it('should check a button toggle upon interaction with underlying native checkbox', () => {
490-
let nativeCheckboxInput = buttonToggleDebugElements[0].query(By.css('input')).nativeElement;
490+
let nativeCheckboxButton = buttonToggleDebugElements[0].query(By.css('button')).nativeElement;
491491

492-
nativeCheckboxInput.click();
492+
nativeCheckboxButton.click();
493493
fixture.detectChanges();
494494

495495
expect(groupInstance.value).toEqual(['eggs']);
@@ -562,15 +562,19 @@ describe('MatButtonToggle without forms', () => {
562562
let buttonToggleNativeElement: HTMLElement;
563563
let buttonToggleLabelElement: HTMLLabelElement;
564564
let buttonToggleInstance: MatButtonToggle;
565+
let buttonToggleButtonElement: HTMLButtonElement;
565566

566567
beforeEach(fakeAsync(() => {
567568
fixture = TestBed.createComponent(StandaloneButtonToggle);
568569
fixture.detectChanges();
569570

570571
buttonToggleDebugElement = fixture.debugElement.query(By.directive(MatButtonToggle));
571572
buttonToggleNativeElement = buttonToggleDebugElement.nativeElement;
572-
buttonToggleLabelElement = fixture.debugElement.query(By.css('label')).nativeElement;
573+
buttonToggleLabelElement = fixture.debugElement
574+
.query(By.css('.mat-button-toggle-label-content')).nativeElement;
573575
buttonToggleInstance = buttonToggleDebugElement.componentInstance;
576+
buttonToggleButtonElement =
577+
buttonToggleNativeElement.querySelector('button')! as HTMLButtonElement;
574578
}));
575579

576580
it('should toggle when clicked', fakeAsync(() => {
@@ -609,66 +613,77 @@ describe('MatButtonToggle without forms', () => {
609613
}));
610614

611615
it('should focus on underlying input element when focus() is called', () => {
612-
let nativeRadioInput = buttonToggleDebugElement.query(By.css('input')).nativeElement;
613-
expect(document.activeElement).not.toBe(nativeRadioInput);
616+
let nativeButton = buttonToggleDebugElement.query(By.css('button')).nativeElement;
617+
expect(document.activeElement).not.toBe(nativeButton);
614618

615619
buttonToggleInstance.focus();
616620
fixture.detectChanges();
617621

618-
expect(document.activeElement).toBe(nativeRadioInput);
622+
expect(document.activeElement).toBe(nativeButton);
619623
});
620624

621625
it('should not assign a name to the underlying input if one is not passed in', () => {
622-
expect(buttonToggleNativeElement.querySelector('input')!.getAttribute('name')).toBeFalsy();
626+
expect(buttonToggleButtonElement.getAttribute('name')).toBeFalsy();
623627
});
624628

629+
it('should have correct aria-pressed attribute', () => {
630+
expect(buttonToggleButtonElement.getAttribute('aria-pressed'))
631+
.toBe('false');
632+
633+
buttonToggleLabelElement.click();
634+
635+
fixture.detectChanges();
636+
637+
expect(buttonToggleButtonElement.getAttribute('aria-pressed'))
638+
.toBe('true');
639+
});
625640
});
626641

627642
describe('aria-label handling ', () => {
628643
it('should not set the aria-label attribute if none is provided', () => {
629644
let fixture = TestBed.createComponent(StandaloneButtonToggle);
630645
let checkboxDebugElement = fixture.debugElement.query(By.directive(MatButtonToggle));
631646
let checkboxNativeElement = checkboxDebugElement.nativeElement;
632-
let inputElement = checkboxNativeElement.querySelector('input') as HTMLInputElement;
647+
let buttonElement = checkboxNativeElement.querySelector('button') as HTMLButtonElement;
633648

634649
fixture.detectChanges();
635-
expect(inputElement.hasAttribute('aria-label')).toBe(false);
650+
expect(buttonElement.hasAttribute('aria-label')).toBe(false);
636651
});
637652

638653
it('should use the provided aria-label', () => {
639654
let fixture = TestBed.createComponent(ButtonToggleWithAriaLabel);
640655
let checkboxDebugElement = fixture.debugElement.query(By.directive(MatButtonToggle));
641656
let checkboxNativeElement = checkboxDebugElement.nativeElement;
642-
let inputElement = checkboxNativeElement.querySelector('input') as HTMLInputElement;
657+
let buttonElement = checkboxNativeElement.querySelector('button') as HTMLButtonElement;
643658

644659
fixture.detectChanges();
645-
expect(inputElement.getAttribute('aria-label')).toBe('Super effective');
660+
expect(buttonElement.getAttribute('aria-label')).toBe('Super effective');
646661
});
647662
});
648663

649664
describe('with provided aria-labelledby ', () => {
650665
let checkboxDebugElement: DebugElement;
651666
let checkboxNativeElement: HTMLElement;
652-
let inputElement: HTMLInputElement;
667+
let buttonElement: HTMLButtonElement;
653668

654669
it('should use the provided aria-labelledby', () => {
655670
let fixture = TestBed.createComponent(ButtonToggleWithAriaLabelledby);
656671
checkboxDebugElement = fixture.debugElement.query(By.directive(MatButtonToggle));
657672
checkboxNativeElement = checkboxDebugElement.nativeElement;
658-
inputElement = checkboxNativeElement.querySelector('input') as HTMLInputElement;
673+
buttonElement = checkboxNativeElement.querySelector('button') as HTMLButtonElement;
659674

660675
fixture.detectChanges();
661-
expect(inputElement.getAttribute('aria-labelledby')).toBe('some-id');
676+
expect(buttonElement.getAttribute('aria-labelledby')).toBe('some-id');
662677
});
663678

664679
it('should not assign aria-labelledby if none is provided', () => {
665680
let fixture = TestBed.createComponent(StandaloneButtonToggle);
666681
checkboxDebugElement = fixture.debugElement.query(By.directive(MatButtonToggle));
667682
checkboxNativeElement = checkboxDebugElement.nativeElement;
668-
inputElement = checkboxNativeElement.querySelector('input') as HTMLInputElement;
683+
buttonElement = checkboxNativeElement.querySelector('button') as HTMLButtonElement;
669684

670685
fixture.detectChanges();
671-
expect(inputElement.getAttribute('aria-labelledby')).toBe(null);
686+
expect(buttonElement.getAttribute('aria-labelledby')).toBe(null);
672687
});
673688
});
674689

‎src/lib/button-toggle/button-toggle.ts‎

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ export class MatButtonToggleChange {
8282
],
8383
inputs: ['disabled'],
8484
host: {
85-
'[attr.role]': 'multiple ? "group" : "radiogroup"',
85+
'role': 'group',
8686
'class': 'mat-button-toggle-group',
8787
'[class.mat-button-toggle-vertical]': 'vertical'
8888
},
@@ -353,13 +353,13 @@ export class MatButtonToggle extends _MatButtonToggleMixinBase implements OnInit
353353
/** Type of the button toggle. Either 'radio' or 'checkbox'. */
354354
_type: ToggleType;
355355

356-
@ViewChild('input') _inputElement: ElementRef<HTMLInputElement>;
356+
@ViewChild('button') _buttonElement: ElementRef<HTMLButtonElement>;
357357

358358
/** The parent button toggle group (exclusive selection). Optional. */
359359
buttonToggleGroup: MatButtonToggleGroup;
360360

361-
/** Unique ID for the underlying `input` element. */
362-
get inputId(): string { return `${this.id}-input`; }
361+
/** Unique ID for the underlying `button` element. */
362+
get buttonId(): string { return `${this.id}-button`; }
363363

364364
/** The unique ID for this button toggle. */
365365
@Input() id: string;
@@ -432,33 +432,25 @@ export class MatButtonToggle extends _MatButtonToggleMixinBase implements OnInit
432432

433433
/** Focuses the button. */
434434
focus(): void {
435-
this._inputElement.nativeElement.focus();
435+
this._buttonElement.nativeElement.focus();
436436
}
437437

438-
/** Checks the button toggle due to an interaction with the underlying native input. */
439-
_onInputChange(event: Event) {
438+
/** Checks the button toggle due to an interaction with the underlying native button. */
439+
_onButtonClick(event: Event) {
440440
event.stopPropagation();
441441

442-
this._checked = this._isSingleSelector ? true : !this._checked;
442+
const newChecked = this._isSingleSelector ? true : !this._checked;
443443

444-
if (this.buttonToggleGroup) {
445-
this.buttonToggleGroup._syncButtonToggle(this, this._checked, true);
446-
this.buttonToggleGroup._onTouched();
447-
}
448-
449-
// Emit a change event when the native input does.
450-
this.change.emit(new MatButtonToggleChange(this, this.value));
451-
}
444+
if (newChecked !== this._checked) {
445+
this._checked = newChecked;
446+
if (this.buttonToggleGroup) {
447+
this.buttonToggleGroup._syncButtonToggle(this, this._checked, true);
448+
this.buttonToggleGroup._onTouched();
449+
}
452450

453-
_onInputClick(event: Event) {
454-
// We have to stop propagation for click events on the visual hidden input element.
455-
// By default, when a user clicks on a label element, a generated click event will be
456-
// dispatched on the associated input element. Since we are using a label element as our
457-
// root container, the click event on the `slide-toggle` will be executed twice.
458-
// The real click event will bubble up, and the generated click event also tries to bubble up.
459-
// This will lead to multiple click events.
460-
// Preventing bubbling for the second event will solve that issue.
461-
event.stopPropagation();
451+
// Emit a change event when the native button does.
452+
this.change.emit(new MatButtonToggleChange(this, this.value));
453+
}
462454
}
463455

464456
/**

0 commit comments

Comments
 (0)