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 407 type on no value elements #414

Merged
Merged
Show file tree
Hide file tree
Changes from 3 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
28 changes: 14 additions & 14 deletions src/__tests__/type-modifiers.js
Expand Up @@ -471,14 +471,10 @@ test('{space} on a button', () => {
`)
})

test('{space} with preventDefault keydown on button', () => {
const {element, getEventSnapshot} = setup('<button />', {
eventHandlers: {
keyDown: e => e.preventDefault(),
},
})
test(`' ' on a button is the same as '{space}'`, () => {
const {element, getEventSnapshot} = setup('<button />')

userEvent.type(element, '{space}')
userEvent.type(element, ' ')

expect(getEventSnapshot()).toMatchInlineSnapshot(`
Events fired on: button
Expand All @@ -497,15 +493,20 @@ test('{space} with preventDefault keydown on button', () => {
button - mouseup: Left (0)
button - click: Left (0)
button - keydown: (32)
button - keypress: (32)
visualjerk marked this conversation as resolved.
Show resolved Hide resolved
button - keyup: (32)
button - click: Left (0)
`)
})

test(`' ' on a button`, () => {
const {element, getEventSnapshot} = setup('<button />')
test('{space} with preventDefault keydown on button', () => {
const {element, getEventSnapshot} = setup('<button />', {
eventHandlers: {
keyDown: e => e.preventDefault(),
},
})

userEvent.type(element, ' ')
userEvent.type(element, '{space}')

expect(getEventSnapshot()).toMatchInlineSnapshot(`
Events fired on: button
Expand All @@ -524,7 +525,6 @@ test(`' ' on a button`, () => {
button - mouseup: Left (0)
button - click: Left (0)
button - keydown: (32)
button - keypress: (32)
button - keyup: (32)
button - click: Left (0)
`)
Expand Down Expand Up @@ -559,7 +559,7 @@ test('{space} on an input', () => {
`)
})

test('{enter} on an input type="color"', () => {
test('{enter} on an input type="color" fires same events as a button', () => {
const {element, getEventSnapshot} = setup(
`<input value="#ffffff" type="color" />`,
)
Expand Down Expand Up @@ -589,7 +589,7 @@ test('{enter} on an input type="color"', () => {
`)
})

test('{space} on an input type="color"', () => {
test('{space} on an input type="color" fires same events as a button', () => {
const {element, getEventSnapshot} = setup(
`<input value="#ffffff" type="color" />`,
)
Expand Down Expand Up @@ -619,7 +619,7 @@ test('{space} on an input type="color"', () => {
`)
})

test('" " on an input type="color"', () => {
test(`' ' on input type="color" is the same as '{space}'`, () => {
const {element, getEventSnapshot} = setup(
`<input value="#ffffff" type="color" />`,
)
Expand Down
24 changes: 10 additions & 14 deletions src/__tests__/type.js
Expand Up @@ -707,13 +707,12 @@ test('typing an invalid input value', () => {
expect(element.validity.badInput).toBe(false)
})

test('should give error if we are trying to call type on an invalid element', async () => {
const {element} = setup('<div />')
await expect(() =>
userEvent.type(element, "I'm only a div :("),
).rejects.toThrowErrorMatchingInlineSnapshot(
`"the current element is of type BODY and doesn't have a valid value"`,
)
test('should not throw error if we are trying to call type on an element without a value', () => {
const {element} = setup('<div />')
expect.assertions(0)
return userEvent
.type(element, "I'm only a div :(")
.catch(e => expect(e).toBeUndefined())
})

test('typing on button should not alter its value', () => {
Expand Down Expand Up @@ -752,11 +751,8 @@ test('typing on input type submit should not alter its value', () => {
expect(element).toHaveValue('foo')
})

test('typing on input type file should not trigger input event', () => {
const inputHandler = jest.fn()
const {element} = setup('<input type="file" />', {
eventHandlers: {input: inputHandler},
})
userEvent.type(element, 'bar')
expect(inputHandler).toHaveBeenCalledTimes(0)
test('typing on input type file should not result in an error', () => {
const {element} = setup('<input type="file" />')
expect.assertions(0)
return userEvent.type(element, 'bar').catch(e => expect(e).toBeUndefined())
})
6 changes: 1 addition & 5 deletions src/paste.js
Expand Up @@ -38,11 +38,7 @@ function paste(
fireEvent.paste(element, init)

if (!element.readOnly) {
const {newValue, newSelectionStart} = calculateNewValue(
text,
element,
element.value,
)
const {newValue, newSelectionStart} = calculateNewValue(text, element)
fireEvent.input(element, {
inputType: 'insertFromPaste',
target: {value: newValue},
Expand Down
78 changes: 23 additions & 55 deletions src/type.js
Expand Up @@ -88,17 +88,6 @@ async function typeImpl(
// The focused element could change between each event, so get the currently active element each time
const currentElement = () => getActiveElement(element.ownerDocument)

const currentValue = () => {
const activeElement = currentElement()
const value = activeElement.value
if (typeof value === 'undefined') {
throw new TypeError(
`the current element is of type ${activeElement.tagName} and doesn't have a valid value`,
)
}
return value
}

// by default, a new element has it's selection start and end at 0
// but most of the time when people call "type", they expect it to type
// at the end of the current input value. So, if the selection start
Expand All @@ -107,14 +96,16 @@ async function typeImpl(
// the only time it would make sense to pass the initialSelectionStart or
// initialSelectionEnd is if you have an input with a value and want to
// explicitely start typing with the cursor at 0. Not super common.
const value = currentElement().value
if (
value != null &&
currentElement().selectionStart === 0 &&
currentElement().selectionEnd === 0
) {
setSelectionRangeIfNecessary(
currentElement(),
initialSelectionStart ?? currentValue().length,
initialSelectionEnd ?? currentValue().length,
initialSelectionStart ?? value.length,
initialSelectionEnd ?? value.length,
)
}

Expand Down Expand Up @@ -145,7 +136,6 @@ async function typeImpl(
if (!currentElement().disabled) {
const returnValue = callback({
currentElement,
currentValue,
prevWasMinus,
prevWasPeriod,
prevValue,
Expand Down Expand Up @@ -215,26 +205,21 @@ function getSpecialCharCallback(remainingString) {

function getTypeCallback(remainingString) {
const character = remainingString[0]
const callback = createTypeCharacter(character)
const callback = context => typeCharacter(character, context)
return {
callback,
remainingString: remainingString.slice(1),
}
}

function setSelectionRange({
currentElement,
currentValue,
newValue,
newSelectionStart,
}) {
function setSelectionRange({currentElement, newValue, newSelectionStart}) {
// 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
// when the fireEvent.input was triggered).
// The reason we have to do this at all is because it actually *is*
// programmatically changed by fireEvent.input, so we have to simulate the
// browser's default behavior
const value = currentValue()
const value = currentElement().value

if (value === newValue) {
setSelectionRangeIfNecessary(
Expand All @@ -251,13 +236,12 @@ function setSelectionRange({
}

function fireInputEventIfNeeded({
currentElement,
newValue,
newSelectionStart,
eventOverrides,
currentValue,
currentElement,
}) {
const prevValue = currentValue()
const prevValue = currentElement().value
if (
!currentElement().readOnly &&
!isClickable(currentElement()) &&
Expand All @@ -270,7 +254,6 @@ function fireInputEventIfNeeded({

setSelectionRange({
currentElement,
currentValue,
newValue,
newSelectionStart,
})
Expand All @@ -279,15 +262,10 @@ function fireInputEventIfNeeded({
return {prevValue}
}

function createTypeCharacter(character) {
return context => typeCharacter(character, context)
}

function typeCharacter(
char,
{
currentElement,
currentValue,
prevWasMinus = false,
prevWasPeriod = false,
prevValue = '',
Expand All @@ -313,7 +291,7 @@ function typeCharacter(
...eventOverrides,
})

if (keyPressDefaultNotPrevented) {
if (currentElement().value != null && keyPressDefaultNotPrevented) {
let newEntry = char
if (prevWasMinus) {
newEntry = `-${char}`
Expand All @@ -322,13 +300,12 @@ function typeCharacter(
}

const inputEvent = fireInputEventIfNeeded({
...calculateNewValue(newEntry, currentElement(), currentValue()),
...calculateNewValue(newEntry, currentElement()),
eventOverrides: {
data: key,
inputType: 'insertText',
...eventOverrides,
},
currentValue,
currentElement,
})
prevValue = inputEvent.prevValue
Expand All @@ -340,7 +317,7 @@ function typeCharacter(
// to typing an invalid character (typing "-a3" results in "-3")
// same applies for the decimal character.
if (currentElement().type === 'number') {
const newValue = currentValue()
const newValue = currentElement().value
if (newValue === prevValue && newEntry !== '-') {
nextPrevWasMinus = prevWasMinus
} else {
Expand Down Expand Up @@ -373,8 +350,8 @@ function typeCharacter(
// and you may be tempted to create a shared abstraction.
// If you, brave soul, decide to so endevor, please increment this count
// when you inevitably fail: 1
function calculateNewBackspaceValue(element, value) {
const {selectionStart, selectionEnd} = element
function calculateNewBackspaceValue(element) {
const {selectionStart, selectionEnd, value} = element
let newValue, newSelectionStart

if (selectionStart === null) {
Expand Down Expand Up @@ -406,8 +383,8 @@ function calculateNewBackspaceValue(element, value) {
return {newValue, newSelectionStart}
}

function calculateNewDeleteValue(element, value) {
const {selectionStart, selectionEnd} = element
function calculateNewDeleteValue(element) {
const {selectionStart, selectionEnd, value} = element
let newValue

if (selectionStart === null) {
Expand Down Expand Up @@ -471,7 +448,7 @@ function createModifierCallbackEntries({name, key, keyCode, modifierProperty}) {
}
}

function handleEnter({currentElement, currentValue, eventOverrides}) {
function handleEnter({currentElement, eventOverrides}) {
const key = 'Enter'
const keyCode = 13

Expand Down Expand Up @@ -501,7 +478,6 @@ function handleEnter({currentElement, currentValue, eventOverrides}) {
const {newValue, newSelectionStart} = calculateNewValue(
'\n',
currentElement(),
currentValue(),
)
fireEvent.input(currentElement(), {
target: {value: newValue},
Expand All @@ -510,7 +486,6 @@ function handleEnter({currentElement, currentValue, eventOverrides}) {
})
setSelectionRange({
currentElement,
currentValue,
newValue,
newSelectionStart,
})
Expand Down Expand Up @@ -545,7 +520,7 @@ function handleEsc({currentElement, eventOverrides}) {
})
}

function handleDel({currentElement, currentValue, eventOverrides}) {
function handleDel({currentElement, eventOverrides}) {
const key = 'Delete'
const keyCode = 46

Expand All @@ -558,13 +533,12 @@ function handleDel({currentElement, currentValue, eventOverrides}) {

if (keyPressDefaultNotPrevented) {
fireInputEventIfNeeded({
...calculateNewDeleteValue(currentElement(), currentValue()),
...calculateNewDeleteValue(currentElement()),
eventOverrides: {
inputType: 'deleteContentForward',
...eventOverrides,
},
currentElement,
currentValue,
})
}

Expand All @@ -576,7 +550,7 @@ function handleDel({currentElement, currentValue, eventOverrides}) {
})
}

function handleBackspace({currentElement, currentValue, eventOverrides}) {
function handleBackspace({currentElement, eventOverrides}) {
const key = 'Backspace'
const keyCode = 8

Expand All @@ -589,13 +563,12 @@ function handleBackspace({currentElement, currentValue, eventOverrides}) {

if (keyPressDefaultNotPrevented) {
fireInputEventIfNeeded({
...calculateNewBackspaceValue(currentElement(), currentValue()),
...calculateNewBackspaceValue(currentElement()),
eventOverrides: {
inputType: 'deleteContentBackward',
...eventOverrides,
},
currentElement,
currentValue,
})
}

Expand All @@ -607,10 +580,10 @@ function handleBackspace({currentElement, currentValue, eventOverrides}) {
})
}

function handleSelectall({currentElement, currentValue}) {
function handleSelectall({currentElement}) {
// the user can actually select in several different ways
// we're not going to choose, so we'll *only* set the selection range
currentElement().setSelectionRange(0, currentValue().length)
currentElement().setSelectionRange(0, currentElement().value.length)
}

function handleSpace(context) {
Expand Down Expand Up @@ -654,8 +627,3 @@ function handleSpaceOnClickable({currentElement, eventOverrides}) {
}

export {type}

/*
eslint
no-loop-func: "off",
*/