Skip to content

Commit

Permalink
fix(datepicker): unable to close calendar when opened on focus in IE11
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
crisbeto committed Dec 10, 2017
1 parent b488b39 commit d166b24
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 10 deletions.
70 changes: 65 additions & 5 deletions src/lib/datepicker/datepicker.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -63,6 +63,7 @@ describe('MatDatepicker', () => {
NoInputDatepicker,
StandardDatepicker,
DatepickerWithEvents,
DatepickerOpeningOnFocus,
],
});

Expand Down Expand Up @@ -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();

Expand All @@ -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', () => {
Expand Down Expand Up @@ -997,18 +999,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<DatepickerOpeningOnFocus>;
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', () => {
Expand Down Expand Up @@ -1367,3 +1416,14 @@ class DatepickerWithEvents {
closedSpy = jasmine.createSpy('closed spy');
@ViewChild('d') datepicker: MatDatepicker<Date>;
}


@Component({
template: `
<input (focus)="d.open()" [matDatepicker]="d">
<mat-datepicker #d="matDatepicker"></mat-datepicker>
`,
})
class DatepickerOpeningOnFocus {
@ViewChild(MatDatepicker) datepicker: MatDatepicker<Date>;
}
20 changes: 15 additions & 5 deletions src/lib/datepicker/datepicker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,15 +310,25 @@ export class MatDatepicker<D> 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. */
Expand Down

0 comments on commit d166b24

Please sign in to comment.