From e636e027800177e6a3c8f6fd203bea10ada25da9 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sun, 10 Dec 2017 15:24:25 +0100 Subject: [PATCH] fix(datepicker): unable to close calendar when opened on focus in IE11 Fixes not being able to close a datepicker's calendar in IE11, if the datepicker's trigger opens it on focus. The issue comes down to the fact that all browsers focus elements synchronously, whereas IE does so asynchronously. Since our logic depends on everything firing in sequence, when IE focuses at a later point, the datepicker is already considered as closed which causes the logic that restores focus to the trigger to reopen the calendar. Fixes #8914. --- src/lib/datepicker/datepicker.spec.ts | 69 +++++++++++++++++++++++++-- src/lib/datepicker/datepicker.ts | 20 ++++++-- 2 files changed, 79 insertions(+), 10 deletions(-) diff --git a/src/lib/datepicker/datepicker.spec.ts b/src/lib/datepicker/datepicker.spec.ts index eadd19caf9ff..0de4640bb937 100644 --- a/src/lib/datepicker/datepicker.spec.ts +++ b/src/lib/datepicker/datepicker.spec.ts @@ -8,7 +8,7 @@ import { dispatchMouseEvent, } from '@angular/cdk/testing'; import {Component, ViewChild} from '@angular/core'; -import {async, ComponentFixture, inject, TestBed} from '@angular/core/testing'; +import {async, ComponentFixture, inject, TestBed, fakeAsync, flush} from '@angular/core/testing'; import {FormControl, FormsModule, ReactiveFormsModule} from '@angular/forms'; import { DEC, @@ -63,6 +63,7 @@ describe('MatDatepicker', () => { NoInputDatepicker, StandardDatepicker, DatepickerWithEvents, + DatepickerOpeningOnFocus, ], }); @@ -248,7 +249,7 @@ describe('MatDatepicker', () => { }); it('clicking the currently selected date should close the calendar ' + - 'without firing selectedChanged', () => { + 'without firing selectedChanged', fakeAsync(() => { const selectedChangedSpy = spyOn(testComponent.datepicker.selectedChanged, 'emit').and.callThrough(); @@ -263,12 +264,13 @@ describe('MatDatepicker', () => { let cells = document.querySelectorAll('.mat-calendar-body-cell'); dispatchMouseEvent(cells[1], 'click'); fixture.detectChanges(); + flush(); } expect(selectedChangedSpy.calls.count()).toEqual(1); expect(document.querySelector('mat-dialog-container')).toBeNull(); expect(testComponent.datepickerInput.value).toEqual(new Date(2020, JAN, 2)); - }); + })); it('pressing enter on the currently selected date should close the calendar without ' + 'firing selectedChanged', () => { @@ -997,18 +999,64 @@ describe('MatDatepicker', () => { expect(testComponent.openedSpy).toHaveBeenCalled(); }); - it('should dispatch an event when a datepicker is closed', () => { + it('should dispatch an event when a datepicker is closed', fakeAsync(() => { testComponent.datepicker.open(); fixture.detectChanges(); testComponent.datepicker.close(); + flush(); fixture.detectChanges(); expect(testComponent.closedSpy).toHaveBeenCalled(); - }); + })); }); + describe('datepicker that opens on focus', () => { + let fixture: ComponentFixture; + let testComponent: DatepickerOpeningOnFocus; + let input: HTMLInputElement; + + beforeEach(fakeAsync(() => { + fixture = TestBed.createComponent(DatepickerOpeningOnFocus); + fixture.detectChanges(); + testComponent = fixture.componentInstance; + input = fixture.debugElement.query(By.css('input')).nativeElement; + })); + + it('should not reopen if the browser fires the focus event asynchronously', fakeAsync(() => { + // Stub out the `activeElement` so the datepicker always picks up the input. + spyOnProperty(document, 'activeElement', 'get').and.callFake(() => input); + + // Stub out the real focus method so we can call it reliably. + spyOn(input, 'focus').and.callFake(() => { + // Dispatch the event handler async to simulate the IE11 behavior. + Promise.resolve().then(() => dispatchFakeEvent(input, 'focus')); + }); + + // Open initially by focusing. + input.focus(); + fixture.detectChanges(); + flush(); + + // Ensure that the datepicker is actually open. + expect(testComponent.datepicker.opened).toBe(true, 'Expected datepicker to be open.'); + + // Close the datepicker. + testComponent.datepicker.close(); + fixture.detectChanges(); + + // Schedule the input to be focused asynchronously. + input.focus(); + fixture.detectChanges(); + + // Flush out the scheduled tasks. + flush(); + + expect(testComponent.datepicker.opened).toBe(false, 'Expected datepicker to be closed.'); + })); + }); + }); describe('with missing DateAdapter and MAT_DATE_FORMATS', () => { @@ -1367,3 +1415,14 @@ class DatepickerWithEvents { closedSpy = jasmine.createSpy('closed spy'); @ViewChild('d') datepicker: MatDatepicker; } + + +@Component({ + template: ` + + + `, +}) +class DatepickerOpeningOnFocus { + @ViewChild(MatDatepicker) datepicker: MatDatepicker; +} diff --git a/src/lib/datepicker/datepicker.ts b/src/lib/datepicker/datepicker.ts index 61d150e5a8be..205c293d6183 100644 --- a/src/lib/datepicker/datepicker.ts +++ b/src/lib/datepicker/datepicker.ts @@ -310,15 +310,25 @@ export class MatDatepicker implements OnDestroy { if (this._calendarPortal && this._calendarPortal.isAttached) { this._calendarPortal.detach(); } + + const completeClose = () => { + this._opened = false; + this.closedStream.emit(); + this._focusedElementBeforeOpen = null; + }; + if (this._focusedElementBeforeOpen && typeof this._focusedElementBeforeOpen.focus === 'function') { - + // Because IE moves focus asynchronously, we can't count on it being restored before we've + // marked the datepicker as closed. If the event fires out of sequence and the element that + // we're refocusing opens the datepicker on focus, the user could be stuck with not being + // able to close the calendar at all. We work around it by making the logic, that marks + // the datepicker as closed, async as well. this._focusedElementBeforeOpen.focus(); - this._focusedElementBeforeOpen = null; + setTimeout(completeClose); + } else { + completeClose(); } - - this._opened = false; - this.closedStream.emit(); } /** Open the calendar as a dialog. */