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

*ByRole reads hidden element names as "" #846

Open
tpict opened this issue Dec 10, 2020 · 7 comments · May be fixed by #1161
Open

*ByRole reads hidden element names as "" #846

tpict opened this issue Dec 10, 2020 · 7 comments · May be fixed by #1161

Comments

@tpict
Copy link

tpict commented Dec 10, 2020

  • @testing-library/dom version: 7.28.1
  • Testing Framework and version: Jest 26.6.3
  • DOM Environment: jsdom 16.4.0

Relevant code or config:

import { render } from "@testing-library/react";
import { getByRole } from "@testing-library/dom";

const { container } = render(
  <>
    <input aria-label="Input" style={{ display: "none" }} />
    <button aria-label="Button" style={{ display: "none" }} />
  </>,
);

expect(
  getByRole(container, "textbox", { name: "Input", hidden: true }),
).toBeInTheDocument();
expect(
  getByRole(container, "button", { name: "Button", hidden: true }),
).toBeInTheDocument();

What you did:

Attempted to test the presence of accessible elements with display: none;

What happened:

The Testing Library error reporter claims the name of these elements is "" and fails to match them against the provided name:

    TestingLibraryElementError: Unable to find an element with the role "textbox" and name "Input"

    Here are the available roles:

      textbox:

      Name "":
      <input
        aria-label="Input"
        style="display: none;"
      />

      --------------------------------------------------button:

      Name "":
      <button
        aria-label="Button"
        style="display: none;"
      />

      --------------------------------------------------

Reproduction:

Hopefully the provided code is sufficient, let me know if not

Problem description:

The example I've given is a little simplified–the real code I'm testing contains elements that are visible in desktop-width viewports, and if I understand correctly media query parsing is out-of-scope for Testing Library–but the issue is the same: these elements will become part of the accessibility tree under certain circumstances and I'm ignoring their invisibility in order to check that they have the correct accessible names. I would expect that I'd be able to do that with the hidden: true option set.

Suggested solution:

I don't have the time to spare to figure out the cause of this right now :( it might be related to recent changes around #804 ?

@eps1lon
Copy link
Member

eps1lon commented Dec 10, 2020

I lean towards leaving this as undefined behavior. Since the accessible name is affected by styling, ignoring it leaves you in an awkward spot. It makes sense in this specific case but the code is going to be really hairy getting all the edge cases right if the element isn't actually part of the a11y tree. For example what happens if you use aria-labelledby, pass hidden: true but the referenced element by aria-labelledby is also hidden?

these elements will become part of the accessibility tree under certain circumstances

I would recommend testing under these circumstances. Then you also make sure that your assumptions are actually correct.

@tpict
Copy link
Author

tpict commented Dec 10, 2020

That would be my preference too, but I don't think I can make that happen without moving our whole test infrastructure to Cypress (or something else–jsdom appears to ignore media queries). The only work-around within Testing Library I'm aware of is to switch to a less semantically meaningful query which feels like a lot to sacrifice just because our mobile layout has changed.

I do see your point about the room for edge cases here. Maybe the best fix for my scenario is to patch getComputedStyles to apply the display property from the screen width I'm attempting to test directly.

@Rolfchen
Copy link

Rolfchen commented Dec 22, 2020

I'm having similar issues, but in my case it was aria-hidden: true elements displaying with Name: "".

Example:

//  pseudo code of the component to test

const Carousel = () => {
   return (
      <ul>
          <li aria-hidden="false">Slide 1</li>
          <li aria-hidden="true">Slide 2</li>
      </ul>
   )
}
// sample test

render(<Carousel />);
expect(screen.getAllByRole('listitem', { name: /Slide/, hidden: true }).toHaveLength(3);

This test will fail with similar message as the OP.

I "think" it was because the isHidden function in computeTextAlternative is true due to the aria-hidden, and therefore returning "".

I noticed that it also checks for !context.isReferenced, but not sure how that's being passed in via ComputeTextAlternativeOptions in computeAccessibleName. I wonder if that's something testing library's query can use (Or i suppose a different parameter) to indicate hidden: true has been passed in, and therefore ignore isHidden?

@rosczja
Copy link

rosczja commented Apr 1, 2021

I am having a similar issue, where I have two button elements and one is hidden. I cannot use getByRole with 'hidden = true', because the library is complaining that it found two elements with the same role.

As stated above, a hidden element does not have a name. So tried to use getAllByRole, but then I get a big fat warning that I am using this with elements that are inaccessable.

Why do I get this warning with getAllByRole, but not with getByRole?

@sheryarshirazi
Copy link

sheryarshirazi commented Sep 27, 2021

I'm facing similar issue. I want to find a display value of a textbox but getting multiple results

Example code:

<input name="text" value="val" /> <input name="text" value="val" aria-hidden="true" style={{display: 'none'}} />

Test Query:

await waitFor(() => { screen.getByDisplayValue("val"); });

and the error i'm getting:

Error: Found multiple elements with the display value: val.

in my expectation displayed value are the elements in the DOM with visibility

Packages Detail:
@testing-library/jest-dom: 5.11.6
@testing-library/react: 11.2.2
react-scripts: 3.4.0

@PiotrFidurski
Copy link

I'm facing similar issue. I have a menu bar that turns into hamburger on mobile devices and my tests are failing when using visibility: hidden, I'm using visibility hidden because the mobile and desktop devices use the same navbar component
and I don't want mobile users to be able to focus on links inside the hamburger menu when its collapsed (only when the menu is expanded), which visibility property is perfect for, the only workaround I found to this is using visibility: collapse, in my case this is ok because I'm using elements that are treated by collapse the same as hidden, but it still feels hacky.

@czm1290433700
Copy link

czm1290433700 commented Nov 2, 2022

Sorry, I had a similar problem and I was wondering if this problem was solved. I tried to filter the results with hidden: true, but getAllByRole returns me all the roles

example code for unit code
const hiddenNote = screen.getAllByRole("note", { hidden: true }); const normalNote = screen.getByRole("note");

example code for dom
<div role="note" style={{ display: "none" }} aria-hidden> 1234 </div> <div role="note">1234</div>

In my expectation, this should return that element corresponding to the aria attribute under this role instead of throwing it all to me

If anyone can help me with this problem, it would be really appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants