Skip to content

Commit

Permalink
fix: omit keypress with ctrlKey or altKey (#615)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ph-fritsche
ph-fritsche authored Mar 23, 2021
1 parent 2b0632a commit 391e513
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 32 deletions.
17 changes: 17 additions & 0 deletions src/__tests__/keyboard/keyboardImplementation.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import userEvent from '../../index'
import {setup} from '../helpers/utils'

test('no character input if `altKey` or `ctrlKey` is pressed', () => {
const {element, eventWasFired} = setup(`<input/>`)
;(element as HTMLInputElement).focus()

userEvent.keyboard('[ControlLeft>]g')

expect(eventWasFired('keypress')).toBe(false)
expect(eventWasFired('input')).toBe(false)

userEvent.keyboard('[AltLeft>]g')

expect(eventWasFired('keypress')).toBe(false)
expect(eventWasFired('input')).toBe(false)
})
44 changes: 16 additions & 28 deletions src/__tests__/type-modifiers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ test('{alt}a{/alt}', () => {
userEvent.type(element, '{alt}a{/alt}')

expect(getEventSnapshot()).toMatchInlineSnapshot(`
Events fired on: input[value="a"]
Events fired on: input[value=""]
input[value=""] - pointerover
input[value=""] - pointerenter
Expand All @@ -252,11 +252,8 @@ test('{alt}a{/alt}', () => {
input[value=""] - click: Left (0)
input[value=""] - keydown: Alt (18) {alt}
input[value=""] - keydown: a (97) {alt}
input[value=""] - keypress: a (97) {alt}
input[value="a"] - input
"{CURSOR}" -> "a{CURSOR}"
input[value="a"] - keyup: a (97) {alt}
input[value="a"] - keyup: Alt (18)
input[value=""] - keyup: a (97) {alt}
input[value=""] - keyup: Alt (18)
`)
})

Expand Down Expand Up @@ -297,7 +294,7 @@ test('{ctrl}a{/ctrl}', () => {
userEvent.type(element, '{ctrl}a{/ctrl}')

expect(getEventSnapshot()).toMatchInlineSnapshot(`
Events fired on: input[value="a"]
Events fired on: input[value=""]
input[value=""] - pointerover
input[value=""] - pointerenter
Expand All @@ -314,11 +311,8 @@ test('{ctrl}a{/ctrl}', () => {
input[value=""] - click: Left (0)
input[value=""] - keydown: Control (17) {ctrl}
input[value=""] - keydown: a (97) {ctrl}
input[value=""] - keypress: a (97) {ctrl}
input[value="a"] - input
"{CURSOR}" -> "a{CURSOR}"
input[value="a"] - keyup: a (97) {ctrl}
input[value="a"] - keyup: Control (17)
input[value=""] - keyup: a (97) {ctrl}
input[value=""] - keyup: Control (17)
`)
})

Expand Down Expand Up @@ -896,7 +890,7 @@ test('{meta}{alt}{ctrl}a{/ctrl}{/alt}{/meta}', () => {
userEvent.type(element, '{meta}{alt}{ctrl}a{/ctrl}{/alt}{/meta}')

expect(getEventSnapshot()).toMatchInlineSnapshot(`
Events fired on: input[value="a"]
Events fired on: input[value=""]
input[value=""] - pointerover
input[value=""] - pointerenter
Expand All @@ -915,13 +909,10 @@ test('{meta}{alt}{ctrl}a{/ctrl}{/alt}{/meta}', () => {
input[value=""] - keydown: Alt (18) {alt}{meta}
input[value=""] - keydown: Control (17) {alt}{meta}{ctrl}
input[value=""] - keydown: a (97) {alt}{meta}{ctrl}
input[value=""] - keypress: a (97) {alt}{meta}{ctrl}
input[value="a"] - input
"{CURSOR}" -> "a{CURSOR}"
input[value="a"] - keyup: a (97) {alt}{meta}{ctrl}
input[value="a"] - keyup: Control (17) {alt}{meta}
input[value="a"] - keyup: Alt (18) {meta}
input[value="a"] - keyup: Meta (93)
input[value=""] - keyup: a (97) {alt}{meta}{ctrl}
input[value=""] - keyup: Control (17) {alt}{meta}
input[value=""] - keyup: Alt (18) {meta}
input[value=""] - keyup: Meta (93)
`)
})

Expand Down Expand Up @@ -1123,7 +1114,7 @@ test('any remaining type modifiers are automatically released at the end', () =>
userEvent.type(element, '{meta}{alt}{ctrl}a{/alt}')

expect(getEventSnapshot()).toMatchInlineSnapshot(`
Events fired on: input[value="a"]
Events fired on: input[value=""]
input[value=""] - pointerover
input[value=""] - pointerenter
Expand All @@ -1142,13 +1133,10 @@ test('any remaining type modifiers are automatically released at the end', () =>
input[value=""] - keydown: Alt (18) {alt}{meta}
input[value=""] - keydown: Control (17) {alt}{meta}{ctrl}
input[value=""] - keydown: a (97) {alt}{meta}{ctrl}
input[value=""] - keypress: a (97) {alt}{meta}{ctrl}
input[value="a"] - input
"{CURSOR}" -> "a{CURSOR}"
input[value="a"] - keyup: a (97) {alt}{meta}{ctrl}
input[value="a"] - keyup: Alt (18) {meta}{ctrl}
input[value="a"] - keyup: Meta (93) {ctrl}
input[value="a"] - keyup: Control (17)
input[value=""] - keyup: a (97) {alt}{meta}{ctrl}
input[value=""] - keyup: Alt (18) {meta}{ctrl}
input[value=""] - keyup: Meta (93) {ctrl}
input[value=""] - keyup: Control (17)
`)
})

Expand Down
13 changes: 9 additions & 4 deletions src/keyboard/keyboardImplementation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,7 @@ export async function keyboardImplementation(
state,
)

if (
unpreventedDefault &&
(keyDef.key?.length === 1 || keyDef.key === 'Enter')
) {
if (unpreventedDefault && hasKeyPress(keyDef, state)) {
keypress(keyDef, getCurrentElement, options, state)
}

Expand Down Expand Up @@ -194,3 +191,11 @@ function applyPlugins(

return !!plugin
}

function hasKeyPress(keyDef: keyboardKey, state: keyboardState) {
return (
(keyDef.key?.length === 1 || keyDef.key === 'Enter') &&
!state.modifiers.ctrl &&
!state.modifiers.alt
)
}

2 comments on commit 391e513

@gazrogers
Copy link

Choose a reason for hiding this comment

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

Is there a bug in this change?
I updated my React project this morning and updated @testing-library along with it. Now my tests of elements using 'onKeyPress' are failing.

Where previously userEvent.type would fire a keypress event, now no keypress event is fired.

@ph-fritsche
Copy link
Member Author

Choose a reason for hiding this comment

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

We previously triggered some ’keypress’ events that don't happen in the browser.
See #613

If you have different observations, please share the details on the issue.
Should there be new problems introduced by these changes, please file a new issue with a reproduction example.
Thanks for your help. :)

Please sign in to comment.