Skip to content

Commit 747bc0f

Browse files
tinayuangaotinayuangao
committed
fix(button-toggle): use native button and aria-pressed for button-toggle
1 parent 85039ca commit 747bc0f

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+
[name]="name"
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
@@ -79,3 +79,13 @@ $mat-button-toggle-border-radius: 2px !default;
7979
// Pointer events can be safely disabled because the ripple trigger element is the label element.
8080
pointer-events: none;
8181
}
82+
83+
.mat-button-toggle-button {
84+
border: 0;
85+
background: none;
86+
color: inherit;
87+
padding: inherit;
88+
margin: inherit;
89+
font: inherit;
90+
outline: none;
91+
}

‎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
}));
@@ -243,7 +243,7 @@ describe('MatButtonToggle without forms', () => {
243243
buttonToggleNativeElements = buttonToggleDebugElements
244244
.map(debugEl => debugEl.nativeElement);
245245

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

249249
buttonToggleInstances = buttonToggleDebugElements.map(debugEl => debugEl.componentInstance);
@@ -338,7 +338,7 @@ describe('MatButtonToggle without forms', () => {
338338
buttonToggleLabelElements[0].click();
339339
fixture.detectChanges();
340340
tick();
341-
expect(changeSpy).toHaveBeenCalled();
341+
expect(changeSpy).toHaveBeenCalledTimes(1);
342342

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

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

467467
nativeCheckboxLabel.click();
468468

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

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

491-
nativeCheckboxInput.click();
491+
nativeCheckboxButton.click();
492492
fixture.detectChanges();
493493

494494
expect(groupInstance.value).toEqual(['eggs']);
@@ -561,15 +561,19 @@ describe('MatButtonToggle without forms', () => {
561561
let buttonToggleNativeElement: HTMLElement;
562562
let buttonToggleLabelElement: HTMLLabelElement;
563563
let buttonToggleInstance: MatButtonToggle;
564+
let buttonToggleButtonElement: HTMLButtonElement;
564565

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

569570
buttonToggleDebugElement = fixture.debugElement.query(By.directive(MatButtonToggle));
570571
buttonToggleNativeElement = buttonToggleDebugElement.nativeElement;
571-
buttonToggleLabelElement = fixture.debugElement.query(By.css('label')).nativeElement;
572+
buttonToggleLabelElement = fixture.debugElement
573+
.query(By.css('.mat-button-toggle-label-content')).nativeElement;
572574
buttonToggleInstance = buttonToggleDebugElement.componentInstance;
575+
buttonToggleButtonElement =
576+
buttonToggleNativeElement.querySelector('button')! as HTMLButtonElement;
573577
}));
574578

575579
it('should toggle when clicked', fakeAsync(() => {
@@ -607,66 +611,77 @@ describe('MatButtonToggle without forms', () => {
607611
}));
608612

609613
it('should focus on underlying input element when focus() is called', () => {
610-
let nativeRadioInput = buttonToggleDebugElement.query(By.css('input')).nativeElement;
611-
expect(document.activeElement).not.toBe(nativeRadioInput);
614+
let nativeButton = buttonToggleDebugElement.query(By.css('button')).nativeElement;
615+
expect(document.activeElement).not.toBe(nativeButton);
612616

613617
buttonToggleInstance.focus();
614618
fixture.detectChanges();
615619

616-
expect(document.activeElement).toBe(nativeRadioInput);
620+
expect(document.activeElement).toBe(nativeButton);
617621
});
618622

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

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

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

632647
fixture.detectChanges();
633-
expect(inputElement.hasAttribute('aria-label')).toBe(false);
648+
expect(buttonElement.hasAttribute('aria-label')).toBe(false);
634649
});
635650

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

642657
fixture.detectChanges();
643-
expect(inputElement.getAttribute('aria-label')).toBe('Super effective');
658+
expect(buttonElement.getAttribute('aria-label')).toBe('Super effective');
644659
});
645660
});
646661

647662
describe('with provided aria-labelledby ', () => {
648663
let checkboxDebugElement: DebugElement;
649664
let checkboxNativeElement: HTMLElement;
650-
let inputElement: HTMLInputElement;
665+
let buttonElement: HTMLButtonElement;
651666

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

658673
fixture.detectChanges();
659-
expect(inputElement.getAttribute('aria-labelledby')).toBe('some-id');
674+
expect(buttonElement.getAttribute('aria-labelledby')).toBe('some-id');
660675
});
661676

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

668683
fixture.detectChanges();
669-
expect(inputElement.getAttribute('aria-labelledby')).toBe(null);
684+
expect(buttonElement.getAttribute('aria-labelledby')).toBe(null);
670685
});
671686
});
672687

‎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)