ByText finds children of hidden elements

[mancristiana]

• @testing-library/react version: 11.1.0
• Testing Framework and version:
  I am using the built-in testing tools that come with create-react-app
@testing-library/jest-dom: 5.11.4
  @testing-library/user-event: 12.1.10
  react-scripts: 4.0.3
  jest: 26.6.0 (hoisted from react-scripts)
• DOM Environment: jsdom: 16.4.0

Relevant code or config:

Consider this collapsible element with aria-hidden attribute that displays children props

<div
  aria-hidden={isCollapsed}
  className={isCollapsed ? "hide" : "show"}
>
  {children}
</div>

This assertion fails

expect(screen.queryByText("children")).toBeNull();

What you did:

When using screen.queryByText("children") I expected not to find hidden elements that specify aria-hidden={isCollapsed} attribute.
However, when running the test I could see that the element was found.

What happened:

I got the following failed test:

  ● collapses and expands children

    expect(received).toBeNull()

    Received: <div aria-hidden="true" class="hide">children</div>

       5 | test('collapses and expands children', () => {
       6 |   render(<Collapsible>children</Collapsible>)
    >  7 |   expect(screen.queryByText("children")).toBeNull();
         |                                          ^
       8 |
       9 |   userEvent.click(screen.getByRole("button", { name: "Expand" }));
      10 |   expect(screen.getByText("children")).toBeInTheDocument();

      at Object.<anonymous> (src/__tests__/index.test.js:7:42)

Reproduction:

I reproduced this issue in this repo: https://github.com/mancristiana/dom-testing-library-template

Problem description:

Since the findByRole query supports this scenario, I expected this would be true for other queries.
The current behavior is a problem because it does not reflect the way a user would interact with our application. In this concrete example, a hidden element would not appear for the user, whereas our test finds it in the dom tree.

Suggested solution:

Seeing how this behavior is supported for ByRole queries I suggest implementing something similar for ByText query.

[jovicon]

Can I take this issue?

[eps1lon]

Thanks for the report.

Note ByText is a lightweight query helper. Testing for a11y tree inclusion is fairly expensive and comes at the cost of degraded performance which not everybody appreciates (see #820). If people favor performance over confidence then they have the choice to use other queries instead of ByRole such as ByText. This choice would be removed if we normalize the implementation. We also wouldn't need ByText if it would consider a11y.

[MatanBobi]

@eps1lon So do you think we should push this one forwards or not? You've added the enhancement label but from your comment it doesn't sound like it should be implemented.
Maybe we can add a configuration to ByText to state if we want it to take a11y in consideration and the default will be false?

[eps1lon]

I just don't see why you'd want to use ByText with a11y consideration if you already have ByRole. The only concern is with teachability here.

[eps1lon]

Thanks for the feedback but we're not going to implement this.

ByRole already does what you want and has specified behavior. Just considering aria-hidden in ByText is just fragmenting how "a11y tree inclusion" is computed in this library and causes surprising behavior. Considering a11y tree inclusion just like ByRole does comes with perf drawbacks outlined in #929 (comment).

Before, people would need to know whether a query considers a11y tree inclusion or not. Now they would also need to know how a11y tree inclusion is computed depending on the query.

[mancristiana]

I just don't see why you'd want to use ByText with a11y consideration if you already have ByRole. The only concern is with teachability here.

It is not always possible to use the byRole. Consider this scenario where we have two matching paragraphs in the document containing 'TEXT'.

<p class="visuallyHidden">
  TEXT
</p>
<p aria-hidden="true">
  TEXT
</p>

The first one with class="visuallyHidden" might inside a section with aria-live polite optimized for screen readers, whereas the other one with aria-hidden="true" appears visually, but is hidden from screen readers.

I would expect that expect(screen.getByText('TEXT')).toBeInTheDocument(); finds only one paragraph.

[mryechkin]

@mancristiana how did you end up solving your scenario? I'm running into a similar situation and ended up here.

[mancristiana]

One workaround is using a custom TextMatch function.

screen.getByText(
  (content, element) => 
     content.startsWith('TEXT') && 
     !element.hasAttribute("aria-hidden"))

We can explicitly match elements with TEXT content that don't have an aria-hidden attribute

[MatanBobi]

You can also use isInaccessible from dom-testing-library to verify that an element is indeed accessible. This one is what we're using in ByRole queries.