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

Conversation

visualjerk
Copy link
Collaborator

What:

Closes #407

Why:

Because typing on focusable elements should be possible without getting an error.

How:

Checked if an element has a value prop before handling input and selection on it.

Checklist:

  • Documentation (not needed)
  • Tests
  • Typings (not needed)
  • Ready to be merged

@codecov
Copy link

codecov bot commented Jul 24, 2020

Codecov Report

Merging #414 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #414   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           12        12           
  Lines          418       413    -5     
  Branches       116       116           
=========================================
- Hits           418       413    -5     
Impacted Files Coverage Δ
src/paste.js 100.00% <100.00%> (ø)
src/type.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 52c333c...0cfbfbc. Read the comment docs.

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Looks great to me! Just one question.

src/__tests__/type-modifiers.js Show resolved Hide resolved
@marcosvega91
Copy link
Member

I think that we are missing one thing. We should enable modifiers on div only if it is focusable ( only if has a tabindex)

@kentcdodds
Copy link
Member

Good point. In fact, I think most events should not be fired on elements that aren't focusable. That said, I don't think the events will be fired on non-focusable elements anyway because we don't fire them on the given element, we fire them on the element.ownerDocument.activeElement, which could never be anything but a focusable element.

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Solid. Thank you!

@kentcdodds kentcdodds merged commit e1423ed into testing-library:master Jul 24, 2020
@kentcdodds
Copy link
Member

🎉 This PR is included in version 12.0.14 🎉

The release is available on:

Your semantic-release bot 📦🚀

@visualjerk visualjerk deleted the fix-407-type_on_no_value_elements branch July 24, 2020 22:11
jesujcastillom pushed a commit to jesujcastillom/user-event that referenced this pull request Oct 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when using type on an element without a value property
3 participants