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: Don't recommend ByRole if role is generic #664

Merged
merged 2 commits into from
Jun 23, 2020

Conversation

marcosvega91
Copy link
Member

fix #663
What:

I have fixed all the tests that failed

Why:

Because after the update of aria-query something didn't work

How:

I have made two changes in this PR:

  1. With the new aria specifications input without a role has a textbox as default one.
    I' have replaced all the input used in tests that not require a role with input of type=password that doesn't have a
    default role @benmonro

  2. I have skipped generic role because we prefer to suggest others query
    we prefer screen.getByText(/bob/i) instead of screen.getByRole('generic', {name: /bob/i})

Checklist:

  • Documentation added to the
    docs site
  • Tests
  • Typescript definitions updated
  • Ready to be merged

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 23, 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 0120a91:

Sandbox Source
objective-nash-m7e6u Configuration

@codecov
Copy link

codecov bot commented Jun 23, 2020

Codecov Report

Merging #664 into master will increase coverage by 0.34%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #664      +/-   ##
===========================================
+ Coverage   99.65%   100.00%   +0.34%     
===========================================
  Files          24        24              
  Lines         582       583       +1     
  Branches      149       149              
===========================================
+ Hits          580       583       +3     
+ Misses          2         0       -2     
Impacted Files Coverage Δ
src/role-helpers.js 100.00% <100.00%> (ø)
src/suggestions.js 100.00% <100.00%> (+2.04%) ⬆️
src/queries/role.js 100.00% <0.00%> (+1.85%) ⬆️

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 a114f4f...0120a91. Read the comment docs.

@eps1lon eps1lon changed the title fix: build failing fix: Don't recommend ByRole if role is generic Jun 23, 2020
@@ -74,7 +74,9 @@ test(`should not suggest if the suggestion would give different results`, () =>
})

test('should suggest by label over title', () => {
renderIntoDocument(`<label><span>bar</span><input title="foo" /></label>`)
renderIntoDocument(
`<label><span>bar</span><input type="password" title="foo" /></label>`,
Copy link
Member

Choose a reason for hiding this comment

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

To be accurate: The spec related to aria didn't change. This was a bug in earlier versions of aria-query. <input /> with no or an invalid type attribute is always considered <input type="text" /> which is a role="textbox".

@eps1lon eps1lon merged commit a2a3212 into testing-library:master Jun 23, 2020
@kentcdodds
Copy link
Member

🎉 This PR is included in version 7.16.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@marcosvega91 marcosvega91 deleted the pr/fix_build_failing branch June 23, 2020 08:17
@kentcdodds
Copy link
Member

@marcosvega91, and all reviewers... You are my heros!

I really didn't have time to work on this. Thank you!

@kentcdodds
Copy link
Member

🎉 This issue has been resolved in version 7.17.0 🎉

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.

Fix default branch CI build
4 participants