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

feat: Add strictVisibility check option to ByRole queries #1331

Closed
wants to merge 1 commit into from

Conversation

benlesh
Copy link

@benlesh benlesh commented Aug 1, 2024

What:

Added an option called strictVisibilityCheck that defaults to true (for now). If true it will use the existing code path that does a getComputedStyle on each node to check visibility during queries ByRole. If set to false, the visibility check settles for a more lose algorithm that doesn't run getComputedStyle.

Why:

This is being added as a means of improving performance of tests using queries like getAllByRole on larger sets of DOM elements, as can be found in many real-world applications. Anecdotally, using this setting fixed one test I had locally from 8000ms to 500ms total. (still not great, but miles better).

How:

Added an option to the type definition, threaded it through the existing logic, and added a "loose" check for inaccessibility. I added a test with a few checks to make sure the default is still working as expected, and the new setting still works in a similar manner.

It's worth noting that I tried to add a test that checked "indirect" visibility alterations through CSS classes, however, I was unable to get the test harness to recognize the CSS class and trigger layout (I was probably doing something wrong there, happy to follow up).

Checklist:

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

Copy link

codesandbox-ci bot commented Aug 1, 2024

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

Sandbox Source
react-testing-library-examples Configuration

Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

I don't want to define any custom interpretation what it means to be in the a11y tree. This will be hard to teach and probably change too often depending on the performance needs. Users should use the hidden option instead if they don't want to check if an element is in the a11y tree or not.

@benlesh
Copy link
Author

benlesh commented Aug 1, 2024

hidden: true isn't quite accurate to the need, but whatever. Not my library. I'll fork it if I need this consistently, I guess.

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 this pull request may close these issues.

2 participants