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

fix(keyboard): trigger onChange in React #626

Merged
merged 2 commits into from Mar 25, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions package.json
Expand Up @@ -42,12 +42,16 @@
"devDependencies": {
"@testing-library/dom": "^7.28.1",
"@testing-library/jest-dom": "^5.11.6",
"@testing-library/react": "^11.2.5",
"@types/estree": "0.0.45",
"@types/jest-in-case": "^1.0.3",
"@types/react": "^17.0.3",
"is-ci": "^2.0.0",
"jest-in-case": "^1.0.2",
"jest-serializer-ansi": "^1.0.3",
"kcd-scripts": "^7.5.1",
"react": "^17.0.2",
"react-dom": "^17.0.2",
"typescript": "^4.1.2"
},
"peerDependencies": {
Expand Down
6 changes: 6 additions & 0 deletions src/__tests__/react/tsconfig.json
@@ -0,0 +1,6 @@
{
"extends": "../../../tsconfig.json",
"compilerOptions": {
"jsx": "react"
}
}
15 changes: 15 additions & 0 deletions src/__tests__/react/type.tsx
@@ -0,0 +1,15 @@
import React from 'react'
import {render, screen} from '@testing-library/react'
import userEvent from 'index'

test('trigger onChange SyntheticEvent on input', () => {
const inputHandler = jest.fn()
const changeHandler = jest.fn()

render(<input onInput={inputHandler} onChange={changeHandler} />)

userEvent.type(screen.getByRole('textbox'), 'abcdef')

expect(inputHandler).toHaveBeenCalledTimes(6)
expect(changeHandler).toHaveBeenCalledTimes(6)
})
44 changes: 38 additions & 6 deletions src/keyboard/shared/fireInputEvent.ts
Expand Up @@ -23,9 +23,11 @@ export function fireInputEvent(
) {
// apply the changes before firing the input event, so that input handlers can access the altered dom and selection
if (isContentEditable(element)) {
element.textContent = newValue
} else /* istanbul ignore else */ if (isElementType(element, ['input', 'textarea'])) {
element.value = newValue
applyNative(element, 'textContent', newValue)
} /* istanbul ignore else */ else if (
isElementType(element, ['input', 'textarea'])
) {
applyNative(element, 'value', newValue)
} else {
// TODO: properly type guard
throw new Error('Invalid Element')
Expand All @@ -48,7 +50,7 @@ function setSelectionRangeAfterInput(

function setSelectionRangeAfterInputHandler(
element: Element,
newValue: string
newValue: string,
) {
// if we *can* change the selection start, then we will if the new value
// is the same as the current value (so it wasn't programatically changed
Expand All @@ -60,11 +62,41 @@ function setSelectionRangeAfterInputHandler(

// don't apply this workaround on elements that don't necessarily report the visible value - e.g. number
// TODO: this could probably be only applied when there is keyboardState.carryValue
const expectedValue = value === newValue || (value === '' && hasUnreliableEmptyValue(element))
if(!expectedValue) {
const expectedValue =
value === newValue || (value === '' && hasUnreliableEmptyValue(element))
if (!expectedValue) {
// If the currentValue is different than the expected newValue and we *can*
// change the selection range, than we should set it to the length of the
// currentValue to ensure that the browser behavior is mimicked.
setSelectionRange(element, value.length, value.length)
}
}

/**
* React tracks the changes on element properties.
* This workaround tries to alter the DOM element without React noticing,
* so that it later picks up the change.
*
* @see https://github.com/facebook/react/blob/148f8e497c7d37a3c7ab99f01dec2692427272b1/packages/react-dom/src/client/inputValueTracking.js#L51-L104
*/
function applyNative<T extends Element, P extends keyof T>(
element: T,
propName: P,
propValue: T[P],
) {
const descriptor = Object.getOwnPropertyDescriptor(element, propName)
const nativeDescriptor = Object.getOwnPropertyDescriptor(
element.constructor.prototype,
propName,
)

if (descriptor && nativeDescriptor) {
Object.defineProperty(element, propName, nativeDescriptor)
}

element[propName] = propValue

if (descriptor) {
Object.defineProperty(element, propName, descriptor)
}
}
3 changes: 2 additions & 1 deletion tsconfig.json
@@ -1,7 +1,8 @@
{
"extends": "./node_modules/kcd-scripts/shared-tsconfig.json",
"compilerOptions": {
"allowJs": true
"allowJs": true,
"esModuleInterop": true
},
"include": ["./src", "./typings"]
}