From a4113821089f55b68af1f832ccb88ba966b1c223 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 13 Dec 2017 22:58:33 +0100 Subject: [PATCH] fix(datepicker): unable to close calendar when opened on focus in IE11 (#8918) 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 | 70 +++++++++++++++++++++++++-- src/lib/datepicker/datepicker.ts | 20 ++++++-- 2 files changed, 80 insertions(+), 10 deletions(-) diff --git a/src/lib/datepicker/datepicker.spec.ts b/src/lib/datepicker/datepicker.spec.ts index 53bcb684d5ee..3c74d6d315d8 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', () => { @@ -1020,18 +1022,65 @@ 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 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(); + + // Due to some browser limitations we can't install a stub on `document.activeElement` + // so instead we have to override the previously-focused element manually. + (fixture.componentInstance.datepicker as any)._focusedElementBeforeOpen = input; + + // 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', () => { @@ -1390,3 +1439,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 9a2c7e41bd24..527605aca827 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. */