Skip to content

getByRole() doesn't recognize header's implicit banner role #578

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.

Sign up for GitHub

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

Jump to bottom
Closed
darekkay opened this issue May 16, 2020 · 5 comments · Fixed by #656
Closed

getByRole() doesn't recognize header's implicit banner role #578

darekkay opened this issue May 16, 2020 · 5 comments · Fixed by #656
Labels
released

Comments

@darekkay
Copy link
Contributor

  • @testing-library/dom version: 7.5.1
  • @testing-library/jest-dom: 5.7.0
  • @testing-library/react: 10.0.4
  • jest: 24.9.0

Relevant code or config:

import React from "react";
import { render, screen } from "@testing-library/react";

describe("getByRole", () => {
  test("doens't consider a <header> as banner role", () => {
    render(<header>Test</header>);
    expect(screen.getByRole("banner")).toBeInTheDocument();
  });
});

What you did:

The header tag has an implicit banner role. RTL's getByRole should be able to identify this.

What happened:

TestingLibraryElementError: Unable to find an accessible element with the role "banner"

There are no accessible roles. But there might be some inaccessible roles. If you wish to access them, then set the `hidden` option to `true`. Learn more about this here: https://testing-library.com/docs/dom-testing-library/api-queries#byrole

<body>
  <div>
    <header>
      Test
    </header>
  </div>
</body>

Problem description:

From the official W3C specs:

  1. The HTML header element defines a banner landmark when its context is the body element.
  2. The HTML header element is not considered a banner landmark when it is descendant of any of following elements:
  • article
  • aside
  • main
  • nav
  • section

Chrome gets this one right:

image

Suggested solution:

RTL should be able to identify the role correctly with respect to the above banner specs for the header tag.
@darekkay darekkay changed the title getByRole() doesn't recognize header's implicit banner rolle getByRole() doesn't recognize header's implicit banner role May 16, 2020
@OCmilo
Copy link

OCmilo commented May 25, 2020

Just complementing:
In the documentation it states:

Testing Library uses aria-query under the hood to find those elements by their default ARIA roles, you can find in their docs which HTML Elements with inherent roles are mapped to each role.

If you follow the link, you'll see that aria-query doesn't actually support any landmark roles, which seems to be the issue.

@darekkay darekkay mentioned this issue May 25, 2020
Closed
@darekkay
Copy link
Contributor Author

darekkay commented May 25, 2020 •
edited
Loading

Thanks for the information, I've created an issue in the aria-query project. It looks as if there is a huge PR being verified that brings landmark support.

@OCmilo
Copy link

OCmilo commented May 25, 2020

Nice find! I came here for the same problem, so I'm hoping this happens soon.

@darekkay
Copy link
Contributor Author

darekkay commented Jun 19, 2020 •
edited
Loading

I've updated aria-query to the latest version, but the issue still occurs, unfortunately. I've provided a PR.

darekkay added a commit to darekkay/dom-testing-library that referenced this issue Jun 20, 2020
@darekkay
fix: Treat header as banner role
c6e1bb9 
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
@darekkay darekkay mentioned this issue Jun 20, 2020
Merged
4 tasks
kentcdodds pushed a commit that referenced this issue Jun 20, 2020
@darekkay
fix: Treat header as banner role (#656)
79142fb 
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: #578
@kentcdodds
Copy link
Member

🎉 This issue has been resolved in version 7.16.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Ulisseus Ulisseus mentioned this issue Jan 22, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: Treat header as banner role darekkay/dom-testing-library
3 participants
@kentcdodds @darekkay @OCmilo