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: Only return first child element in label query #520

Merged
merged 2 commits into from
Apr 6, 2020

Conversation

airjp73
Copy link
Contributor

@airjp73 airjp73 commented Apr 6, 2020

What:

Bug-Fix: Changes the *ByLabelText queries so that, when finding form controls that are children of labels, only the first form control is returned.

Why:

Following up on the discussion in #373 that started with this comment

How:

When finding children of labels, I filtered the children down to only those that matched the selector and grabbed the first one.

Checklist:

  • Documentation added to the
    docs site N/A
  • I've prepared a PR for types targeting
    DefinitelyTyped N/A
  • Tests
  • Ready to be merged

@codesandbox-ci
Copy link

codesandbox-ci bot commented Apr 6, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 7cf5496:

Sandbox Source
vigilant-sara-6b7sf Configuration

kentcdodds
kentcdodds previously approved these changes Apr 6, 2020
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.

I think this looks good. I doubt this will impact many people, but I would like to get another review from folks on this before it's merged.

@kentcdodds
Copy link
Member

Well that's new:

image

[test]  FAIL   node  src/__node_tests__/index.js
[test]   ● Test suite failed to run
[test] 
[test]     ReferenceError: BigInt is not defined
[test] 
[test]       at createLongLongConversion (node_modules/webidl-conversions/lib/index.js:162:51)
[test]       at Object.<anonymous> (node_modules/webidl-conversions/lib/index.js:224:24)
[test]       at Object.<anonymous> (node_modules/jsdom/lib/jsdom/browser/Window.js:3:27)
[test] 

That's the build failure.

I'm going to open a new issue about this.

@codecov
Copy link

codecov bot commented Apr 6, 2020

Codecov Report

Merging #520 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #520   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          23     23           
  Lines         428    429    +1     
  Branches      102    103    +1     
=====================================
+ Hits          428    429    +1
Impacted Files Coverage Δ
src/queries/label-text.js 100% <100%> (ø) ⬆️

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 672f231...7cf5496. Read the comment docs.

@kentcdodds kentcdodds merged commit 402288f into testing-library:master Apr 6, 2020
@kentcdodds
Copy link
Member

We won't be able to get this released until #521 is worked out 😬

@kentcdodds
Copy link
Member

(Oh, and thanks a lot @airjp73!)

@kentcdodds
Copy link
Member

🎉 This PR is included in version 7.2.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@kentcdodds
Copy link
Member

Huh, maybe that was a fluke for the build 🤔 😬

@airjp73
Copy link
Contributor Author

airjp73 commented Apr 6, 2020

Any time!

@airjp73
Copy link
Contributor Author

airjp73 commented Apr 6, 2020

Also, I hope it's not rude to ask 🙃-- could I be added to the all-contributors thing? 😊

@kentcdodds
Copy link
Member

Thank you for asking! I'm normally good at doing that but do miss it occasionally. Will get you added now!

@kentcdodds
Copy link
Member

@all-contributors please add @airjp73 for code and tests

@allcontributors
Copy link
Contributor

@kentcdodds

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

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.

3 participants