From a7f99066a5623e0bd05ce3092def09907a1d4dc6 Mon Sep 17 00:00:00 2001 From: Philipp Fritsche Date: Thu, 31 Mar 2022 11:44:18 +0200 Subject: [PATCH] fix: maintain UI value on controlled number input (#889) --- src/document/selection.ts | 6 -- src/document/value.ts | 90 ++++++++++++++++++++++------ src/utils/edit/input.ts | 13 +--- tests/document/index.ts | 24 +++++++- tests/react/{type.tsx => index.tsx} | 93 +++++++++++++++++++++++++++-- tests/react/keyboard.tsx | 37 ------------ 6 files changed, 187 insertions(+), 76 deletions(-) rename tests/react/{type.tsx => index.tsx} (67%) delete mode 100644 tests/react/keyboard.tsx diff --git a/src/document/selection.ts b/src/document/selection.ts index 6f01c507..5830150a 100644 --- a/src/document/selection.ts +++ b/src/document/selection.ts @@ -131,9 +131,3 @@ export function getUISelection( endOffset: Math.max(sel.anchorOffset, sel.focusOffset), } } - -export function clearUISelection( - element: HTMLInputElement | HTMLTextAreaElement, -) { - element[UISelection] = undefined -} diff --git a/src/document/value.ts b/src/document/value.ts index 7e763a11..c91a85c1 100644 --- a/src/document/value.ts +++ b/src/document/value.ts @@ -1,5 +1,6 @@ +import {isElementType} from '../utils' import {prepareInterceptor} from './interceptor' -import {clearUISelection} from './selection' +import {setUISelection} from './selection' const UIValue = Symbol('Displayed value in UI') const InitialValue = Symbol('Initial value to compare on blur') @@ -14,7 +15,11 @@ declare global { interface Element { [UIValue]?: string [InitialValue]?: string - [TrackChanges]?: string[] + [TrackChanges]?: { + previousValue?: string + tracked?: string[] + nextValue?: string + } } } @@ -24,23 +29,35 @@ function valueInterceptor( ) { const isUI = typeof v === 'object' && v[UIValue] - this[UIValue] = isUI ? String(v) : undefined - if (!isUI) { - trackValue(this, String(v)) - - this[InitialValue] = String(v) - - // Programmatically setting the value property - // moves the cursor to the end of the input. - clearUISelection(this) + if (isUI) { + this[UIValue] = String(v) + setPreviousValue(this, String(this.value)) + } else { + trackOrSetValue(this, String(v)) } return { applyNative: !!isUI, - realArgs: String(v), + realArgs: sanitizeValue(this, v), } } +function sanitizeValue( + element: HTMLInputElement | HTMLTextAreaElement, + v: Value | string, +) { + // Workaround for JSDOM + if ( + isElementType(element, 'input', {type: 'number'}) && + String(v) !== '' && + !Number.isNaN(Number(v)) + ) { + // Setting value to "1." results in `null` in JSDOM + return String(Number(v)) + } + return String(v) +} + export function prepareValueInterceptor(element: HTMLInputElement) { prepareInterceptor(element, 'value', valueInterceptor) } @@ -73,23 +90,62 @@ export function getInitialValue( return element[InitialValue] } +function setPreviousValue( + element: HTMLInputElement | HTMLTextAreaElement, + v: string, +) { + element[TrackChanges] = {...element[TrackChanges], previousValue: v} +} + export function startTrackValue( element: HTMLInputElement | HTMLTextAreaElement, ) { - element[TrackChanges] = [] + element[TrackChanges] = { + ...element[TrackChanges], + nextValue: String(element.value), + tracked: [], + } +} + +function trackOrSetValue( + element: HTMLInputElement | HTMLTextAreaElement, + v: string, +) { + element[TrackChanges]?.tracked?.push(v) + + if (!element[TrackChanges]?.tracked) { + setCleanValue(element, v) + } } -function trackValue( +function setCleanValue( element: HTMLInputElement | HTMLTextAreaElement, v: string, ) { - element[TrackChanges]?.push(v) + element[UIValue] = undefined + element[InitialValue] = v + + // Programmatically setting the value property + // moves the cursor to the end of the input. + setUISelection(element, {focusOffset: v.length}) } +/** + * @returns `true` if we recognize a React state reset and update + */ export function endTrackValue(element: HTMLInputElement | HTMLTextAreaElement) { - const tracked = element[TrackChanges] + const changes = element[TrackChanges] element[TrackChanges] = undefined - return tracked + const isJustReactStateUpdate = + changes?.tracked?.length === 2 && + changes.tracked[0] === changes.previousValue && + changes.tracked[1] === changes.nextValue + + if (changes?.tracked?.length && !isJustReactStateUpdate) { + setCleanValue(element, changes.tracked[changes.tracked.length - 1]) + } + + return isJustReactStateUpdate } diff --git a/src/utils/edit/input.ts b/src/utils/edit/input.ts index d98dd5cb..cc858626 100644 --- a/src/utils/edit/input.ts +++ b/src/utils/edit/input.ts @@ -167,11 +167,11 @@ function editInputElement( if (isDateOrTime(element)) { if (isValidDateOrTimeValue(element, newValue)) { - commitInput(config, element, oldValue, newValue, newOffset, {}) + commitInput(config, element, newOffset, {}) dispatchUIEvent(config, element, 'change') } } else { - commitInput(config, element, oldValue, newValue, newOffset, { + commitInput(config, element, newOffset, { data, inputType, }) @@ -228,8 +228,6 @@ function calculateNewValue( function commitInput( config: Config, element: EditableInputOrTextarea, - oldValue: string, - newValue: string, newOffset: number, inputInit: InputEventInit, ) { @@ -244,12 +242,7 @@ function commitInput( dispatchUIEvent(config, element, 'input', inputInit) - const tracked = endTrackValue(element as HTMLInputElement) - if ( - tracked?.length === 2 && - tracked[0] === oldValue && - tracked[1] === newValue - ) { + if (endTrackValue(element as HTMLInputElement)) { setSelection({ focusNode: element, anchorOffset: newOffset, diff --git a/tests/document/index.ts b/tests/document/index.ts index a4319d52..10e4c564 100644 --- a/tests/document/index.ts +++ b/tests/document/index.ts @@ -15,19 +15,39 @@ function prepare(element: Element) { } test('keep track of value in UI', async () => { + // JSDOM implements the `value` property differently than the browser. + // In the browser it is always a `string`. + // In JSDOM it is `null` or `number` for `` const {element} = render(``) prepare(element) - setUIValue(element, '2e-') + setUIValue(element, '2') + expect(element).toHaveValue(2) + setUIValue(element, '2e') + expect(element).toHaveValue(null) + expect(getUIValue(element)).toBe('2e') + + setUIValue(element, '2e-') expect(element).toHaveValue(null) expect(getUIValue(element)).toBe('2e-') - element.value = '3' + setUIValue(element, '2e-5') + expect(element).toHaveValue(2e-5) + expect(getUIValue(element)).toBe('2e-5') + element.value = '3' expect(element).toHaveValue(3) expect(getUIValue(element)).toBe('3') + + setUIValue(element, '3.') + expect(element).toHaveValue(3) + expect(getUIValue(element)).toBe('3.') + + setUIValue(element, '3.5') + expect(element).toHaveValue(3.5) + expect(getUIValue(element)).toBe('3.5') }) test('trigger `change` event if value changed since focus/set', async () => { diff --git a/tests/react/type.tsx b/tests/react/index.tsx similarity index 67% rename from tests/react/type.tsx rename to tests/react/index.tsx index 37ffbfae..c4db5a10 100644 --- a/tests/react/type.tsx +++ b/tests/react/index.tsx @@ -1,8 +1,93 @@ import React, {useState} from 'react' import {render, screen} from '@testing-library/react' import userEvent from '#src' +import {getUIValue} from '#src/document' import {addListeners} from '#testHelpers' +// Run twice to verify we handle this correctly no matter +// if React applies its magic before or after our document preparation. +test.each([0, 1])('maintain cursor position on controlled input', async () => { + function Input({initialValue}: {initialValue: string}) { + const [val, setVal] = useState(initialValue) + + return setVal(e.target.value)} /> + } + + render() + screen.getByRole('textbox').focus() + screen.getByRole('textbox').setSelectionRange(1, 1) + await userEvent.keyboard('b') + + expect(screen.getByRole('textbox')).toHaveValue('abcd') + expect(screen.getByRole('textbox')).toHaveProperty('selectionStart', 2) + expect(screen.getByRole('textbox')).toHaveProperty('selectionEnd', 2) +}) + +test('trigger Synthetic `keypress` event for printable characters', async () => { + const onKeyPress = jest.fn() + render() + const user = userEvent.setup() + screen.getByRole('textbox').focus() + + await user.keyboard('a') + expect(onKeyPress).toHaveBeenCalledTimes(1) + expect(onKeyPress.mock.calls[0][0]).toHaveProperty('charCode', 97) + + await user.keyboard('[Enter]') + expect(onKeyPress).toHaveBeenCalledTimes(2) + expect(onKeyPress.mock.calls[1][0]).toHaveProperty('charCode', 13) +}) + +test.each(['1.5', '1e5'])( + 'insert number with invalid intermediate values into controlled ``: %s', + async input => { + function Input() { + const [val, setVal] = useState('') + + return ( + setVal(e.target.value)} + /> + ) + } + render() + const user = userEvent.setup() + screen.getByRole('spinbutton').focus() + + await user.keyboard(input) + expect(getUIValue(screen.getByRole('spinbutton'))).toBe(input) + expect(screen.getByRole('spinbutton')).toHaveValue(Number(input)) + }, +) + +test('detect value change in event handler', async () => { + function Input() { + const [val, setVal] = useState('') + + return ( + { + if (Number(e.target.value) == 12) { + e.target.value = '34' + } + setVal(e.target.value) + }} + /> + ) + } + render() + const user = userEvent.setup() + screen.getByRole('spinbutton').focus() + + await user.keyboard('125') + expect(getUIValue(screen.getByRole('spinbutton'))).toBe('345') + expect(screen.getByRole('spinbutton')).toHaveValue(345) +}) + test('trigger onChange SyntheticEvent on input', async () => { const inputHandler = jest.fn() const changeHandler = jest.fn() @@ -16,7 +101,7 @@ test('trigger onChange SyntheticEvent on input', async () => { expect(changeHandler).toHaveBeenCalledTimes(6) }) -describe('typing in a controlled input', () => { +describe('typing in a formatted input', () => { function DollarInput({initialValue = ''}) { const [val, setVal] = useState(initialValue) return ( @@ -45,7 +130,7 @@ describe('typing in a controlled input', () => { } } - test('typing in empty controlled input', async () => { + test('typing in empty formatted input', async () => { const {element, getEventSnapshot, user} = setupDollarInput() await user.type(element, '23') @@ -81,7 +166,7 @@ describe('typing in a controlled input', () => { `) }) - test('typing in the middle of a controlled input', async () => { + test('typing in the middle of a formatted input', async () => { const {element, getEventSnapshot, user} = setupDollarInput({ initialValue: '$23', }) @@ -120,7 +205,7 @@ describe('typing in a controlled input', () => { `) }) - test('ignored {backspace} in controlled input', async () => { + test('ignored {backspace} in formatted input', async () => { const {element, getEventSnapshot, user} = setupDollarInput({ initialValue: '$23', }) diff --git a/tests/react/keyboard.tsx b/tests/react/keyboard.tsx deleted file mode 100644 index d49a86ae..00000000 --- a/tests/react/keyboard.tsx +++ /dev/null @@ -1,37 +0,0 @@ -import React, {useState} from 'react' -import {render, screen} from '@testing-library/react' -import userEvent from '#src' - -// Run twice to verify we handle this correctly no matter -// if React applies its magic before or after our document preparation. -test.each([0, 1])('maintain cursor position on controlled input', async () => { - function Input({initialValue}: {initialValue: string}) { - const [val, setVal] = useState(initialValue) - - return setVal(e.target.value)} /> - } - - render() - screen.getByRole('textbox').focus() - screen.getByRole('textbox').setSelectionRange(1, 1) - await userEvent.keyboard('b') - - expect(screen.getByRole('textbox')).toHaveValue('abcd') - expect(screen.getByRole('textbox')).toHaveProperty('selectionStart', 2) - expect(screen.getByRole('textbox')).toHaveProperty('selectionEnd', 2) -}) - -test('trigger Synthetic `keypress` event for printable characters', async () => { - const onKeyPress = jest.fn() - render() - const user = userEvent.setup() - screen.getByRole('textbox').focus() - - await user.keyboard('a') - expect(onKeyPress).toHaveBeenCalledTimes(1) - expect(onKeyPress.mock.calls[0][0]).toHaveProperty('charCode', 97) - - await user.keyboard('[Enter]') - expect(onKeyPress).toHaveBeenCalledTimes(2) - expect(onKeyPress.mock.calls[1][0]).toHaveProperty('charCode', 13) -})