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

Adjust toBeVisible matcher #263

Closed
kylemh opened this issue Jun 15, 2020 · 5 comments
Closed

Adjust toBeVisible matcher #263

kylemh opened this issue Jun 15, 2020 · 5 comments

Comments

@kylemh
Copy link

kylemh commented Jun 15, 2020

Describe the feature you'd like:

Many developers leverage a component to render content, but not have it be visible for the sake of accessibility. There are many places where this strategy is recommended, but the most popular one is outlined in the US Government's accessibility documentation. The .toBeVisible matcher would assert that any element leveraging these styles IS visible, but I would love to be able to assert that it is NOT visible.

Suggested implementation:

Explicitly check for all the properties matching as-is OR the existing logic in the matcher source code.

Describe alternatives you've considered:

🤷

Teachability, Documentation, Adoption, Migration Strategy:

🤷

@gnapse
Copy link
Member

gnapse commented Jun 15, 2020

I'm hesitant about changing the semantics of toBeVisible so drastically. This would be not only a breaking change if done as you suggest, but a breaking change that would be a game changer for anyone using the matcher in the way you now want it not to behave. Or correct me if I got that wrong.

Also, can you please share here a more precise approach to check for whatever makes an element invisible but still visible to screen readers? Is it checking the precise set of styles that the .sr-only css class has in that linked documentation you shared?

And last but not least, visibility detection has been one of the most controversial or confusing features we've released with the matchers in this lib. Relying on things such as what css is applied to an element at any given point is tricky. Mostly because in the test env usually the full css that would normally be available in the app when running is not loaded or even attached to the jsdom document. I wrote more about that here #116 (comment) and especially here #113 (comment). If this would-be feature relies on checking those sets of css it can be tricky for people to actually make it work.

@kylemh
Copy link
Author

kylemh commented Jun 15, 2020

Is it checking the precise set of styles that the .sr-only css class has in that linked documentation you shared?

Correct. My mindset would be to check the computed properties for a strict match.

I'm hesitant about changing the semantics of toBeVisible so drastically

No worries! I figured there may be hesitation. I think it's the right way to go about it since the element is definitely not visible; however, if it's easier, we could make a new matcher! Maybe .toRenderAsHidden?

Relying on things such as what css is applied to an element at any given point is tricky. Mostly because in the test env usually the full css that would normally be available in the app when running is not loaded or even attached to the jsdom document.

Definitely. Visibility seems to be a rough issue for anything riding a jsdom environment...

Perhaps the best way for me to go about this would be to make my own custom matcher 🤷

@eps1lon
Copy link
Member

eps1lon commented Jun 15, 2020

@testing-library/dom can already check for elements being accessible or not. We have a public method for that which you can use to implement your matcher:

import { isInaccessible } from '@testing-library/dom';

Then you apply the same semantics that e.g. getByRole('textbox', { hidden: false }) would be using.

I wouldn't recommend a matcher for visually hidden. Visibility has very little meaning. What you probably want is "perceivable" and for that you're better off with visual regression tools or manual testing.

@kylemh kylemh closed this as completed Jun 15, 2020
@kylemh
Copy link
Author

kylemh commented Jun 15, 2020

import { isInaccessible } from '@testing-library/dom';

This wouldn't be relevant, but your remark about visual regression testing rings true.

@gnapse
Copy link
Member

gnapse commented Jun 15, 2020

Nice discussion here, and very good points added by @eps1lon. Thanks!

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

No branches or pull requests

3 participants