Skip to content
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

onKeyDown charCode/keyCode/which is 0 #842

Closed
make-github-pseudonymous-again opened this issue Jan 23, 2022 · 3 comments
Closed

onKeyDown charCode/keyCode/which is 0 #842

make-github-pseudonymous-again opened this issue Jan 23, 2022 · 3 comments

Comments

@make-github-pseudonymous-again
  • @testing-library/user-event version: a5ca2e4
  • Testing Framework and version: NA
  • DOM Environment: NA

Problem description

Adding the following test to src/keyboard/keyboardAction.ts

test('trigger Synthetic `keydown` event for printable characters', async () => {
  const onKeyDown = jest.fn<unknown, [React.KeyboardEvent]>()
  render(<input onKeyDown={onKeyDown} />)
  screen.getByRole('textbox').focus()

  await userEvent.keyboard('a')
  expect(onKeyDown).toHaveBeenCalledTimes(1)
  expect(onKeyDown.mock.calls[0][0]).toHaveProperty('charCode', 65)

  await userEvent.keyboard('[Enter]')
  expect(onKeyDown).toHaveBeenCalledTimes(2)
  expect(onKeyDown.mock.calls[1][0]).toHaveProperty('charCode', 13)
})

yields the error

    trigger Synthetic `keydown` event for printable characters

    expect(received).toHaveProperty(path, value)

    Expected path: "charCode"

    Expected value: 65
    Received value: 0

      43 |   await userEvent.keyboard('a')
      44 |   expect(onKeyDown).toHaveBeenCalledTimes(1)
    > 45 |   expect(onKeyDown.mock.calls[0][0]).toHaveProperty('charCode', 65)
         |                                      ^
      46 |
      47 |   await userEvent.keyboard('[Enter]')
      48 |   expect(onKeyDown).toHaveBeenCalledTimes(2)

      at Object.<anonymous> (tests/react/keyboard.tsx:45:38)

Testing only [Enter] yields the same error. Found this while having trouble with my tests because UI relies on the charCode of keydown events. Expected?

The MDN docs on keyDown hint at the fact that codes should be present, but I do not know the specs. Missing implementation? I can help with providing a fix.

Suggested solution

Some patch was made to keypress to fix a similar issue (see 55e194a).
Similar patch should probably be made to keyup and keydown too, although the key mapping is not the same.

@ph-fritsche
Copy link
Member

KeyboardEvent.keyCode is deprecated. You should use KeyboardEvent.code instead.

KeyboardEvent.charCode is non-standard and deprecated. You should use KeyboardEvent.key instead.

UIEvent.which is non-standard. You should use KeyboardEvent.key, KeyboardEvent.code or MouseEvent.button.

We removed support for deprecated features, as we neither want to maintain features that nobody should use, nor want to encourage their usage.
We only apply charCode on the keypress because the React implementation still relies on it and behaves differently if the value is 0. We will remove support for this when React updates their implementation.

@make-github-pseudonymous-again
Copy link
Author

That's perfectly understandable. Thanks.

@coltanium13
Copy link

coltanium13 commented Sep 18, 2023

@ph-fritsche
Am i understanding it correctly that the KeyboardEvent.code for 'Enter' is still just keyboard('[Enter]'), that is the KeyboardEvent.code right?

What does it mean that you should use KeyboardEvent.code instead, does that mean use the keyboard('[Enter]') bracket syntax?

i also have this discussion question out:
#1164

alixlahuec added a commit to alixlahuec/zotero-roam that referenced this issue Sep 23, 2023
0.2 breaks Storybook testing because of the update to testing-library v14, which [removes support](testing-library/user-event#842 (comment)) for `keyCode`, `charCode`, and `which`.

Blueprint has moved to key names, but in [v5 only](https://github.com/palantir/blueprint/pull/6106/files#diff-fec59d61737971c807d9d0b765c8d0c3ba1d1f1880cdd4b3a652ab0789d016db). Since Roam is using v3, the extension has to follow.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants