[Feature]: Visible option for locators

[qk5]

🚀 Feature Request

I have a locator:
page.getByText('Waiting for response...', { exact: true });

And playwright points to multiple locators on screen, one visible locator and other invisible ones.

I only want the visible one, and my workaround is to use the following locator:
page.locator('div:has-text("Waiting for response..."):visible');

Please note that: If I remove the part :visible, this locator will behave the same as my first example using getByText()

So, is it possible to add an additional option within getByText() and other getBy locator methods for visible locator?

Maybe something like:
page.getByText('Waiting for response...', { visible: true });

Example

No response

Motivation

I believe having the visible option will give us more flexibility when creating any getBy locators.

[qk5]

Update: Upon reading more on the playwright documentation, I found out the following also works:

page.getByText('Waiting for response...', { exact: true }).locator('visible=true');

But I still think if we can expose visible as an option, it will look cleaner. Please feel free to correct me if I am wrong.

Thank you.

[Skn0tt]

Hey @qk5! We've discussed this in our team sync yesterday. The fact that by default, getByText selects not just visible elements but also invisible ones is a design regret of ours. All the RTL-Style selectors (think getByText, getByAltText, getByRole) are modeled after human perception, and that means that it's surprising for it to include hidden elements. So if we could go back in time, we would change the defaults so that getByText only selects visible elements.

We can't though, and we'd prefer not to introduce breaking changes in such a fundamental part of our API. Even if we made this behaviour a config toggle, we'd be forking our userbase into two fundamentally different behaviours, which would lead to even more confusion. Really bad, especially when you consider that libraries have no way of detecting it.

Now, the visible option you propose is an interesting way out of this! Before we think about implementing it though, we'd like to understand how big this problem really is. I'll label it "P3: Collecting Feedback" and then we can collect some upvotes on the issue. Hopefully that gives us some perspective on the gravitas of this, and sleep a couple of nights about other solutions to the problem.

[Skn0tt]

Also, an interesting observation in API design: in getByText("...", { visible: true }), the visible option is trimodal! With the defaults we have, { visible: false } clearly means "only select invisible", { visible: true } means "only select visible", and { visible: undefined } means "do the default", which is selecting both visible and invisible elements.

[dtinth]

I want to chime in that a workaround for this is to use getByRole instead of getByText, as we can exploit the fact that invisible elements do not have a role:

when visible	when hidden

Maybe the documentation can address getByText’s quirk (of also selecting visible elements) to nudge people into using getByRole more.

[miso-belica]

We can't though, and we'd prefer not to introduce breaking changes in such a fundamental part of our API. Even if we made this behaviour a config toggle, we'd be forking our userbase into two fundamentally different behaviours, which would lead to even more confusion. Really bad, especially when you consider that libraries have no way of detecting it.

@Skn0tt For testing a react-native-web applications this is really important because the routing works by hiding the pages and stack them one on another. Then selecting base things like navigation can lead to multiple elements found in all tests. Adding a global option to ignore invisible/hidden elements would be really a great help here so we don't have to add .locator('visible=true') to every selector.

The package react-native-testing-library changed the default behavior to exclude elements hidden from accessibility by default so maybe you can inspire or talk to them about this. It was a big help for our tests.

[grundmanise]

I was surprised when getByText locators returned invisible elements. I would recommend adding a warning to the docsite to mention that scenario as it's not obvious, and I found this thread only by chance. { visible: true } is an option, but I would give my vote to have this as the default in a major release.

[miso-belica]

We can't though, and we'd prefer not to introduce breaking changes in such a fundamental part of our API. Even if we made this behaviour a config toggle, we'd be forking our userbase into two fundamentally different behaviours, which would lead to even more confusion. Really bad, especially when you consider that libraries have no way of detecting it.

I was thinking about this more as it is still very annoying for our tests. I understand this is a breaking change but I believe this is a good one. Considering people are currently probably testing with selectors catching on hidden elements they would actually catch bugs in their code after a break change. I can imagine someone not practicing TDD and not seeing red test in the first place writing a green test assuming everything is working when the actual user would not be able to do an action because of some hidden element. So it's more like fixing an important bug in the testing framework :) Also, for people whose tests are properly written nothing would break because they are already selecting visible elements.

In conclusion, this is only a breaking change for people with false-green tests and by updating they catch bugs in their app. Also, if that helps, it is possible without hard breaking change. With config flag and a warning, everyone can migrate smoothly, fix this issue with config and then migrate on next version with turned this by default.

[dtinth]

Playwright since v1.51.0 now allows filtering an existing locator for visible ones.

• Docs: https://playwright.dev/docs/locators#matching-only-visible-elements
• Seems to be implemented in chore: replace locator.visible with filter({ visible }) #34947.
• I think this issue can be closed now.