feat(type): add time input support #502

gndelia · 2020-11-25T22:01:42Z

What: Adding support for <input type="time" />. Check #484

Why: That type of input does not work. It does with fireEvent, though

How: Adding the functionality for it. I had to interpret the typed characters to hours and minutes, adding the : between both.

Checklist:

Documentation
Tests
Typings N/A
Ready to be merged

codecov · 2020-11-25T22:03:02Z

Codecov Report

Merging #502 (774a030) into master (f7620ab) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #502   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           13        13           
  Lines          615       664   +49     
  Branches       200       211   +11     
=========================================
+ Hits           615       664   +49

Impacted Files	Coverage Δ
src/type.js	`100.00% <100.00%> (ø)`
src/utils.js	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f7620ab...774a030. Read the comment docs.

kentcdodds

Thanks for your work on this! Just one thing.

Also, let's make sure we add docs :)

kentcdodds · 2020-11-26T06:43:24Z

src/__tests__/type.js

+
+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')


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.

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).

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!

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

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

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

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

kentcdodds

This is super. Just a few small things and then we can get this merged. Thanks!

src/type.js

src/utils.js

src/type.js

kentcdodds

Well done 👏

kentcdodds · 2020-12-03T01:43:08Z

@all-contributors please add @gndelia for code, tests, and docs

allcontributors · 2020-12-03T01:43:18Z

@kentcdodds

I've put up a pull request to add @gndelia! 🎉

kentcdodds · 2020-12-03T01:44:10Z

Thanks so much for your help! I've added you as a collaborator on the project. Please make sure that you review the other/MAINTAINING.md and CONTRIBUTING.md files (specifically the bit about the commit messages and the git hooks) and familiarize yourself with the code of conduct (we're using the contributor covenant). You might also want to watch the repo to be notified when someone files an issue/PR. Please continue to make PRs as you feel the need (you can make your branches directly on the repo rather than your fork if you want). Thanks! And welcome to the team :)

github-actions · 2020-12-03T01:46:37Z

🎉 This PR is included in version 12.4.0 🎉

The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot 📦🚀

feat(type): add time input support

144f142

kentcdodds reviewed Nov 26, 2020

View reviewed changes

docs: improved docs for input time support

8333075

gndelia requested a review from kentcdodds December 2, 2020 22:19

kentcdodds reviewed Dec 2, 2020

View reviewed changes

src/type.js Outdated Show resolved Hide resolved

src/utils.js Outdated Show resolved Hide resolved

src/utils.js Outdated Show resolved Hide resolved

src/type.js Outdated Show resolved Hide resolved

src/type.js Outdated Show resolved Hide resolved

refactor: renamed function in type

32b5dd2

gndelia requested a review from kentcdodds December 2, 2020 23:24

docs: improved example in time support

774a030

kentcdodds approved these changes Dec 3, 2020

View reviewed changes

kentcdodds merged commit 0d30a79 into testing-library:master Dec 3, 2020

allcontributors bot mentioned this pull request Dec 3, 2020

docs: add gndelia as a contributor #514

Merged

github-actions bot added the released label Dec 3, 2020

nickmccurdy linked an issue Dec 3, 2020 that may be closed by this pull request

userEvent.type not working with <input type="time"> #484

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(type): add time input support #502

feat(type): add time input support #502

gndelia commented Nov 25, 2020 •

edited

codecov bot commented Nov 25, 2020 •

edited

kentcdodds left a comment

kentcdodds Nov 26, 2020

gndelia Nov 26, 2020

kentcdodds Nov 27, 2020

gndelia Dec 2, 2020

gndelia Dec 2, 2020

kentcdodds Dec 2, 2020

gndelia Dec 2, 2020

kentcdodds left a comment

kentcdodds left a comment

kentcdodds commented Dec 3, 2020

allcontributors bot commented Dec 3, 2020

kentcdodds commented Dec 3, 2020

github-actions bot commented Dec 3, 2020

feat(type): add time input support #502

feat(type): add time input support #502

Conversation

gndelia commented Nov 25, 2020 • edited

codecov bot commented Nov 25, 2020 • edited

Codecov Report

kentcdodds left a comment

Choose a reason for hiding this comment

kentcdodds Nov 26, 2020

Choose a reason for hiding this comment

gndelia Nov 26, 2020

Choose a reason for hiding this comment

kentcdodds Nov 27, 2020

Choose a reason for hiding this comment

gndelia Dec 2, 2020

Choose a reason for hiding this comment

gndelia Dec 2, 2020

Choose a reason for hiding this comment

kentcdodds Dec 2, 2020

Choose a reason for hiding this comment

gndelia Dec 2, 2020

Choose a reason for hiding this comment

kentcdodds left a comment

Choose a reason for hiding this comment

kentcdodds left a comment

Choose a reason for hiding this comment

kentcdodds commented Dec 3, 2020

allcontributors bot commented Dec 3, 2020

kentcdodds commented Dec 3, 2020

github-actions bot commented Dec 3, 2020

gndelia commented Nov 25, 2020 •

edited

codecov bot commented Nov 25, 2020 •

edited