Skip to content

Commit 0b7572b

Browse files
mmalerbajosephperrott
authored andcommitted
fix(focus-monitor): don't null-out focus until after event is finished with capture & bubble (#10721)
1 parent 9b3e9a3 commit 0b7572b

File tree

6 files changed

+74
-35
lines changed

6 files changed

+74
-35
lines changed

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

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ import {
66
patchElementFocus,
77
} from '@angular/cdk/testing';
88
import {Component} from '@angular/core';
9-
import {ComponentFixture, fakeAsync, inject, TestBed, tick} from '@angular/core/testing';
9+
import {ComponentFixture, fakeAsync, flush, inject, TestBed, tick} from '@angular/core/testing';
1010
import {By} from '@angular/platform-browser';
11-
import {FocusMonitor, FocusOrigin, TOUCH_BUFFER_MS} from './focus-monitor';
1211
import {A11yModule} from '../index';
12+
import {FocusMonitor, FocusOrigin, TOUCH_BUFFER_MS} from './focus-monitor';
1313

1414

1515
describe('FocusMonitor', () => {
@@ -224,6 +224,7 @@ describe('cdkMonitorFocus', () => {
224224
ButtonWithFocusClasses,
225225
ComplexComponentWithMonitorElementFocus,
226226
ComplexComponentWithMonitorSubtreeFocus,
227+
ComplexComponentWithMonitorSubtreeFocusAnfMonitorElementFocus,
227228
],
228229
}).compileComponents();
229230
});
@@ -391,6 +392,36 @@ describe('cdkMonitorFocus', () => {
391392
expect(parentElement.classList.length).toBe(2, 'button should have exactly 2 focus classes');
392393
}));
393394
});
395+
396+
describe('complex component with cdkMonitorSubtreeFocus and cdkMonitorElementFocus', () => {
397+
let fixture: ComponentFixture<ComplexComponentWithMonitorSubtreeFocusAnfMonitorElementFocus>;
398+
let parentElement: HTMLElement;
399+
let childElement: HTMLElement;
400+
let focusMonitor: FocusMonitor;
401+
402+
beforeEach(inject([FocusMonitor], (fm: FocusMonitor) => {
403+
focusMonitor = fm;
404+
fixture =
405+
TestBed.createComponent(ComplexComponentWithMonitorSubtreeFocusAnfMonitorElementFocus);
406+
fixture.detectChanges();
407+
408+
parentElement = fixture.debugElement.query(By.css('div')).nativeElement;
409+
childElement = fixture.debugElement.query(By.css('button')).nativeElement;
410+
411+
patchElementFocus(parentElement);
412+
patchElementFocus(childElement);
413+
}));
414+
415+
it('should add keyboard focus classes on both elements when child is focused via keyboard',
416+
fakeAsync(() => {
417+
focusMonitor.focusVia(childElement, 'keyboard');
418+
fixture.detectChanges();
419+
flush();
420+
421+
expect(parentElement.classList).toContain('cdk-keyboard-focused');
422+
expect(childElement.classList).toContain('cdk-keyboard-focused');
423+
}));
424+
});
394425
});
395426

396427

@@ -418,3 +449,8 @@ class ComplexComponentWithMonitorElementFocus {}
418449
template: `<div tabindex="0" cdkMonitorSubtreeFocus><button></button></div>`
419450
})
420451
class ComplexComponentWithMonitorSubtreeFocus {}
452+
453+
@Component({
454+
template: `<div cdkMonitorSubtreeFocus><button cdkMonitorElementFocus></button></div>`
455+
})
456+
class ComplexComponentWithMonitorSubtreeFocusAnfMonitorElementFocus {}

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

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
Output,
1919
SkipSelf,
2020
} from '@angular/core';
21-
import {of as observableOf, Observable, Subject, Subscription} from 'rxjs';
21+
import {Observable, of as observableOf, Subject, Subscription} from 'rxjs';
2222

2323

2424
// This is the value used by AngularJS Material. Through trial and error (on iPhone 6S) they found
@@ -187,7 +187,7 @@ export class FocusMonitor implements OnDestroy {
187187
// focused element.
188188
let windowFocusListener = () => {
189189
this._windowFocused = true;
190-
this._windowFocusTimeoutId = setTimeout(() => this._windowFocused = false, 0);
190+
this._windowFocusTimeoutId = setTimeout(() => this._windowFocused = false);
191191
};
192192

193193
// Note: we listen to events in the capture phase so we can detect them even if the user stops
@@ -246,7 +246,7 @@ export class FocusMonitor implements OnDestroy {
246246
private _setOriginForCurrentEventQueue(origin: FocusOrigin): void {
247247
this._ngZone.runOutsideAngular(() => {
248248
this._origin = origin;
249-
this._originTimeoutId = setTimeout(() => this._origin = null, 0);
249+
this._originTimeoutId = setTimeout(() => this._origin = null);
250250
});
251251
}
252252

@@ -302,20 +302,20 @@ export class FocusMonitor implements OnDestroy {
302302
// 2) It was caused by a touch event, in which case we mark the origin as 'touch'.
303303
// 3) The element was programmatically focused, in which case we should mark the origin as
304304
// 'program'.
305-
if (!this._origin) {
305+
let origin = this._origin;
306+
if (!origin) {
306307
if (this._windowFocused && this._lastFocusOrigin) {
307-
this._origin = this._lastFocusOrigin;
308+
origin = this._lastFocusOrigin;
308309
} else if (this._wasCausedByTouch(event)) {
309-
this._origin = 'touch';
310+
origin = 'touch';
310311
} else {
311-
this._origin = 'program';
312+
origin = 'program';
312313
}
313314
}
314315

315-
this._setClasses(element, this._origin);
316-
elementInfo.subject.next(this._origin);
317-
this._lastFocusOrigin = this._origin;
318-
this._origin = null;
316+
this._setClasses(element, origin);
317+
elementInfo.subject.next(origin);
318+
this._lastFocusOrigin = origin;
319319
}
320320

321321
/**
@@ -351,7 +351,6 @@ export class FocusMonitor implements OnDestroy {
351351
this._unregisterGlobalListeners = () => {};
352352
}
353353
}
354-
355354
}
356355

357356

src/demo-app/focus-origin/focus-origin-demo.html

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,8 @@
1919
<p>div with subtree focus monitored</p>
2020
<button>1</button><button>2</button>
2121
</div>
22+
23+
<div class="demo-focusable" cdkMonitorSubtreeFocus>
24+
<p>Parent div should get same focus origin as button when button is focused:</p>
25+
<button class="demo-focusable" cdkMonitorElementFocus>focus me</button>
26+
</div>

src/lib/button-toggle/button-toggle.spec.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
1-
import {fakeAsync, tick, ComponentFixture, TestBed} from '@angular/core/testing';
21
import {dispatchMouseEvent} from '@angular/cdk/testing';
3-
import {NgModel, FormsModule, ReactiveFormsModule, FormControl} from '@angular/forms';
4-
import {Component, DebugElement, ViewChild, ViewChildren, QueryList} from '@angular/core';
2+
import {Component, DebugElement, QueryList, ViewChild, ViewChildren} from '@angular/core';
3+
import {ComponentFixture, fakeAsync, flush, TestBed, tick} from '@angular/core/testing';
4+
import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms';
55
import {By} from '@angular/platform-browser';
66
import {
7-
MatButtonToggleGroup,
8-
MatButtonToggleGroupMultiple,
97
MatButtonToggle,
108
MatButtonToggleChange,
9+
MatButtonToggleGroup,
10+
MatButtonToggleGroupMultiple,
1111
MatButtonToggleModule,
1212
} from './index';
1313

@@ -574,13 +574,14 @@ describe('MatButtonToggle without forms', () => {
574574

575575
it('should toggle when clicked', fakeAsync(() => {
576576
buttonToggleLabelElement.click();
577-
578577
fixture.detectChanges();
578+
flush();
579579

580580
expect(buttonToggleInstance.checked).toBe(true);
581581

582582
buttonToggleLabelElement.click();
583583
fixture.detectChanges();
584+
flush();
584585

585586
expect(buttonToggleInstance.checked).toBe(false);
586587
}));

src/lib/datepicker/datepicker.spec.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import {ENTER, ESCAPE, RIGHT_ARROW, UP_ARROW} from '@angular/cdk/keycodes';
22
import {Overlay, OverlayContainer, ScrollDispatcher} from '@angular/cdk/overlay';
33
import {
4+
createKeyboardEvent,
5+
dispatchEvent,
46
dispatchFakeEvent,
57
dispatchKeyboardEvent,
68
dispatchMouseEvent,
7-
createKeyboardEvent,
8-
dispatchEvent,
99
} from '@angular/cdk/testing';
1010
import {Component, FactoryProvider, Type, ValueProvider, ViewChild} from '@angular/core';
1111
import {ComponentFixture, fakeAsync, flush, inject, TestBed} from '@angular/core/testing';
@@ -23,12 +23,12 @@ import {
2323
import {MatFormField, MatFormFieldModule} from '@angular/material/form-field';
2424
import {By} from '@angular/platform-browser';
2525
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
26+
import {Subject} from 'rxjs';
2627
import {MatInputModule} from '../input/index';
2728
import {MatDatepicker} from './datepicker';
2829
import {MatDatepickerInput} from './datepicker-input';
2930
import {MatDatepickerToggle} from './datepicker-toggle';
3031
import {MAT_DATEPICKER_SCROLL_STRATEGY, MatDatepickerIntl, MatDatepickerModule} from './index';
31-
import {Subject} from 'rxjs';
3232
import {Directionality} from '@angular/cdk/bidi';
3333
import {BrowserDynamicTestingModule} from '@angular/platform-browser-dynamic/testing';
3434

@@ -308,6 +308,7 @@ describe('MatDatepicker', () => {
308308

309309
testComponent.datepicker.open();
310310
fixture.detectChanges();
311+
flush();
311312

312313
let ownedElementId = inputEl.getAttribute('aria-owns');
313314
expect(ownedElementId).not.toBeNull();
@@ -945,6 +946,7 @@ describe('MatDatepicker', () => {
945946
testComponent.formField.color = 'primary';
946947
testComponent.datepicker.open();
947948
fixture.detectChanges();
949+
flush();
948950

949951
let contentEl = document.querySelector('.mat-datepicker-content')!;
950952

@@ -959,6 +961,7 @@ describe('MatDatepicker', () => {
959961

960962
contentEl = document.querySelector('.mat-datepicker-content')!;
961963
fixture.detectChanges();
964+
flush();
962965

963966
expect(contentEl.classList).toContain('mat-warn');
964967
expect(contentEl.classList).not.toContain('mat-primary');
@@ -969,6 +972,7 @@ describe('MatDatepicker', () => {
969972
testComponent.formField.color = 'warn';
970973
testComponent.datepicker.open();
971974
fixture.detectChanges();
975+
flush();
972976

973977
const contentEl = document.querySelector('.mat-datepicker-content')!;
974978

src/lib/slide-toggle/slide-toggle.spec.ts

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,12 @@
1+
import {MutationObserverFactory} from '@angular/cdk/observers';
2+
import {dispatchFakeEvent} from '@angular/cdk/testing';
13
import {Component} from '@angular/core';
4+
import {ComponentFixture, fakeAsync, flushMicrotasks, TestBed, tick} from '@angular/core/testing';
5+
import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms';
6+
import {defaultRippleAnimationConfig} from '@angular/material/core';
27
import {By, HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
3-
import {
4-
ComponentFixture,
5-
TestBed,
6-
fakeAsync,
7-
tick,
8-
flushMicrotasks,
9-
} from '@angular/core/testing';
10-
import {NgModel, FormsModule, ReactiveFormsModule, FormControl} from '@angular/forms';
11-
import {MatSlideToggle, MatSlideToggleChange, MatSlideToggleModule} from './index';
128
import {TestGestureConfig} from '../slider/test-gesture-config';
13-
import {dispatchFakeEvent} from '@angular/cdk/testing';
14-
import {defaultRippleAnimationConfig} from '@angular/material/core';
15-
import {MutationObserverFactory} from '@angular/cdk/observers';
9+
import {MatSlideToggle, MatSlideToggleChange, MatSlideToggleModule} from './index';
1610

1711
describe('MatSlideToggle without forms', () => {
1812
let gestureConfig: TestGestureConfig;

0 commit comments

Comments
 (0)