Skip to content

Commit c67de2a

Browse files
committed
fix(material-experimental/mdc-checkbox): remove extra a11y tree node for the <label/>
In the mdc checkbox component, removes the click handler on <label/> and handles stoping propgation of clicks on the label in the label's parent. This removes the extra a11y tree node on the label and fixes TalkBack having an extra navigation stop (#14385). A11y tree before this commit. It has an un-necessary node, which coresponds to the `<label>` element. ``` - Generic - Checkbox, "Field A" - Textlabel, "Field A" ``` A11y tree with this commit applied ``` - Generic - Checkbox, "Field A" ``` fixes #14385
1 parent 5d578ef commit c67de2a

File tree

4 files changed

+36
-12
lines changed

4 files changed

+36
-12
lines changed

src/material-experimental/mdc-checkbox/checkbox.html

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<div class="mdc-form-field"
2-
[class.mdc-form-field--align-end]="labelPosition == 'before'">
2+
[class.mdc-form-field--align-end]="labelPosition == 'before'" (click)="_preventBubblingFromLabel($event)">
33
<div #checkbox class="mdc-checkbox">
44
<!-- Render this element first so the input is on top. -->
55
<div class="mat-mdc-checkbox-touch-target" (click)="_onClick()"></div>
@@ -37,9 +37,13 @@
3737
[matRippleDisabled]="disableRipple || disabled"
3838
[matRippleCentered]="true"></div>
3939
</div>
40+
<!--
41+
Avoid putting a click handler on the <label/> to fix duplicate navigation stop on Talk Back
42+
(#14385). Putting a click handler on the <label/> caused this bug because the browser produced
43+
an unnecessary accessibility tree node.
44+
-->
4045
<label #label
41-
[for]="inputId"
42-
(click)="$event.stopPropagation()">
46+
[for]="inputId">
4347
<ng-content></ng-content>
4448
</label>
4549
</div>

src/material-experimental/mdc-checkbox/checkbox.spec.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,20 @@ describe('MDC-based MatCheckbox', () => {
306306
);
307307
}));
308308

309-
it('should not trigger the click event multiple times', fakeAsync(() => {
309+
it('should trigger the click once when clicking on the <input/>', fakeAsync(() => {
310+
spyOn(testComponent, 'onCheckboxClick');
311+
312+
expect(inputElement.checked).toBe(false);
313+
314+
inputElement.click();
315+
fixture.detectChanges();
316+
flush();
317+
318+
expect(inputElement.checked).toBe(true);
319+
expect(testComponent.onCheckboxClick).toHaveBeenCalledTimes(1);
320+
}));
321+
322+
it('should trigger the click event once when clicking on the label', fakeAsync(() => {
310323
// By default, when clicking on a label element, a generated click will be dispatched
311324
// on the associated input element.
312325
// Since we're using a label element and a visual hidden input, this behavior can led
@@ -1034,7 +1047,7 @@ describe('MatCheckboxDefaultOptions', () => {
10341047
/** Simple component for testing a single checkbox. */
10351048
@Component({
10361049
template: `
1037-
<div (click)="parentElementClicked = true" (keyup)="parentElementKeyedUp = true">
1050+
<div (click)="parentElementClicked = true" (keyup)="parentElementKeyedUp = true" (click)="onCheckboxClick($event)">
10381051
<mat-checkbox
10391052
[id]="checkboxId"
10401053
[required]="isRequired"
@@ -1045,7 +1058,6 @@ describe('MatCheckboxDefaultOptions', () => {
10451058
[color]="checkboxColor"
10461059
[disableRipple]="disableRipple"
10471060
[value]="checkboxValue"
1048-
(click)="onCheckboxClick($event)"
10491061
(change)="onCheckboxChange($event)">
10501062
Simple checkbox
10511063
</mat-checkbox>

src/material-experimental/mdc-checkbox/checkbox.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,4 +388,17 @@ export class MatCheckbox
388388
nativeCheckbox.nativeElement.indeterminate = value;
389389
}
390390
}
391+
392+
/**
393+
* Prevent click events that come from the `<label/>` element from bubbling. This prevents the
394+
* click handler on the host from triggering twice when clicking on the `<label/>` element. After
395+
* the click event on the `<label/>` propagates, the browsers dispatches click on the associated
396+
* `<input/>`. By preventing clicks on the label by bubbling, we ensure only one click event
397+
* bubbles when the label is clicked.
398+
*/
399+
_preventBubblingFromLabel(event: MouseEvent) {
400+
if (!!event.target && this._label.nativeElement.contains(event.target as HTMLElement)) {
401+
event.stopPropagation();
402+
}
403+
}
391404
}

src/material/checkbox/checkbox.spec.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -296,12 +296,7 @@ describe('MatCheckbox', () => {
296296
expect(checkboxNativeElement.classList).toContain('mat-checkbox-label-before');
297297
});
298298

299-
it('should not trigger the click event multiple times', () => {
300-
// By default, when clicking on a label element, a generated click will be dispatched
301-
// on the associated input element.
302-
// Since we're using a label element and a visual hidden input, this behavior can led
303-
// to an issue, where the click events on the checkbox are getting executed twice.
304-
299+
it('should trigger the click event once when clicking on the label', () => {
305300
spyOn(testComponent, 'onCheckboxClick');
306301

307302
expect(inputElement.checked).toBe(false);

0 commit comments

Comments
 (0)