Skip to content

Commit 13702c4

Browse files
committed
fix(radio): clicks not landing correctly in some cases on Chrome
This is along the same lines as #18285 since the radio button and slide toggle have a very similar setup. Whenever the radio button host receives focus we forward it immediately to the underlying input, but this bouncing around of focus could cause clicks to be interrupted in some cases. These changes make it so that we only move focus if the host was focused by the keyboard or mouse.
1 parent 8865e78 commit 13702c4

File tree

3 files changed

+25
-6
lines changed

3 files changed

+25
-6
lines changed

src/material/radio/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ ng_test_library(
5555
),
5656
deps = [
5757
":radio",
58+
"//src/cdk/a11y",
5859
"//src/cdk/testing/private",
5960
"@npm//@angular/forms",
6061
"@npm//@angular/platform-browser",

src/material/radio/radio.spec.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
import {async, ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing';
1+
import {async, ComponentFixture, fakeAsync, TestBed, tick, inject} from '@angular/core/testing';
22
import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms';
33
import {Component, DebugElement, ViewChild} from '@angular/core';
44
import {By} from '@angular/platform-browser';
55
import {dispatchFakeEvent} from '@angular/cdk/testing/private';
6+
import {FocusMonitor} from '@angular/cdk/a11y';
67

78
import {MAT_RADIO_DEFAULT_OPTIONS} from './radio';
89
import {MatRadioButton, MatRadioChange, MatRadioGroup, MatRadioModule} from './index';
@@ -387,6 +388,21 @@ describe('MatRadio', () => {
387388
expect(radioNativeElements[2].classList).toContain('mat-warn');
388389
});
389390

391+
it('should not manually move focus to underlying input when focus comes from mouse or touch',
392+
inject([FocusMonitor], (focusMonitor: FocusMonitor) => {
393+
const radioElement = radioNativeElements[0];
394+
const inputElement = radioInputElements[0];
395+
expect(document.activeElement).not.toBe(inputElement);
396+
397+
focusMonitor.focusVia(radioElement, 'mouse');
398+
fixture.detectChanges();
399+
expect(document.activeElement).not.toBe(inputElement);
400+
401+
focusMonitor.focusVia(radioElement, 'touch');
402+
fixture.detectChanges();
403+
expect(document.activeElement).not.toBe(inputElement);
404+
}));
405+
390406
});
391407

392408
describe('group with ngModel', () => {

src/material/radio/radio.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -351,10 +351,6 @@ const _MatRadioButtonMixinBase:
351351
'[attr.aria-label]': 'null',
352352
'[attr.aria-labelledby]': 'null',
353353
'[attr.aria-describedby]': 'null',
354-
// Note: under normal conditions focus shouldn't land on this element, however it may be
355-
// programmatically set, for example inside of a focus trap, in this case we want to forward
356-
// the focus to the native element.
357-
'(focus)': '_inputElement.nativeElement.focus()',
358354
},
359355
changeDetection: ChangeDetectionStrategy.OnPush,
360356
})
@@ -544,7 +540,13 @@ export class MatRadioButton extends _MatRadioButtonMixinBase
544540
this._focusMonitor
545541
.monitor(this._elementRef, true)
546542
.subscribe(focusOrigin => {
547-
if (!focusOrigin && this.radioGroup) {
543+
// Only forward focus manually when it was received programmatically or through the
544+
// keyboard. We should not do this for mouse/touch focus for two reasons:
545+
// 1. It can prevent clicks from landing in Chrome (see #18269).
546+
// 2. They're already handled by the wrapping `label` element.
547+
if (focusOrigin === 'keyboard' || focusOrigin === 'program') {
548+
this._inputElement.nativeElement.focus();
549+
} else if (!focusOrigin && this.radioGroup) {
548550
this.radioGroup._touch();
549551
}
550552
});

0 commit comments

Comments
 (0)