Skip to content

Commit b86fbaa

Browse files
crisbetoannieyw
authored andcommitted
fix(cdk/a11y): allow for origin of already focused element to be changed (#20966)
The way `FocusMonitor.focusVia` works is by calling `focus` on the specified element and waiting for a `focus` event to trigger so the origin is applied. The problem is that if `focusVia` is called on an element that already has focus, the event won't be dispatched and the origin won't be updated which can cause the UI to look stuck. These changes make it so that if we detect that an element is focused already, we update its classes and dispatch the relevant event without trying to focus it. Related to #20965. (cherry picked from commit c1ab0b8)
1 parent 69ed76c commit b86fbaa

File tree

5 files changed

+89
-12
lines changed

5 files changed

+89
-12
lines changed

src/cdk/a11y/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ ng_test_library(
5252
"//src/cdk/platform",
5353
"//src/cdk/portal",
5454
"//src/cdk/testing/private",
55+
"@npm//@angular/common",
5556
"@npm//@angular/platform-browser",
5657
"@npm//rxjs",
5758
],

src/cdk/a11y/focus-monitor/focus-monitor.spec.ts

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
createMouseEvent,
88
dispatchEvent,
99
} from '@angular/cdk/testing/private';
10+
import {DOCUMENT} from '@angular/common';
1011
import {Component, NgZone} from '@angular/core';
1112
import {ComponentFixture, fakeAsync, flush, inject, TestBed, tick} from '@angular/core/testing';
1213
import {By} from '@angular/platform-browser';
@@ -25,13 +26,39 @@ describe('FocusMonitor', () => {
2526
let buttonElement: HTMLElement;
2627
let focusMonitor: FocusMonitor;
2728
let changeHandler: (origin: FocusOrigin) => void;
29+
let fakeActiveElement: HTMLElement | null;
2830

2931
beforeEach(() => {
32+
fakeActiveElement = null;
33+
3034
TestBed.configureTestingModule({
3135
imports: [A11yModule],
32-
declarations: [
33-
PlainButton,
34-
],
36+
declarations: [PlainButton],
37+
providers: [{
38+
provide: DOCUMENT,
39+
useFactory: () => {
40+
// We have to stub out the `document` in order to be able to fake `activeElement`.
41+
const fakeDocument = {body: document.body};
42+
43+
[
44+
'createElement',
45+
'dispatchEvent',
46+
'querySelectorAll',
47+
'addEventListener',
48+
'removeEventListener'
49+
].forEach(method => {
50+
(fakeDocument as any)[method] = function() {
51+
return (document as any)[method].apply(document, arguments);
52+
};
53+
});
54+
55+
Object.defineProperty(fakeDocument, 'activeElement', {
56+
get: () => fakeActiveElement || document.activeElement
57+
});
58+
59+
return fakeDocument;
60+
}
61+
}]
3562
}).compileComponents();
3663
});
3764

@@ -294,6 +321,37 @@ describe('FocusMonitor', () => {
294321
expect(parent.classList).toContain('cdk-mouse-focused');
295322
}));
296323

324+
it('focusVia should change the focus origin when called on the focused node', fakeAsync(() => {
325+
spyOn(buttonElement, 'focus').and.callThrough();
326+
focusMonitor.focusVia(buttonElement, 'keyboard');
327+
flush();
328+
fakeActiveElement = buttonElement;
329+
330+
expect(buttonElement.classList.length)
331+
.toBe(2, 'button should have exactly 2 focus classes');
332+
expect(buttonElement.classList.contains('cdk-focused'))
333+
.toBe(true, 'button should have cdk-focused class');
334+
expect(buttonElement.classList.contains('cdk-keyboard-focused'))
335+
.toBe(true, 'button should have cdk-keyboard-focused class');
336+
expect(changeHandler).toHaveBeenCalledTimes(1);
337+
expect(changeHandler).toHaveBeenCalledWith('keyboard');
338+
expect(buttonElement.focus).toHaveBeenCalledTimes(1);
339+
340+
focusMonitor.focusVia(buttonElement, 'mouse');
341+
flush();
342+
fakeActiveElement = buttonElement;
343+
344+
expect(buttonElement.classList.length)
345+
.toBe(2, 'button should have exactly 2 focus classes');
346+
expect(buttonElement.classList.contains('cdk-focused'))
347+
.toBe(true, 'button should have cdk-focused class');
348+
expect(buttonElement.classList.contains('cdk-mouse-focused'))
349+
.toBe(true, 'button should have cdk-mouse-focused class');
350+
expect(changeHandler).toHaveBeenCalledTimes(2);
351+
expect(changeHandler).toHaveBeenCalledWith('mouse');
352+
expect(buttonElement.focus).toHaveBeenCalledTimes(1);
353+
}));
354+
297355
});
298356

299357
describe('FocusMonitor with "eventual" detection', () => {

src/cdk/a11y/focus-monitor/focus-monitor.ts

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -308,13 +308,20 @@ export class FocusMonitor implements OnDestroy {
308308
options?: FocusOptions): void {
309309

310310
const nativeElement = coerceElement(element);
311+
const focusedElement = this._getDocument().activeElement;
311312

312-
this._setOriginForCurrentEventQueue(origin);
313+
// If the element is focused already, calling `focus` again won't trigger the event listener
314+
// which means that the focus classes won't be updated. If that's the case, update the classes
315+
// directly without waiting for an event.
316+
if (nativeElement === focusedElement && this._elementInfo.has(nativeElement)) {
317+
this._originChanged(nativeElement, origin, this._elementInfo.get(nativeElement)!);
318+
} else {
319+
this._setOriginForCurrentEventQueue(origin);
313320

314-
// `focus` isn't available on the server
315-
if (typeof nativeElement.focus === 'function') {
316-
// Cast the element to `any`, because the TS typings don't have the `options` parameter yet.
317-
(nativeElement as any).focus(options);
321+
// `focus` isn't available on the server
322+
if (typeof nativeElement.focus === 'function') {
323+
nativeElement.focus(options);
324+
}
318325
}
319326
}
320327

@@ -438,10 +445,7 @@ export class FocusMonitor implements OnDestroy {
438445
return;
439446
}
440447

441-
const origin = this._getFocusOrigin(event);
442-
this._setClasses(element, origin);
443-
this._emitOrigin(elementInfo.subject, origin);
444-
this._lastFocusOrigin = origin;
448+
this._originChanged(element, this._getFocusOrigin(event), elementInfo);
445449
}
446450

447451
/**
@@ -541,6 +545,14 @@ export class FocusMonitor implements OnDestroy {
541545
clearTimeout(this._originTimeoutId);
542546
}
543547
}
548+
549+
/** Updates all the state on an element once its focus origin has changed. */
550+
private _originChanged(element: HTMLElement, origin: FocusOrigin,
551+
elementInfo: MonitoredElementInfo) {
552+
this._setClasses(element, origin);
553+
this._emitOrigin(elementInfo.subject, origin);
554+
this._lastFocusOrigin = origin;
555+
}
544556
}
545557

546558
/** Gets the target of an event, accounting for Shadow DOM. */

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -804,6 +804,9 @@ describe('MDC-based MatMenu', () => {
804804
fixture.componentInstance.trigger.openMenu();
805805
fixture.detectChanges();
806806

807+
// Reset the automatic focus when the menu is opened.
808+
(document.activeElement as HTMLElement)?.blur();
809+
807810
const panel = overlayContainerElement.querySelector('.mat-mdc-menu-panel')!;
808811
const items = Array.from(panel.querySelectorAll('.mat-mdc-menu-item')) as HTMLElement[];
809812
items.forEach(patchElementFocus);

src/material/menu/menu.spec.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -802,6 +802,9 @@ describe('MatMenu', () => {
802802
fixture.componentInstance.trigger.openMenu();
803803
fixture.detectChanges();
804804

805+
// Reset the automatic focus when the menu is opened.
806+
(document.activeElement as HTMLElement)?.blur();
807+
805808
const panel = overlayContainerElement.querySelector('.mat-menu-panel')!;
806809
const items = Array.from(panel.querySelectorAll('.mat-menu-item')) as HTMLElement[];
807810
items.forEach(patchElementFocus);

0 commit comments

Comments
 (0)