testing-library / dom-testing-library

🐙 Simple and complete DOM testing utilities that encourage good testing practices.
https://testing-library.com/dom
MIT License
3.26k stars 466 forks source link

add `excludeHidden` for *ByText queries #1184

Open rluvaton opened 1 year ago

rluvaton commented 1 year ago

What: add excludeHidden option for *ByText queries

Why: #196

How: used the already existing isInaccessible function

Checklist:

rluvaton commented 1 year ago

Before I start adding tests, what do you think about this?


Cc @kentcdodds

I'll reopen this and I'd be willing to look at a PR for this. https://github.com/testing-library/dom-testing-library/issues/196#issuecomment-628268484

codesandbox-ci[bot] commented 1 year ago

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

Sandbox Source
react-testing-library-examples Configuration
kentcdodds commented 1 year ago

Honestly, I think this would be a worthwhile thing in all the queries and eventually should be enabled by default (which of course would be a breaking change).

kentcdodds commented 1 year ago

I think that's fair @eps1lon. As I'm no longer maintaining Testing Library, I don't think it's fair for me to make decisions here. I think the best would be to have this supported in all the queries and the "faster, less accurate helper" can be offered by using the option.

eps1lon commented 1 year ago

I'm not fundamentally opposed. But the added dimension due to the naming of the option is a bigger concern for me right now.

kentcdodds commented 1 year ago

Oh, yeah, I agree we should probably keep the names consistent 👍

rluvaton commented 1 year ago

I used different name to avoid confusion with *ByRole as that hidden: true there means to show hidden as well While in the *ByText we want to exclude them, and if we use the same configuration we would break our users as currently defaultHidden is false - meaning that we don't return hidden results and which is not what we currently do for *ByText queries...