Skip to content

Commit bd9d4e1

Browse files
committed
fix(material/datepicker): fix Voiceover losing focus on PageDown
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.
1 parent 43214b5 commit bd9d4e1

File tree

5 files changed

+187
-62
lines changed

5 files changed

+187
-62
lines changed

src/material/datepicker/calendar-body.ts

+33-10
Original file line numberDiff line numberDiff line change
@@ -216,21 +216,44 @@ export class MatCalendarBody implements OnChanges, OnDestroy, AfterViewChecked {
216216
return cellNumber == this.activeCell;
217217
}
218218

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

227-
if (activeCell) {
228-
if (!movePreview) {
229-
this._skipNextFocus = true;
230-
}
249+
if (activeCell) {
250+
if (!movePreview) {
251+
this._skipNextFocus = true;
252+
}
231253

232-
activeCell.focus();
233-
}
254+
activeCell.focus();
255+
}
256+
});
234257
});
235258
});
236259
}

src/material/datepicker/calendar.spec.ts

+11-3
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,14 @@ import {
77
MockNgZone,
88
} from '../../cdk/testing/private';
99
import {Component, NgZone} from '@angular/core';
10-
import {waitForAsync, ComponentFixture, inject, TestBed} from '@angular/core/testing';
10+
import {
11+
fakeAsync,
12+
waitForAsync,
13+
ComponentFixture,
14+
inject,
15+
TestBed,
16+
tick,
17+
} from '@angular/core/testing';
1118
import {DateAdapter, MatNativeDateModule} from '@angular/material/core';
1219
import {DEC, FEB, JAN, JUL, NOV} from '../testing';
1320
import {By} from '@angular/platform-browser';
@@ -190,7 +197,7 @@ describe('MatCalendar', () => {
190197
expect(activeCell.focus).not.toHaveBeenCalled();
191198
});
192199

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

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

202209
zone.simulateZoneExit();
210+
tick();
203211

204212
expect(activeCell.focus).toHaveBeenCalled();
205-
});
213+
}));
206214

207215
describe('year view', () => {
208216
beforeEach(() => {

src/material/datepicker/date-range-input.spec.ts

+1
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,7 @@ describe('MatDateRangeInput', () => {
675675

676676
rangePicker.open();
677677
fixture.detectChanges();
678+
tick();
678679
flush();
679680

680681
expect(startModel.dirty).toBe(false);

src/material/datepicker/datepicker-actions.spec.ts

+23-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {Component, ElementRef, Type, ViewChild} from '@angular/core';
2-
import {ComponentFixture, TestBed, flush, fakeAsync} from '@angular/core/testing';
2+
import {ComponentFixture, TestBed, flush, fakeAsync, tick} from '@angular/core/testing';
33
import {FormControl, FormsModule, ReactiveFormsModule} from '@angular/forms';
44
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
55
import {MatNativeDateModule} from '@angular/material/core';
@@ -28,37 +28,42 @@ describe('MatDatepickerActions', () => {
2828
return TestBed.createComponent(component);
2929
}
3030

31-
it('should render the actions inside calendar panel in popup mode', () => {
31+
it('should render the actions inside calendar panel in popup mode', fakeAsync(() => {
3232
const fixture = createComponent(DatepickerWithActions);
3333
fixture.detectChanges();
3434
fixture.componentInstance.datepicker.open();
3535
fixture.detectChanges();
36+
tick();
37+
flush();
3638

3739
const actions = document.querySelector('.mat-datepicker-content .mat-datepicker-actions');
3840
expect(actions).toBeTruthy();
3941
expect(actions?.querySelector('.cancel')).toBeTruthy();
4042
expect(actions?.querySelector('.apply')).toBeTruthy();
41-
});
43+
}));
4244

43-
it('should render the actions inside calendar panel in touch UI mode', () => {
45+
it('should render the actions inside calendar panel in touch UI mode', fakeAsync(() => {
4446
const fixture = createComponent(DatepickerWithActions);
4547
fixture.componentInstance.touchUi = true;
4648
fixture.detectChanges();
4749
fixture.componentInstance.datepicker.open();
4850
fixture.detectChanges();
51+
tick();
52+
flush();
4953

5054
const actions = document.querySelector('.mat-datepicker-content .mat-datepicker-actions');
5155
expect(actions).toBeTruthy();
5256
expect(actions?.querySelector('.cancel')).toBeTruthy();
5357
expect(actions?.querySelector('.apply')).toBeTruthy();
54-
});
58+
}));
5559

5660
it('should not assign the value or close the datepicker when a value is selected', fakeAsync(() => {
5761
const fixture = createComponent(DatepickerWithActions);
5862
fixture.detectChanges();
5963
const {control, datepicker, onDateChange, input} = fixture.componentInstance;
6064
datepicker.open();
6165
fixture.detectChanges();
66+
tick();
6267

6368
const content = document.querySelector('.mat-datepicker-content')!;
6469
const cells = content.querySelectorAll<HTMLElement>('.mat-calendar-body-cell');
@@ -86,6 +91,8 @@ describe('MatDatepickerActions', () => {
8691
const {control, datepicker, onDateChange, input} = fixture.componentInstance;
8792
datepicker.open();
8893
fixture.detectChanges();
94+
tick();
95+
flush();
8996

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

99106
cells[10].click();
100107
fixture.detectChanges();
108+
tick();
101109
flush();
102110

103111
expect(datepicker.opened).toBe(true);
@@ -125,6 +133,8 @@ describe('MatDatepickerActions', () => {
125133
fixture.detectChanges();
126134
datepicker.open();
127135
fixture.detectChanges();
136+
tick();
137+
flush();
128138

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

136146
cells[10].click();
137147
fixture.detectChanges();
148+
tick();
138149
flush();
139150

140151
expect(datepicker.opened).toBe(true);
@@ -156,6 +167,8 @@ describe('MatDatepickerActions', () => {
156167
const {control, datepicker, onDateChange, input} = fixture.componentInstance;
157168
datepicker.open();
158169
fixture.detectChanges();
170+
tick();
171+
flush();
159172

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

169182
cells[10].click();
170183
fixture.detectChanges();
184+
tick();
171185
flush();
172186

173187
expect(datepicker.opened).toBe(true);
@@ -192,6 +206,8 @@ describe('MatDatepickerActions', () => {
192206
const {control, datepicker, onDateChange} = fixture.componentInstance;
193207
datepicker.open();
194208
fixture.detectChanges();
209+
tick();
210+
flush();
195211

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

205221
cells[10].click();
206222
fixture.detectChanges();
223+
tick();
207224
flush();
208225

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

234251
cells[10].click();
235252
fixture.detectChanges();
253+
tick();
236254
flush();
237255

238256
expect(datepicker.opened).toBe(false);

0 commit comments

Comments
 (0)