Skip to content

Commit

Permalink
fix(focus-monitor): don't null-out focus until after event is finishe…
Browse files Browse the repository at this point in the history
…d with capture & bubble (#10721)
  • Loading branch information
mmalerba authored and josephperrott committed May 3, 2018
1 parent 9b3e9a3 commit 0b7572b
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 35 deletions.
40 changes: 38 additions & 2 deletions src/cdk/a11y/focus-monitor/focus-monitor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ import {
patchElementFocus,
} from '@angular/cdk/testing';
import {Component} from '@angular/core';
import {ComponentFixture, fakeAsync, inject, TestBed, tick} from '@angular/core/testing';
import {ComponentFixture, fakeAsync, flush, inject, TestBed, tick} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
import {FocusMonitor, FocusOrigin, TOUCH_BUFFER_MS} from './focus-monitor';
import {A11yModule} from '../index';
import {FocusMonitor, FocusOrigin, TOUCH_BUFFER_MS} from './focus-monitor';


describe('FocusMonitor', () => {
Expand Down Expand Up @@ -224,6 +224,7 @@ describe('cdkMonitorFocus', () => {
ButtonWithFocusClasses,
ComplexComponentWithMonitorElementFocus,
ComplexComponentWithMonitorSubtreeFocus,
ComplexComponentWithMonitorSubtreeFocusAnfMonitorElementFocus,
],
}).compileComponents();
});
Expand Down Expand Up @@ -391,6 +392,36 @@ describe('cdkMonitorFocus', () => {
expect(parentElement.classList.length).toBe(2, 'button should have exactly 2 focus classes');
}));
});

describe('complex component with cdkMonitorSubtreeFocus and cdkMonitorElementFocus', () => {
let fixture: ComponentFixture<ComplexComponentWithMonitorSubtreeFocusAnfMonitorElementFocus>;
let parentElement: HTMLElement;
let childElement: HTMLElement;
let focusMonitor: FocusMonitor;

beforeEach(inject([FocusMonitor], (fm: FocusMonitor) => {
focusMonitor = fm;
fixture =
TestBed.createComponent(ComplexComponentWithMonitorSubtreeFocusAnfMonitorElementFocus);
fixture.detectChanges();

parentElement = fixture.debugElement.query(By.css('div')).nativeElement;
childElement = fixture.debugElement.query(By.css('button')).nativeElement;

patchElementFocus(parentElement);
patchElementFocus(childElement);
}));

it('should add keyboard focus classes on both elements when child is focused via keyboard',
fakeAsync(() => {
focusMonitor.focusVia(childElement, 'keyboard');
fixture.detectChanges();
flush();

expect(parentElement.classList).toContain('cdk-keyboard-focused');
expect(childElement.classList).toContain('cdk-keyboard-focused');
}));
});
});


Expand Down Expand Up @@ -418,3 +449,8 @@ class ComplexComponentWithMonitorElementFocus {}
template: `<div tabindex="0" cdkMonitorSubtreeFocus><button></button></div>`
})
class ComplexComponentWithMonitorSubtreeFocus {}

@Component({
template: `<div cdkMonitorSubtreeFocus><button cdkMonitorElementFocus></button></div>`
})
class ComplexComponentWithMonitorSubtreeFocusAnfMonitorElementFocus {}
23 changes: 11 additions & 12 deletions src/cdk/a11y/focus-monitor/focus-monitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
Output,
SkipSelf,
} from '@angular/core';
import {of as observableOf, Observable, Subject, Subscription} from 'rxjs';
import {Observable, of as observableOf, Subject, Subscription} from 'rxjs';


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

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

Expand Down Expand Up @@ -302,20 +302,20 @@ export class FocusMonitor implements OnDestroy {
// 2) It was caused by a touch event, in which case we mark the origin as 'touch'.
// 3) The element was programmatically focused, in which case we should mark the origin as
// 'program'.
if (!this._origin) {
let origin = this._origin;
if (!origin) {
if (this._windowFocused && this._lastFocusOrigin) {
this._origin = this._lastFocusOrigin;
origin = this._lastFocusOrigin;
} else if (this._wasCausedByTouch(event)) {
this._origin = 'touch';
origin = 'touch';
} else {
this._origin = 'program';
origin = 'program';
}
}

this._setClasses(element, this._origin);
elementInfo.subject.next(this._origin);
this._lastFocusOrigin = this._origin;
this._origin = null;
this._setClasses(element, origin);
elementInfo.subject.next(origin);
this._lastFocusOrigin = origin;
}

/**
Expand Down Expand Up @@ -351,7 +351,6 @@ export class FocusMonitor implements OnDestroy {
this._unregisterGlobalListeners = () => {};
}
}

}


Expand Down
5 changes: 5 additions & 0 deletions src/demo-app/focus-origin/focus-origin-demo.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,8 @@
<p>div with subtree focus monitored</p>
<button>1</button><button>2</button>
</div>

<div class="demo-focusable" cdkMonitorSubtreeFocus>
<p>Parent div should get same focus origin as button when button is focused:</p>
<button class="demo-focusable" cdkMonitorElementFocus>focus me</button>
</div>
13 changes: 7 additions & 6 deletions src/lib/button-toggle/button-toggle.spec.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import {fakeAsync, tick, ComponentFixture, TestBed} from '@angular/core/testing';
import {dispatchMouseEvent} from '@angular/cdk/testing';
import {NgModel, FormsModule, ReactiveFormsModule, FormControl} from '@angular/forms';
import {Component, DebugElement, ViewChild, ViewChildren, QueryList} from '@angular/core';
import {Component, DebugElement, QueryList, ViewChild, ViewChildren} from '@angular/core';
import {ComponentFixture, fakeAsync, flush, TestBed, tick} from '@angular/core/testing';
import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms';
import {By} from '@angular/platform-browser';
import {
MatButtonToggleGroup,
MatButtonToggleGroupMultiple,
MatButtonToggle,
MatButtonToggleChange,
MatButtonToggleGroup,
MatButtonToggleGroupMultiple,
MatButtonToggleModule,
} from './index';

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

it('should toggle when clicked', fakeAsync(() => {
buttonToggleLabelElement.click();

fixture.detectChanges();
flush();

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

buttonToggleLabelElement.click();
fixture.detectChanges();
flush();

expect(buttonToggleInstance.checked).toBe(false);
}));
Expand Down
10 changes: 7 additions & 3 deletions src/lib/datepicker/datepicker.spec.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import {ENTER, ESCAPE, RIGHT_ARROW, UP_ARROW} from '@angular/cdk/keycodes';
import {Overlay, OverlayContainer, ScrollDispatcher} from '@angular/cdk/overlay';
import {
createKeyboardEvent,
dispatchEvent,
dispatchFakeEvent,
dispatchKeyboardEvent,
dispatchMouseEvent,
createKeyboardEvent,
dispatchEvent,
} from '@angular/cdk/testing';
import {Component, FactoryProvider, Type, ValueProvider, ViewChild} from '@angular/core';
import {ComponentFixture, fakeAsync, flush, inject, TestBed} from '@angular/core/testing';
Expand All @@ -23,12 +23,12 @@ import {
import {MatFormField, MatFormFieldModule} from '@angular/material/form-field';
import {By} from '@angular/platform-browser';
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
import {Subject} from 'rxjs';
import {MatInputModule} from '../input/index';
import {MatDatepicker} from './datepicker';
import {MatDatepickerInput} from './datepicker-input';
import {MatDatepickerToggle} from './datepicker-toggle';
import {MAT_DATEPICKER_SCROLL_STRATEGY, MatDatepickerIntl, MatDatepickerModule} from './index';
import {Subject} from 'rxjs';
import {Directionality} from '@angular/cdk/bidi';
import {BrowserDynamicTestingModule} from '@angular/platform-browser-dynamic/testing';

Expand Down Expand Up @@ -308,6 +308,7 @@ describe('MatDatepicker', () => {

testComponent.datepicker.open();
fixture.detectChanges();
flush();

let ownedElementId = inputEl.getAttribute('aria-owns');
expect(ownedElementId).not.toBeNull();
Expand Down Expand Up @@ -945,6 +946,7 @@ describe('MatDatepicker', () => {
testComponent.formField.color = 'primary';
testComponent.datepicker.open();
fixture.detectChanges();
flush();

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

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

contentEl = document.querySelector('.mat-datepicker-content')!;
fixture.detectChanges();
flush();

expect(contentEl.classList).toContain('mat-warn');
expect(contentEl.classList).not.toContain('mat-primary');
Expand All @@ -969,6 +972,7 @@ describe('MatDatepicker', () => {
testComponent.formField.color = 'warn';
testComponent.datepicker.open();
fixture.detectChanges();
flush();

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

Expand Down
18 changes: 6 additions & 12 deletions src/lib/slide-toggle/slide-toggle.spec.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,12 @@
import {MutationObserverFactory} from '@angular/cdk/observers';
import {dispatchFakeEvent} from '@angular/cdk/testing';
import {Component} from '@angular/core';
import {ComponentFixture, fakeAsync, flushMicrotasks, TestBed, tick} from '@angular/core/testing';
import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms';
import {defaultRippleAnimationConfig} from '@angular/material/core';
import {By, HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
import {
ComponentFixture,
TestBed,
fakeAsync,
tick,
flushMicrotasks,
} from '@angular/core/testing';
import {NgModel, FormsModule, ReactiveFormsModule, FormControl} from '@angular/forms';
import {MatSlideToggle, MatSlideToggleChange, MatSlideToggleModule} from './index';
import {TestGestureConfig} from '../slider/test-gesture-config';
import {dispatchFakeEvent} from '@angular/cdk/testing';
import {defaultRippleAnimationConfig} from '@angular/material/core';
import {MutationObserverFactory} from '@angular/cdk/observers';
import {MatSlideToggle, MatSlideToggleChange, MatSlideToggleModule} from './index';

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

0 comments on commit 0b7572b

Please sign in to comment.