Skip to content

Commit

Permalink
fix(material/datepicker): page scrolling for fast keyboard repeat (#2…
Browse files Browse the repository at this point in the history
…4991)

In an earlier change we introduced a timeout between when the datepicker is opened and when focus is moved into the current view. This means that the browser has some time to fire another keyboard event before we start preventing their default actions from inside the calendar, potentially allowing the page to be scrolled.

These changes fix the issue by always preventing the default action of navigation keys at the overlay level.

Fixes #24969.

(cherry picked from commit 4ca4fbd)
  • Loading branch information
crisbeto committed Jun 1, 2022
1 parent 6fd2711 commit 2443607
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 2 deletions.
30 changes: 29 additions & 1 deletion src/material/datepicker/datepicker-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,16 @@

import {Directionality} from '@angular/cdk/bidi';
import {BooleanInput, coerceBooleanProperty, coerceStringArray} from '@angular/cdk/coercion';
import {ESCAPE, hasModifierKey, UP_ARROW} from '@angular/cdk/keycodes';
import {
DOWN_ARROW,
ESCAPE,
hasModifierKey,
LEFT_ARROW,
PAGE_DOWN,
PAGE_UP,
RIGHT_ARROW,
UP_ARROW,
} from '@angular/cdk/keycodes';
import {
Overlay,
OverlayConfig,
Expand Down Expand Up @@ -657,6 +666,25 @@ export abstract class MatDatepickerBase<
this.close();
});

// The `preventDefault` call happens inside the calendar as well, however focus moves into
// it inside a timeout which can give browsers a chance to fire off a keyboard event in-between
// that can scroll the page (see #24969). Always block default actions of arrow keys for the
// entire overlay so the page doesn't get scrolled by accident.
overlayRef.keydownEvents().subscribe(event => {
const keyCode = event.keyCode;

if (
keyCode === UP_ARROW ||
keyCode === DOWN_ARROW ||
keyCode === LEFT_ARROW ||
keyCode === RIGHT_ARROW ||
keyCode === PAGE_UP ||
keyCode === PAGE_DOWN
) {
event.preventDefault();
}
});

this._componentRef = overlayRef.attach(portal);
this._forwardContentValues(this._componentRef.instance);

Expand Down
29 changes: 28 additions & 1 deletion src/material/datepicker/datepicker.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
import {Directionality} from '@angular/cdk/bidi';
import {DOWN_ARROW, ENTER, ESCAPE, RIGHT_ARROW, UP_ARROW} from '@angular/cdk/keycodes';
import {
DOWN_ARROW,
ENTER,
ESCAPE,
LEFT_ARROW,
PAGE_DOWN,
PAGE_UP,
RIGHT_ARROW,
UP_ARROW,
} from '@angular/cdk/keycodes';
import {Overlay} from '@angular/cdk/overlay';
import {ScrollDispatcher} from '@angular/cdk/scrolling';
import {
Expand Down Expand Up @@ -605,6 +614,24 @@ describe('MatDatepicker', () => {

expect(document.querySelector('.mat-datepicker-content')).toBeNull();
}));

it('should prevent the default action of navigation keys before the focus timeout has elapsed', fakeAsync(() => {
testComponent.datepicker.open();
fixture.detectChanges();

// Do the assertions before flushing the delays since we want
// to check specifically what happens before they have fired.
[UP_ARROW, DOWN_ARROW, LEFT_ARROW, RIGHT_ARROW, PAGE_UP, PAGE_DOWN].forEach(keyCode => {
const event = dispatchKeyboardEvent(document.body, 'keydown', keyCode);
fixture.detectChanges();
expect(event.defaultPrevented)
.withContext(`Expected default action to be prevented for key code ${keyCode}`)
.toBe(true);
});

tick();
flush();
}));
});

describe('datepicker with too many inputs', () => {
Expand Down

0 comments on commit 2443607

Please sign in to comment.