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.

Sign up for GitHub

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

Jump to bottom

feat(type): add time input support #502

Merged
merged 4 commits into from
Dec 3, 2020
Merged

feat(type): add time input support #502

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
144f142
feat(type): add time input support
gndelia Nov 25, 2020
8333075
docs: improved docs for input time support
gndelia Dec 2, 2020
32b5dd2
refactor: renamed function in type
gndelia Dec 2, 2020
774a030
docs: improved example in time support
gndelia Dec 2, 2020
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
271 changes: 271 additions & 0 deletions src/__tests__/type.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1066,3 +1066,274 @@ test('navigation key: {arrowleft} and {arrowright} moves the cursor', () => {
input[value="abc"] - keyup: c (99)
`)
})

test('can type into an input with type `time`', () => {
const {element, getEventSnapshot} = setup('<input type="time" />')
userEvent.type(element, '0105')
expect(getEventSnapshot()).toMatchInlineSnapshot(`
Events fired on: input[value="01:05"]

input[value=""] - pointerover
input[value=""] - pointerenter
input[value=""] - mouseover: Left (0)
input[value=""] - mouseenter: Left (0)
input[value=""] - pointermove
input[value=""] - mousemove: Left (0)
input[value=""] - pointerdown
input[value=""] - mousedown: Left (0)
input[value=""] - focus
input[value=""] - focusin
input[value=""] - pointerup
input[value=""] - mouseup: Left (0)
input[value=""] - click: Left (0)
input[value=""] - keydown: 0 (48)
input[value=""] - keypress: 0 (48)
input[value=""] - keyup: 0 (48)
input[value=""] - keydown: 1 (49)
input[value=""] - keypress: 1 (49)
input[value=""] - keyup: 1 (49)
input[value=""] - keydown: 0 (48)
input[value=""] - keypress: 0 (48)
input[value="01:00"] - input
"{CURSOR}" -> "{CURSOR}01:00"
input[value="01:00"] - change
input[value="01:00"] - keyup: 0 (48)
input[value="01:00"] - keydown: 5 (53)
input[value="01:00"] - keypress: 5 (53)
input[value="01:05"] - input
"{CURSOR}01:00" -> "{CURSOR}01:05"
input[value="01:05"] - change
input[value="01:05"] - keyup: 5 (53)
`)
expect(element.value).toBe('01:05')
})

test('can type more a number higher than 60 minutes into an input `time` and they are converted into 59 minutes', () => {
const {element, getEventSnapshot} = setup('<input type="time" />')
userEvent.type(element, '2390')
Copy link
Member

Choose a reason for hiding this comment

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

If I type 2390 in the time input here: https://codesandbox.io/s/user-event-playground-eo909?file=/index.html

That doesn't end up firing a change event because I need to type in a (for AM) or p (for PM). It's possible that this is a user-agent thing, in which case I think the implementation here is fine. In any case, we'll want to make sure we document how a time is intended to be entered for a given time value.

I'm thinking that rather than having them provide 0105 we should have them pass the value they want the input.value to be when the typing is complete. So that would be: userEvent.type(element, '01:05'). I think that would be more intuitive. We'd just need to make sure we handle this internally because the end user wouldn't actually bother typing the : I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That doesn't end up firing a change event because I need to type in a (for AM) or p (for PM). It's possible that this is a user-agent thing, in which case I think the implementation here is fine. In any case, we'll want to make sure we document how a time is intended to be entered for a given time value.

In my case trying with chrome I don't have to type a or p. I guess you're right, it depends on the browser or user-agent, or something else.

I'm thinking that rather than having them provide 0105 we should have them pass the value they want the input.value to be when the typing is complete. So that would be: userEvent.type(element, '01:05'). I think that would be more intuitive. We'd just need to make sure we handle this internally because the end user wouldn't actually bother typing the : I think.

technically speaking, all the non-digit characters are ignored, as that's what happens when using the <input type="time" />. So I could rewrite the tests to use : and they will work with the current implementation (and the docs could be with that). What do you think?

if that's enough as the request, I could rewrite the tests and the docs (yet to be added) to show the examples using :. That's probably better in terms of DX, but as you say the user would never type :. Working without : is an extra in that sense

If you think it is better to change the implementation to just split hours and minutes by :, then I can do it; in this scenario, the user would always have to type : or it wouldn't work (with the current approach, both works). and I'd have to validate both separatedly

(I also went without : probably biased by the example in the ticket, which was not using it).

Copy link
Member

Choose a reason for hiding this comment

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

That all sounds great. Thank you. I think having the : in the docs would be preferable/sensible. And having at least one test that uses : as well as at least one that does not would be good.

Thanks for all of this work!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changes applied in 8333075

  • updated docs to have an example of using <input type="time />. My main doubt is that there are no roles associated to this type of input, so the only way to query it was with a data-testid. Should I use another option?

  • I updated all tests to use 01:05, and also added one without the : to verify it works in both scenarios

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

to clarify, I tried with labels in testing-playground but it did not work either - that's why I went for data-testid

Copy link
Member

Choose a reason for hiding this comment

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

How about this? https://testing-playground.com/gist/7b5d922642a0a1814209c81db6867b59/1b0329837c8afa1d5e972e219ed65ba041f69c82

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I swear I tried it and I was getting an error 🤔 but 🤷
updated in 774a030


expect(getEventSnapshot()).toMatchInlineSnapshot(`
Events fired on: input[value="23:59"]

input[value=""] - pointerover
input[value=""] - pointerenter
input[value=""] - mouseover: Left (0)
input[value=""] - mouseenter: Left (0)
input[value=""] - pointermove
input[value=""] - mousemove: Left (0)
input[value=""] - pointerdown
input[value=""] - mousedown: Left (0)
input[value=""] - focus
input[value=""] - focusin
input[value=""] - pointerup
input[value=""] - mouseup: Left (0)
input[value=""] - click: Left (0)
input[value=""] - keydown: 2 (50)
input[value=""] - keypress: 2 (50)
input[value=""] - keyup: 2 (50)
input[value=""] - keydown: 3 (51)
input[value=""] - keypress: 3 (51)
input[value=""] - keyup: 3 (51)
input[value=""] - keydown: 9 (57)
input[value=""] - keypress: 9 (57)
input[value="23:09"] - input
"{CURSOR}" -> "{CURSOR}23:09"
input[value="23:09"] - change
input[value="23:09"] - keyup: 9 (57)
input[value="23:09"] - keydown: 0 (48)
input[value="23:09"] - keypress: 0 (48)
input[value="23:59"] - input
"{CURSOR}23:09" -> "{CURSOR}23:59"
input[value="23:59"] - change
input[value="23:59"] - keyup: 0 (48)
`)

expect(element.value).toBe('23:59')
})

test('can type letters into an input `time` and they are ignored', () => {
const {element, getEventSnapshot} = setup('<input type="time" />')
userEvent.type(element, '1a6bc36abd')

expect(getEventSnapshot()).toMatchInlineSnapshot(`
Events fired on: input[value="16:36"]

input[value=""] - pointerover
input[value=""] - pointerenter
input[value=""] - mouseover: Left (0)
input[value=""] - mouseenter: Left (0)
input[value=""] - pointermove
input[value=""] - mousemove: Left (0)
input[value=""] - pointerdown
input[value=""] - mousedown: Left (0)
input[value=""] - focus
input[value=""] - focusin
input[value=""] - pointerup
input[value=""] - mouseup: Left (0)
input[value=""] - click: Left (0)
input[value=""] - keydown: 1 (49)
input[value=""] - keypress: 1 (49)
input[value=""] - keyup: 1 (49)
input[value=""] - keydown: a (97)
input[value=""] - keypress: a (97)
input[value=""] - keyup: a (97)
input[value=""] - keydown: 6 (54)
input[value=""] - keypress: 6 (54)
input[value=""] - keyup: 6 (54)
input[value=""] - keydown: b (98)
input[value=""] - keypress: b (98)
input[value=""] - keyup: b (98)
input[value=""] - keydown: c (99)
input[value=""] - keypress: c (99)
input[value=""] - keyup: c (99)
input[value=""] - keydown: 3 (51)
input[value=""] - keypress: 3 (51)
input[value="16:03"] - input
"{CURSOR}" -> "{CURSOR}16:03"
input[value="16:03"] - change
input[value="16:03"] - keyup: 3 (51)
input[value="16:03"] - keydown: 6 (54)
input[value="16:03"] - keypress: 6 (54)
input[value="16:36"] - input
"{CURSOR}16:03" -> "{CURSOR}16:36"
input[value="16:36"] - change
input[value="16:36"] - keyup: 6 (54)
input[value="16:36"] - keydown: a (97)
input[value="16:36"] - keypress: a (97)
input[value="16:36"] - keyup: a (97)
input[value="16:36"] - keydown: b (98)
input[value="16:36"] - keypress: b (98)
input[value="16:36"] - keyup: b (98)
input[value="16:36"] - keydown: d (100)
input[value="16:36"] - keypress: d (100)
input[value="16:36"] - keyup: d (100)
`)

expect(element.value).toBe('16:36')
})

test('can type a digit bigger in the hours section, bigger than 2 and it shows the time correctly', () => {
const {element, getEventSnapshot} = setup('<input type="time" />')
userEvent.type(element, '925')

expect(getEventSnapshot()).toMatchInlineSnapshot(`
Events fired on: input[value="09:25"]

input[value=""] - pointerover
input[value=""] - pointerenter
input[value=""] - mouseover: Left (0)
input[value=""] - mouseenter: Left (0)
input[value=""] - pointermove
input[value=""] - mousemove: Left (0)
input[value=""] - pointerdown
input[value=""] - mousedown: Left (0)
input[value=""] - focus
input[value=""] - focusin
input[value=""] - pointerup
input[value=""] - mouseup: Left (0)
input[value=""] - click: Left (0)
input[value=""] - keydown: 9 (57)
input[value=""] - keypress: 9 (57)
input[value=""] - keyup: 9 (57)
input[value=""] - keydown: 2 (50)
input[value=""] - keypress: 2 (50)
input[value="09:02"] - input
"{CURSOR}" -> "{CURSOR}09:02"
input[value="09:02"] - change
input[value="09:02"] - keyup: 2 (50)
input[value="09:02"] - keydown: 5 (53)
input[value="09:02"] - keypress: 5 (53)
input[value="09:25"] - input
"{CURSOR}09:02" -> "{CURSOR}09:25"
input[value="09:25"] - change
input[value="09:25"] - keyup: 5 (53)
`)

expect(element.value).toBe('09:25')
})

test('can type two digits in the hours section, equals to 24 and it shows the hours as 23', () => {
const {element, getEventSnapshot} = setup('<input type="time" />')
userEvent.type(element, '2452')

expect(getEventSnapshot()).toMatchInlineSnapshot(`
Events fired on: input[value="23:52"]

input[value=""] - pointerover
input[value=""] - pointerenter
input[value=""] - mouseover: Left (0)
input[value=""] - mouseenter: Left (0)
input[value=""] - pointermove
input[value=""] - mousemove: Left (0)
input[value=""] - pointerdown
input[value=""] - mousedown: Left (0)
input[value=""] - focus
input[value=""] - focusin
input[value=""] - pointerup
input[value=""] - mouseup: Left (0)
input[value=""] - click: Left (0)
input[value=""] - keydown: 2 (50)
input[value=""] - keypress: 2 (50)
input[value=""] - keyup: 2 (50)
input[value=""] - keydown: 4 (52)
input[value=""] - keypress: 4 (52)
input[value=""] - keyup: 4 (52)
input[value=""] - keydown: 5 (53)
input[value=""] - keypress: 5 (53)
input[value="23:05"] - input
"{CURSOR}" -> "{CURSOR}23:05"
input[value="23:05"] - change
input[value="23:05"] - keyup: 5 (53)
input[value="23:05"] - keydown: 2 (50)
input[value="23:05"] - keypress: 2 (50)
input[value="23:52"] - input
"{CURSOR}23:05" -> "{CURSOR}23:52"
input[value="23:52"] - change
input[value="23:52"] - keyup: 2 (50)
`)

expect(element.value).toBe('23:52')
})

test('can type two digits in the hours section, bigger than 24 and less than 30, and it shows the hours as 23', () => {
const {element, getEventSnapshot} = setup('<input type="time" />')
userEvent.type(element, '2752')

expect(getEventSnapshot()).toMatchInlineSnapshot(`
Events fired on: input[value="23:52"]

input[value=""] - pointerover
input[value=""] - pointerenter
input[value=""] - mouseover: Left (0)
input[value=""] - mouseenter: Left (0)
input[value=""] - pointermove
input[value=""] - mousemove: Left (0)
input[value=""] - pointerdown
input[value=""] - mousedown: Left (0)
input[value=""] - focus
input[value=""] - focusin
input[value=""] - pointerup
input[value=""] - mouseup: Left (0)
input[value=""] - click: Left (0)
input[value=""] - keydown: 2 (50)
input[value=""] - keypress: 2 (50)
input[value=""] - keyup: 2 (50)
input[value=""] - keydown: 7 (55)
input[value=""] - keypress: 7 (55)
input[value=""] - keyup: 7 (55)
input[value=""] - keydown: 5 (53)
input[value=""] - keypress: 5 (53)
input[value="23:05"] - input
"{CURSOR}" -> "{CURSOR}23:05"
input[value="23:05"] - change
input[value="23:05"] - keyup: 5 (53)
input[value="23:05"] - keydown: 2 (50)
input[value="23:05"] - keypress: 2 (50)
input[value="23:52"] - input
"{CURSOR}23:05" -> "{CURSOR}23:52"
input[value="23:52"] - change
input[value="23:52"] - keyup: 2 (50)
`)

expect(element.value).toBe('23:52')
})
22 changes: 22 additions & 0 deletions src/type.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import {
getSelectionRange,
getValue,
isContentEditable,
isValidInputTimevalue,
gndelia marked this conversation as resolved.
Show resolved Hide resolved
buildTimeValue,
} from './utils'
import {click} from './click'
import {navigationKey} from './keys/navigation-key'
Expand Down Expand Up @@ -324,6 +326,11 @@ function typeCharacter(
newEntry = textToBeTyped
}

const timeNewEntry = buildTimeValue(textToBeTyped)
if (isValidInputTimevalue(currentElement(), timeNewEntry)) {
gndelia marked this conversation as resolved.
Show resolved Hide resolved
newEntry = timeNewEntry
}

const inputEvent = fireInputEventIfNeeded({
...calculateNewValue(newEntry, currentElement()),
eventOverrides: {
Expand All @@ -339,6 +346,8 @@ function typeCharacter(
fireEvent.change(currentElement(), {target: {value: textToBeTyped}})
}

fireChangeForInputTimeIfValid(currentElement, prevValue, timeNewEntry)

// typing "-" into a number input will not actually update the value
// so for the next character we type, the value should be set to
// `-${newEntry}`
Expand Down Expand Up @@ -376,6 +385,19 @@ function typeCharacter(
}
}

function fireChangeForInputTimeIfValid(
currentElement,
prevValue,
timeNewEntry,
) {
if (
isValidInputTimevalue(currentElement(), timeNewEntry) &&
prevValue !== timeNewEntry
) {
fireEvent.change(currentElement(), {target: {value: timeNewEntry}})
}
}

gndelia marked this conversation as resolved.
Show resolved Hide resolved
// yes, calculateNewBackspaceValue and calculateNewValue look extremely similar
// and you may be tempted to create a shared abstraction.
// If you, brave soul, decide to so endevor, please increment this count
Expand Down
51 changes: 51 additions & 0 deletions src/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,14 @@ function calculateNewValue(newEntry, element) {
newValue = value
}

if (element.type === 'time' && !isValidInputTimevalue(element, newValue)) {
if (isValidInputTimevalue(element, newEntry)) {
newValue = newEntry
} else {
newValue = value
}
}

if (!supportsMaxLength(element) || maxLength < 0) {
return {newValue, newSelectionStart}
} else {
Expand Down Expand Up @@ -269,6 +277,47 @@ function isValidDateValue(element, value) {
return clone.value === value
}

function buildTimeValue(value) {
function build(onlyDigitsValue, index) {
const hours = onlyDigitsValue.slice(0, index)
const validHours = Math.min(parseInt(hours, 10), 23)
const minuteCharacters = onlyDigitsValue.slice(index)
const parsedMinutes = parseInt(minuteCharacters, 10)
const validMinutes = Math.min(parsedMinutes, 59)
return `${validHours
.toString()
.padStart(2, '0')}:${validMinutes.toString().padStart(2, '0')}`
}

const onlyDigitsValue = value.replace(/\D/g, '')
if (onlyDigitsValue.length < 2) {
return value
}
const firstDigit = parseInt(onlyDigitsValue[0], 10)
const secondDigit = parseInt(onlyDigitsValue[1], 10)
if (firstDigit >= 3 || (firstDigit === 2 && secondDigit >= 4)) {
let index
if (firstDigit >= 3) {
index = 1
} else {
index = 2
}
return build(onlyDigitsValue, index)
}
if (value.length === 2) {
return value
}
return build(onlyDigitsValue, 2)
}

function isValidInputTimevalue(element, timeValue) {
gndelia marked this conversation as resolved.
Show resolved Hide resolved
if (element.type !== 'time') return false

const clone = element.cloneNode()
clone.value = timeValue
return clone.value === timeValue
}

export {
FOCUSABLE_SELECTOR,
isFocusable,
Expand All @@ -280,6 +329,8 @@ export {
setSelectionRangeIfNecessary,
eventWrapper,
isValidDateValue,
isValidInputTimevalue,
gndelia marked this conversation as resolved.
Show resolved Hide resolved
buildTimeValue,
getValue,
getSelectionRange,
isContentEditable,
Expand Down