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: add normalizer for matching aria-label #855

Merged
merged 1 commit into from
Jan 7, 2021
Merged

fix: add normalizer for matching aria-label #855

merged 1 commit into from
Jan 7, 2021

Conversation

johnjesse
Copy link
Contributor

@johnjesse johnjesse commented Dec 21, 2020

What:
fixed the label matcher - so that it passes on the normalizer when matching the aria-label, added a test too
see #854

Why:
because the normalizer wasn't being correctly passed on

How:
see - src/queries/label-text.js

Checklist:

  • Documentation added to the
    docs site N/A
  • Tests
  • Typescript definitions updated N/A
  • Ready to be merged
  • I did opt in to run git hooks - but had some issues with the validate script - it just seemed to error and not output anything useful so I ended up bypassing it.
  • Also - all the tests in src/tests/text-matchers.js were named after a method. Because ...byLabelText can get the label in multiple ways - and I wanted a specific test for the aria-label case - I've added a case that isn't named after a specific method

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 21, 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 d8bb655:

Sandbox Source
react-testing-library-examples Configuration

@nickserv nickserv linked an issue Dec 22, 2020 that may be closed by this pull request
@johnjesse
Copy link
Contributor Author

The failing checks appear to be either
error TS2345: Argument of type 'number' is not assignable to parameter of type 'Matcher'. - which I don't think is down to my changes

OR

@testing-library/dom@0.0.0-semantically-released validate: `kcd-scripts validate - again I don't know what I may have done to cause that

@marcosvega91
Copy link
Member

Hi @johnjesse, I have fixed this issue in #850. The problem is that the typescript configuration is wrong because is using types from node_modules instead of types from types folder.

@codecov
Copy link

codecov bot commented Jan 4, 2021

Codecov Report

Merging #855 (d8bb655) into master (ec1b642) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #855   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           26        26           
  Lines          945       945           
  Branches       290       290           
=========================================
  Hits           945       945           
Impacted Files Coverage Δ
src/queries/label-text.js 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 ec1b642...d8bb655. Read the comment docs.

@johnjesse
Copy link
Contributor Author

Happy new year 😄, is there anyone available to review this pr?

Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

Thanks 👍

@eps1lon eps1lon merged commit 5831f60 into testing-library:master Jan 7, 2021
@eps1lon
Copy link
Member

eps1lon commented Jan 7, 2021

@all-contributors add @johnjesse for bug and code.

@allcontributors
Copy link
Contributor

@eps1lon

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

@github-actions
Copy link

github-actions bot commented Jan 7, 2021

🎉 This PR is included in version 7.29.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

Normalizer is missing when querying by aria-label
3 participants