Skip to content

Voiceover pagedown #24399

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 33 additions & 10 deletions src/material/datepicker/calendar-body.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,21 +216,44 @@ export class MatCalendarBody implements OnChanges, OnDestroy, AfterViewChecked {
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