Skip to content

Commit 22eca83

Browse files
authored
fix(list): don't redirect focus to first option on mouse focus (#19889)
We have some logic that redirects focus to the first option whenever the list receives focus. The main target for this is tabbing into the list, but because we were using the `focus` event, it was also happening for mouse focus which can cause the page to jump around for long lists. These changes make it so that we only move it on keyboard and programmatic focus. Fixes #18948.
1 parent fc67ff3 commit 22eca83

File tree

4 files changed

+52
-23
lines changed

4 files changed

+52
-23
lines changed

src/material/list/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ ng_test_library(
5858
),
5959
deps = [
6060
":list",
61+
"//src/cdk/a11y",
6162
"//src/cdk/keycodes",
6263
"//src/cdk/testing/private",
6364
"//src/material/core",

src/material/list/selection-list.spec.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,15 @@ import {
1313
QueryList,
1414
ViewChildren,
1515
} from '@angular/core';
16-
import {async, ComponentFixture, fakeAsync, TestBed, tick, flush} from '@angular/core/testing';
16+
import {
17+
async,
18+
ComponentFixture,
19+
fakeAsync,
20+
TestBed,
21+
tick,
22+
flush,
23+
inject,
24+
} from '@angular/core/testing';
1725
import {MatRipple, defaultRippleAnimationConfig, ThemePalette} from '@angular/material/core';
1826
import {By} from '@angular/platform-browser';
1927
import {
@@ -23,6 +31,7 @@ import {
2331
MatSelectionListChange
2432
} from './index';
2533
import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms';
34+
import {FocusMonitor} from '@angular/cdk/a11y';
2635

2736
describe('MatSelectionList without forms', () => {
2837
describe('with list option', () => {
@@ -302,6 +311,21 @@ describe('MatSelectionList without forms', () => {
302311
expect(listOptions[0].componentInstance.focus).toHaveBeenCalled();
303312
});
304313

314+
it('should not move focus to the first item if focus originated from a mouse interaction',
315+
fakeAsync(inject([FocusMonitor], (focusMonitor: FocusMonitor) => {
316+
spyOn(listOptions[0].componentInstance, 'focus').and.callThrough();
317+
318+
const manager = selectionList.componentInstance._keyManager;
319+
expect(manager.activeItemIndex).toBe(-1);
320+
321+
focusMonitor.focusVia(selectionList.nativeElement, 'mouse');
322+
fixture.detectChanges();
323+
flush();
324+
325+
expect(manager.activeItemIndex).toBe(-1);
326+
expect(listOptions[0].componentInstance.focus).not.toHaveBeenCalled();
327+
})));
328+
305329
it('should focus the previously focused option when the list takes focus a second time', () => {
306330
spyOn(listOptions[1].componentInstance, 'focus').and.callThrough();
307331

@@ -731,6 +755,7 @@ describe('MatSelectionList without forms', () => {
731755

732756
dispatchMouseEvent(rippleTarget, 'mousedown');
733757
dispatchMouseEvent(rippleTarget, 'mouseup');
758+
flush();
734759

735760
expect(rippleTarget.querySelectorAll('.mat-ripple-element').length)
736761
.toBe(0, 'Expected no ripples after list ripples are disabled.');

src/material/list/selection-list.ts

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {FocusableOption, FocusKeyManager} from '@angular/cdk/a11y';
9+
import {FocusableOption, FocusKeyManager, FocusMonitor} from '@angular/cdk/a11y';
1010
import {BooleanInput, coerceBooleanProperty} from '@angular/cdk/coercion';
1111
import {SelectionModel} from '@angular/cdk/collections';
1212
import {
@@ -319,7 +319,6 @@ export class MatListOption extends _MatListOptionMixinBase implements AfterConte
319319
host: {
320320
'role': 'listbox',
321321
'class': 'mat-selection-list mat-list-base',
322-
'(focus)': '_onFocus()',
323322
'(keydown)': '_keydown($event)',
324323
'[attr.aria-multiselectable]': 'multiple',
325324
'[attr.aria-disabled]': 'disabled.toString()',
@@ -417,7 +416,9 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements CanD
417416
constructor(private _element: ElementRef<HTMLElement>,
418417
// @breaking-change 11.0.0 Remove `tabIndex` parameter.
419418
@Attribute('tabindex') tabIndex: string,
420-
private _changeDetector: ChangeDetectorRef) {
419+
private _changeDetector: ChangeDetectorRef,
420+
// @breaking-change 11.0.0 `_focusMonitor` parameter to become required.
421+
private _focusMonitor?: FocusMonitor) {
421422
super();
422423
}
423424

@@ -460,6 +461,23 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements CanD
460461
}
461462
}
462463
});
464+
465+
// @breaking-change 11.0.0 Remove null assertion once _focusMonitor is required.
466+
this._focusMonitor?.monitor(this._element)
467+
.pipe(takeUntil(this._destroyed))
468+
.subscribe(origin => {
469+
if (origin === 'keyboard' || origin === 'program') {
470+
const activeIndex = this._keyManager.activeItemIndex;
471+
472+
if (!activeIndex || activeIndex === -1) {
473+
// If there is no active index, set focus to the first option.
474+
this._keyManager.setFirstItemActive();
475+
} else {
476+
// Otherwise, set focus to the active option.
477+
this._keyManager.setActiveItem(activeIndex);
478+
}
479+
}
480+
});
463481
}
464482

465483
ngOnChanges(changes: SimpleChanges) {
@@ -473,6 +491,8 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements CanD
473491
}
474492

475493
ngOnDestroy() {
494+
// @breaking-change 11.0.0 Remove null assertion once _focusMonitor is required.
495+
this._focusMonitor?.stopMonitoring(this._element);
476496
this._destroyed.next();
477497
this._destroyed.complete();
478498
this._isDestroyed = true;
@@ -575,22 +595,6 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements CanD
575595
this.selectionChange.emit(new MatSelectionListChange(this, option));
576596
}
577597

578-
/**
579-
* When the selection list is focused, we want to move focus to an option within the list. Do this
580-
* by setting the appropriate option to be active.
581-
*/
582-
_onFocus(): void {
583-
const activeIndex = this._keyManager.activeItemIndex;
584-
585-
if (!activeIndex || (activeIndex === -1)) {
586-
// If there is no active index, set focus to the first option.
587-
this._keyManager.setFirstItemActive();
588-
} else {
589-
// Otherwise, set focus to the active option.
590-
this._keyManager.setActiveItem(activeIndex);
591-
}
592-
}
593-
594598
/** Implemented as part of ControlValueAccessor. */
595599
writeValue(values: string[]): void {
596600
this._value = values;

tools/public_api_guard/material/list.d.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,9 @@ export declare class MatSelectionList extends _MatSelectionListMixinBase impleme
115115
selectedOptions: SelectionModel<MatListOption>;
116116
readonly selectionChange: EventEmitter<MatSelectionListChange>;
117117
tabIndex: number;
118-
constructor(_element: ElementRef<HTMLElement>, tabIndex: string, _changeDetector: ChangeDetectorRef);
118+
constructor(_element: ElementRef<HTMLElement>, tabIndex: string, _changeDetector: ChangeDetectorRef, _focusMonitor?: FocusMonitor | undefined);
119119
_emitChangeEvent(option: MatListOption): void;
120120
_keydown(event: KeyboardEvent): void;
121-
_onFocus(): void;
122121
_removeOptionFromList(option: MatListOption): MatListOption | null;
123122
_reportValueChange(): void;
124123
_setFocusedOption(option: MatListOption): void;
@@ -136,7 +135,7 @@ export declare class MatSelectionList extends _MatSelectionListMixinBase impleme
136135
static ngAcceptInputType_disabled: BooleanInput;
137136
static ngAcceptInputType_multiple: BooleanInput;
138137
static ɵcmp: i0.ɵɵComponentDefWithMeta<MatSelectionList, "mat-selection-list", ["matSelectionList"], { "disableRipple": "disableRipple"; "tabIndex": "tabIndex"; "color": "color"; "compareWith": "compareWith"; "disabled": "disabled"; "multiple": "multiple"; }, { "selectionChange": "selectionChange"; }, ["options"], ["*"]>;
139-
static ɵfac: i0.ɵɵFactoryDef<MatSelectionList, [null, { attribute: "tabindex"; }, null]>;
138+
static ɵfac: i0.ɵɵFactoryDef<MatSelectionList, [null, { attribute: "tabindex"; }, null, null]>;
140139
}
141140

142141
export declare class MatSelectionListChange {

0 commit comments

Comments
 (0)