Skip to content

Commit

Permalink
fix(material/datepicker): fix Voiceover losing focus on PageDown (#24399
Browse files Browse the repository at this point in the history
)

Add a timeout before programatically focusing cells on the calendar. This seems
to fix an issue where Voiceover loses focus when pressing the
PageDown/PageUp keys.

Fixes #24330.

(cherry picked from commit 6b4f2bf)
  • Loading branch information
zarend committed Mar 14, 2022
1 parent d2bb1e9 commit dfef173
Show file tree
Hide file tree
Showing 5 changed files with 187 additions and 62 deletions.
43 changes: 33 additions & 10 deletions src/material/datepicker/calendar-body.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,21 +195,44 @@ export class MatCalendarBody implements OnChanges, OnDestroy {
return cellNumber == this.activeCell;
}

/** Focuses the active cell after the microtask queue is empty. */
/**
* Focuses the active cell after the microtask queue is empty.
*
* Adding a 0ms setTimeout seems to fix Voiceover losing focus when pressing PageUp/PageDown
* (issue #24330).
*
* Determined a 0ms by gradually increasing duration from 0 and testing two use cases with screen
* reader enabled:
*
* 1. Pressing PageUp/PageDown repeatedly with pausing between each key press.
* 2. Pressing and holding the PageDown key with repeated keys enabled.
*
* Test 1 worked roughly 95-99% of the time with 0ms and got a little bit better as the duration
* increased. Test 2 got slightly better until the duration was long enough to interfere with
* repeated keys. If the repeated key speed was faster than the timeout duration, then pressing
* and holding pagedown caused the entire page to scroll.
*
* Since repeated key speed can verify across machines, determined that any duration could
* potentially interfere with repeated keys. 0ms would be best because it almost entirely
* eliminates the focus being lost in Voiceover (#24330) without causing unintended side effects.
* Adding delay also complicates writing tests.
*/
_focusActiveCell(movePreview = true) {
this._ngZone.runOutsideAngular(() => {
this._ngZone.onStable.pipe(take(1)).subscribe(() => {
const activeCell: HTMLElement | null = this._elementRef.nativeElement.querySelector(
'.mat-calendar-body-active',
);
setTimeout(() => {
const activeCell: HTMLElement | null = this._elementRef.nativeElement.querySelector(
'.mat-calendar-body-active',
);

if (activeCell) {
if (!movePreview) {
this._skipNextFocus = true;
}
if (activeCell) {
if (!movePreview) {
this._skipNextFocus = true;
}

activeCell.focus();
}
activeCell.focus();
}
});
});
});
}
Expand Down
14 changes: 11 additions & 3 deletions src/material/datepicker/calendar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,14 @@ import {
MockNgZone,
} from '../../cdk/testing/private';
import {Component, NgZone} from '@angular/core';
import {waitForAsync, ComponentFixture, inject, TestBed} from '@angular/core/testing';
import {
fakeAsync,
waitForAsync,
ComponentFixture,
inject,
TestBed,
tick,
} from '@angular/core/testing';
import {DateAdapter, MatNativeDateModule} from '@angular/material/core';
import {DEC, FEB, JAN, JUL, NOV} from '../testing';
import {By} from '@angular/platform-browser';
Expand Down Expand Up @@ -190,7 +197,7 @@ describe('MatCalendar', () => {
expect(activeCell.focus).not.toHaveBeenCalled();
});

it('should move focus to the active cell when the view changes', () => {
it('should move focus to the active cell when the view changes', fakeAsync(() => {
calendarInstance.currentView = 'multi-year';
fixture.detectChanges();

Expand All @@ -200,9 +207,10 @@ describe('MatCalendar', () => {
spyOn(activeCell, 'focus').and.callThrough();

zone.simulateZoneExit();
tick();

expect(activeCell.focus).toHaveBeenCalled();
});
}));

describe('year view', () => {
beforeEach(() => {
Expand Down
1 change: 1 addition & 0 deletions src/material/datepicker/date-range-input.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,7 @@ describe('MatDateRangeInput', () => {

rangePicker.open();
fixture.detectChanges();
tick();
flush();

expect(startModel.dirty).toBe(false);
Expand Down
28 changes: 23 additions & 5 deletions src/material/datepicker/datepicker-actions.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {Component, ElementRef, Type, ViewChild} from '@angular/core';
import {ComponentFixture, TestBed, flush, fakeAsync} from '@angular/core/testing';
import {ComponentFixture, TestBed, flush, fakeAsync, tick} from '@angular/core/testing';
import {FormControl, FormsModule, ReactiveFormsModule} from '@angular/forms';
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
import {MatNativeDateModule} from '@angular/material/core';
Expand Down Expand Up @@ -28,37 +28,42 @@ describe('MatDatepickerActions', () => {
return TestBed.createComponent(component);
}

it('should render the actions inside calendar panel in popup mode', () => {
it('should render the actions inside calendar panel in popup mode', fakeAsync(() => {
const fixture = createComponent(DatepickerWithActions);
fixture.detectChanges();
fixture.componentInstance.datepicker.open();
fixture.detectChanges();
tick();
flush();

const actions = document.querySelector('.mat-datepicker-content .mat-datepicker-actions');
expect(actions).toBeTruthy();
expect(actions?.querySelector('.cancel')).toBeTruthy();
expect(actions?.querySelector('.apply')).toBeTruthy();
});
}));

it('should render the actions inside calendar panel in touch UI mode', () => {
it('should render the actions inside calendar panel in touch UI mode', fakeAsync(() => {
const fixture = createComponent(DatepickerWithActions);
fixture.componentInstance.touchUi = true;
fixture.detectChanges();
fixture.componentInstance.datepicker.open();
fixture.detectChanges();
tick();
flush();

const actions = document.querySelector('.mat-datepicker-content .mat-datepicker-actions');
expect(actions).toBeTruthy();
expect(actions?.querySelector('.cancel')).toBeTruthy();
expect(actions?.querySelector('.apply')).toBeTruthy();
});
}));

it('should not assign the value or close the datepicker when a value is selected', fakeAsync(() => {
const fixture = createComponent(DatepickerWithActions);
fixture.detectChanges();
const {control, datepicker, onDateChange, input} = fixture.componentInstance;
datepicker.open();
fixture.detectChanges();
tick();

const content = document.querySelector('.mat-datepicker-content')!;
const cells = content.querySelectorAll<HTMLElement>('.mat-calendar-body-cell');
Expand Down Expand Up @@ -86,6 +91,8 @@ describe('MatDatepickerActions', () => {
const {control, datepicker, onDateChange, input} = fixture.componentInstance;
datepicker.open();
fixture.detectChanges();
tick();
flush();

const content = document.querySelector('.mat-datepicker-content')!;
const cells = content.querySelectorAll<HTMLElement>('.mat-calendar-body-cell');
Expand All @@ -98,6 +105,7 @@ describe('MatDatepickerActions', () => {

cells[10].click();
fixture.detectChanges();
tick();
flush();

expect(datepicker.opened).toBe(true);
Expand Down Expand Up @@ -125,6 +133,8 @@ describe('MatDatepickerActions', () => {
fixture.detectChanges();
datepicker.open();
fixture.detectChanges();
tick();
flush();

const content = document.querySelector('.mat-datepicker-content')!;
const cells = content.querySelectorAll<HTMLElement>('.mat-calendar-body-cell');
Expand All @@ -135,6 +145,7 @@ describe('MatDatepickerActions', () => {

cells[10].click();
fixture.detectChanges();
tick();
flush();

expect(datepicker.opened).toBe(true);
Expand All @@ -156,6 +167,8 @@ describe('MatDatepickerActions', () => {
const {control, datepicker, onDateChange, input} = fixture.componentInstance;
datepicker.open();
fixture.detectChanges();
tick();
flush();

const content = document.querySelector('.mat-datepicker-content')!;
const cells = content.querySelectorAll<HTMLElement>('.mat-calendar-body-cell');
Expand All @@ -168,6 +181,7 @@ describe('MatDatepickerActions', () => {

cells[10].click();
fixture.detectChanges();
tick();
flush();

expect(datepicker.opened).toBe(true);
Expand All @@ -192,6 +206,8 @@ describe('MatDatepickerActions', () => {
const {control, datepicker, onDateChange} = fixture.componentInstance;
datepicker.open();
fixture.detectChanges();
tick();
flush();

let content = document.querySelector('.mat-datepicker-content')!;
let actions = content.querySelector('.mat-datepicker-actions')!;
Expand All @@ -204,6 +220,7 @@ describe('MatDatepickerActions', () => {

cells[10].click();
fixture.detectChanges();
tick();
flush();

expect(datepicker.opened).toBe(true);
Expand Down Expand Up @@ -233,6 +250,7 @@ describe('MatDatepickerActions', () => {

cells[10].click();
fixture.detectChanges();
tick();
flush();

expect(datepicker.opened).toBe(false);
Expand Down
Loading

0 comments on commit dfef173

Please sign in to comment.