Skip to content

fix(cdk/testing): account for preventDefault in keyboard events #27483

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

fix(cdk/testing): account for preventDefault in keyboard events #27483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
68 changes: 54 additions & 14 deletions src/cdk/testing/testbed/fake-events/type-in-element.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,20 @@
*/

import {getNoKeysSpecifiedError, ModifierKeys} from '@angular/cdk/testing';
import {COMMA, PERIOD} from '@angular/cdk/keycodes';
import {
COMMA,
NINE,
PERIOD,
ZERO,
OPEN_SQUARE_BRACKET,
CLOSE_SQUARE_BRACKET,
SINGLE_QUOTE,
SEVEN,
FIVE,
ONE,
THREE,
FOUR,
} from '@angular/cdk/keycodes';
import {dispatchFakeEvent, dispatchKeyboardEvent} from './dispatch-events';
import {triggerFocus} from './element-focus';

Expand All @@ -25,6 +38,19 @@ const incrementalInputTypes = new Set([
/** Characters whose key code doesn't match their character code. */
const KEYCODE_MISMATCHES: Record<string, number> = {
',': COMMA,
'.': PERIOD,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that I listed just the most common conflicting characters that may show up in a test string. Our approach for mapping characters to keycodes using charCodeAt(0) is fundamentally flawed for many more edge cases. It has been this way since we introduced test harnesses.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better design we ought to use?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we would move away from using keyCode altogether, but there's a lot of code still depending on it.

The other option is to manually maintain a map of all characters to all key codes, but that'll only work for English letters and some keyboard layouts AFAIK.

I'm leaning more towards the first option but it'll take some effort.

'[': OPEN_SQUARE_BRACKET,
']': CLOSE_SQUARE_BRACKET,
'!': ONE,
'#': THREE,
'$': FOUR,
// These conflict with arrow keys which are fairly common in tests.
'(': NINE,
')': ZERO,
'&': SEVEN,
'%': FIVE,
"'": SINGLE_QUOTE,
'"': SINGLE_QUOTE,
};

/**
Expand Down Expand Up @@ -112,26 +138,40 @@ export function typeInElement(element: HTMLElement, ...modifiersAndKeys: any[])

triggerFocus(element);

// When we aren't entering the value incrementally, assign it all at once ahead
// of time so that any listeners to the key events below will have access to it.
if (!enterValueIncrementally) {
(element as HTMLInputElement).value = keys.reduce((value, key) => value + (key.key || ''), '');
}
let nonIncrementalValue = '';

for (const key of keys) {
dispatchKeyboardEvent(element, 'keydown', key.keyCode, key.key, modifiers);
dispatchKeyboardEvent(element, 'keypress', key.keyCode, key.key, modifiers);
if (isInput && key.key && key.key.length === 1) {
if (enterValueIncrementally) {
(element as HTMLInputElement | HTMLTextAreaElement).value += key.key;
dispatchFakeEvent(element, 'input');
const downEvent = dispatchKeyboardEvent(element, 'keydown', key.keyCode, key.key, modifiers);

// If the handler called `preventDefault` during `keydown`, the browser doesn't insert the
// value or dispatch `keypress` and `input` events. `keyup` is still dispatched.
if (!downEvent.defaultPrevented) {
const pressEvent = dispatchKeyboardEvent(
element,
'keypress',
key.keyCode,
key.key,
modifiers,
);

// If the handler called `preventDefault` during `keypress`, the browser doesn't insert the
// value or dispatch the `input` event. `keyup` is still dispatched.
if (!pressEvent.defaultPrevented && isInput && key.key && key.key.length === 1) {
if (enterValueIncrementally) {
element.value += key.key;
dispatchFakeEvent(element, 'input');
} else {
nonIncrementalValue += key.key;
}
}
}

dispatchKeyboardEvent(element, 'keyup', key.keyCode, key.key, modifiers);
}

// Since we weren't dispatching `input` events while sending the keys, we have to do it now.
if (!enterValueIncrementally) {
// Since we weren't adding the value or dispatching `input` events, we do it all at once now.
if (!enterValueIncrementally && nonIncrementalValue.length > 0 && isInput) {
element.value = nonIncrementalValue;
dispatchFakeEvent(element, 'input');
}
}
Expand Down
69 changes: 66 additions & 3 deletions src/cdk/testing/tests/cross-environment.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,19 +204,19 @@ export function crossEnvironmentSpecs(
});

it('should send enter key', async () => {
const specialKey = await harness.specaialKey();
const specialKey = await harness.specialKey();
await harness.sendEnter();
expect(await specialKey.text()).toBe('enter');
});

it('should send comma key', async () => {
const specialKey = await harness.specaialKey();
const specialKey = await harness.specialKey();
await harness.sendComma();
expect(await specialKey.text()).toBe(',');
});

it('should send alt+j key', async () => {
const specialKey = await harness.specaialKey();
const specialKey = await harness.specialKey();
await harness.sendAltJ();
expect(await specialKey.text()).toBe('alt-j');
});
Expand Down Expand Up @@ -289,6 +289,69 @@ export function crossEnvironmentSpecs(
expect(await element.getText()).toBe('Has comma inside attribute');
});

it(
'should prevent the value from changing and dispatch the correct event sequence ' +
'when preventDefault is called on an input during `keydown`',
async () => {
const button = await harness.preventDefaultKeydownButton();
const input = await harness.preventDefaultInput();
const value = await harness.preventDefaultInputValues();

await button.click();
await input.sendKeys('321');

expect((await value.text()).split('|')).toEqual([
// Event sequence for 3
'keydown - 3 - <empty>',
'keypress - 3 - <empty>',
'input - <none> - 3',
'keyup - 3 - 3',

// Event sequence for 2 which calls preventDefault
'keydown - 2 - 3',
'keyup - 2 - 3',

// Event sequence for 1
'keydown - 1 - 3',
'keypress - 1 - 3',
'input - <none> - 31',
'keyup - 1 - 31',
]);
},
);

it(
'should prevent the value from changing and dispatch the correct event sequence ' +
'when preventDefault is called on an input during `keypress`',
async () => {
const button = await harness.preventDefaultKeypressButton();
const input = await harness.preventDefaultInput();
const value = await harness.preventDefaultInputValues();

await button.click();
await input.sendKeys('321');

expect((await value.text()).split('|')).toEqual([
// Event sequence for 3
'keydown - 3 - <empty>',
'keypress - 3 - <empty>',
'input - <none> - 3',
'keyup - 3 - 3',

// Event sequence for 2 which calls preventDefault
'keydown - 2 - 3',
'keypress - 2 - 3',
'keyup - 2 - 3',

// Event sequence for 1
'keydown - 1 - 3',
'keypress - 1 - 3',
'input - <none> - 31',
'keyup - 1 - 31',
]);
},
);

if (!skipAsyncTests) {
it('should wait for async operation to complete', async () => {
const asyncCounter = await harness.asyncCounter();
Expand Down
6 changes: 5 additions & 1 deletion src/cdk/testing/tests/harnesses/main-component-harness.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ export class MainComponentHarness extends ComponentHarness {
readonly multiSelectChangeEventCounter = this.locatorFor('#multi-select-change-counter');
readonly numberInput = this.locatorFor('#number-input');
readonly numberInputValue = this.locatorFor('#number-input-value');
readonly preventDefaultInput = this.locatorFor('#prevent-default-input');
readonly preventDefaultInputValues = this.locatorFor('#prevent-default-input-values');
readonly preventDefaultKeydownButton = this.locatorFor('#prevent-default-keydown');
readonly preventDefaultKeypressButton = this.locatorFor('#prevent-default-keypress');
readonly contextmenuTestResult = this.locatorFor('.contextmenu-test-result');
readonly contenteditable = this.locatorFor('#contenteditable');
// Allow null for element
Expand Down Expand Up @@ -68,7 +72,7 @@ export class MainComponentHarness extends ComponentHarness {
SubComponentHarness.with({title: 'List of test tools', itemCount: 4}),
);
readonly lastList = this.locatorFor(SubComponentHarness.with({selector: ':last-child'}));
readonly specaialKey = this.locatorFor('.special-key');
readonly specialKey = this.locatorFor('.special-key');

readonly requiredAncestorRestrictedSubcomponent = this.locatorFor(
SubComponentHarness.with({ancestor: '.other'}),
Expand Down
15 changes: 15 additions & 0 deletions src/cdk/testing/tests/test-main-component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,21 @@ <h1 style="height: 100px; width: 200px;">Main Component</h1>

<input id="number-input" type="number" aria-label="Enter a number" [formControl]="numberControl">
<div id="number-input-value">Number value: {{numberControl.value}}</div>

<button
id="prevent-default-keydown"
(click)="preventDefaultEventType = 'keydown'">Prevent default on keydown</button>
<button
id="prevent-default-keypress"
(click)="preventDefaultEventType = 'keypress'">Prevent default on keypress</button>
<input
id="prevent-default-input"
type="text"
(keydown)="preventDefaultListener($event)"
(keypress)="preventDefaultListener($event)"
(input)="preventDefaultListener($event)"
(keyup)="preventDefaultListener($event)">
<div id="prevent-default-input-values">{{preventDefaultValues.join('|')}}</div>
</div>
<div class="subcomponents">
<test-sub class="test-special" title="test tools" [items]="testTools"></test-sub>
Expand Down
16 changes: 15 additions & 1 deletion src/cdk/testing/tests/test-main-component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {COMMA, ENTER} from '@angular/cdk/keycodes';
import {COMMA, ENTER, TWO} from '@angular/cdk/keycodes';
import {_supportsShadowDom} from '@angular/cdk/platform';
import {FormControl} from '@angular/forms';
import {
Expand Down Expand Up @@ -48,6 +48,8 @@ export class TestMainComponent implements OnDestroy {
clickResult = {x: -1, y: -1};
rightClickResult = {x: -1, y: -1, button: -1};
numberControl = new FormControl<number | null>(null);
preventDefaultEventType: string | null = null;
preventDefaultValues: string[] = [];

@ViewChild('clickTestElement') clickTestElement: ElementRef<HTMLElement>;
@ViewChild('taskStateResult') taskStateResultElement: ElementRef<HTMLElement>;
Expand Down Expand Up @@ -117,6 +119,18 @@ export class TestMainComponent implements OnDestroy {
this.customEventData = JSON.stringify({message: event.message, value: event.value});
}

preventDefaultListener(event: Event) {
// `input` events don't have a key
const key = event.type === 'input' ? '<none>' : (event as KeyboardEvent).key;
const input = event.target as HTMLInputElement;

if (event.type === this.preventDefaultEventType && (event as KeyboardEvent).keyCode === TWO) {
event.preventDefault();
}

this.preventDefaultValues.push(`${event.type} - ${key} - ${input.value || '<empty>'}`);
}

runTaskOutsideZone() {
this._zone.runOutsideAngular(() =>
setTimeout(() => {
Expand Down