Skip to content

Commit fe29835

Browse files
authored
fix(radio): clicks not landing correctly in some cases on Chrome (#18357)
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 807498d commit fe29835

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';
@@ -395,6 +396,21 @@ describe('MatRadio', () => {
395396
.every(element => element.classList.contains('mat-focus-indicator'))).toBe(true);
396397
});
397398

399+
it('should not manually move focus to underlying input when focus comes from mouse or touch',
400+
inject([FocusMonitor], (focusMonitor: FocusMonitor) => {
401+
const radioElement = radioNativeElements[0];
402+
const inputElement = radioInputElements[0];
403+
expect(document.activeElement).not.toBe(inputElement);
404+
405+
focusMonitor.focusVia(radioElement, 'mouse');
406+
fixture.detectChanges();
407+
expect(document.activeElement).not.toBe(inputElement);
408+
409+
focusMonitor.focusVia(radioElement, 'touch');
410+
fixture.detectChanges();
411+
expect(document.activeElement).not.toBe(inputElement);
412+
}));
413+
398414
});
399415

400416
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
})
@@ -540,7 +536,13 @@ export class MatRadioButton extends _MatRadioButtonMixinBase
540536
this._focusMonitor
541537
.monitor(this._elementRef, true)
542538
.subscribe(focusOrigin => {
543-
if (!focusOrigin && this.radioGroup) {
539+
// Only forward focus manually when it was received programmatically or through the
540+
// keyboard. We should not do this for mouse/touch focus for two reasons:
541+
// 1. It can prevent clicks from landing in Chrome (see #18269).
542+
// 2. They're already handled by the wrapping `label` element.
543+
if (focusOrigin === 'keyboard' || focusOrigin === 'program') {
544+
this._inputElement.nativeElement.focus();
545+
} else if (!focusOrigin && this.radioGroup) {
544546
this.radioGroup._touch();
545547
}
546548
});

0 commit comments

Comments
 (0)