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: Treat header as banner role #656

Merged
merged 1 commit into from
Jun 20, 2020

Conversation

darekkay
Copy link
Contributor

What:

<header> is not recognized as banner role, and cannot be selected with getByRole("banner").

See also: #578

Why:

It should work correctly according to the specs.

How:

Update aria-query dependency. The latest aria-query version fixes the issue of failing to recognize <header> as a banner role.

Checklist:

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

Fixes: #578

The latest [aria-query version](https://github.com/A11yance/aria-query/releases/tag/v4.2.2) fixes the issue of failing to recognize `<header>` as a `banner` role.

Fixes: testing-library#578
@codesandbox-ci
Copy link

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 c6e1bb9:

Sandbox Source
vigilant-resonance-6j64y Configuration

@codecov
Copy link

codecov bot commented Jun 20, 2020

Codecov Report

Merging #656 into master will decrease coverage by 0.34%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##            master     #656      +/-   ##
===========================================
- Coverage   100.00%   99.65%   -0.35%     
===========================================
  Files           24       24              
  Lines          582      582              
  Branches       149      149              
===========================================
- Hits           582      580       -2     
- Misses           0        2       +2     
Impacted Files Coverage Δ
src/suggestions.js 97.95% <0.00%> (-2.05%) ⬇️
src/queries/role.js 98.14% <0.00%> (-1.86%) ⬇️

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 55344ea...c6e1bb9. 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.

Sweet! Thank you 👍👍

@kentcdodds kentcdodds merged commit 79142fb into testing-library:master Jun 20, 2020
@kentcdodds
Copy link
Member

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

@allcontributors
Copy link
Contributor

@kentcdodds

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

@darekkay darekkay deleted the fix/header-banner-role branch June 22, 2020 18:57
@kentcdodds
Copy link
Member

🎉 This PR is included in version 7.16.3 🎉

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.

getByRole() doesn't recognize header's implicit banner role
2 participants