Dom-testing-library: Add `*BySrc` query selector

[Salstar24]

Describe the feature you'd like:

I'd like to see a query selector that works on the src property of an <img> element. So, I'd like to add the usual query selector methods:

• getBySrc
• queryBySrc
• findBySrc etc.

Usually, *byAltText is sufficient for testing images. However, quite often in my company we want to deliberately hard-code the alt text as an empty string, so that these images are never announced by a screen-reader. (When thinking of accessibility for people with vision problems, most of the time the picture is of no informational value and so we don't want the user to even know it's there.)

This means I can't test my images very easily. I resort to doing:

expect(container.querySelector('img[src="image-path"]')).toBeTruthy()

This is ugly and not nearly as satisfying as a clean getBySrc('image-path') would be.

Is there a best-practice reason I'm unaware of for not accessing src as a property to test against?

Suggested implementation:

Describe alternatives you've considered:

Is there a different way of suppressing the alt text from being read out by a screen-reader? (If we could keep the alt text in this way then nothing would need to be fixed in the testing approach and it would be good for SEO too.)

Teachability, Documentation, Adoption, Migration Strategy:

This would be added into the documentation under the main query selectors section.

It would be used in the same way as the other query selectors.

[eps1lon]

You can always implement custom ones. The docs should mention that.

I'm hesitant to add more into the core since the decision is already hard enough while also enabling inaccessible images that should have an alt attribute. Does byTestId help?

[Yorkshireman]

You can always implement custom ones. The docs should mention that.

I'm hesitant to add more into the core since the decision is already hard enough while also enabling inaccessible images that should have an alt attribute. Does byTestId help?

I think the argument that images should always have alt text is questionable since, as @Salstar24 pointed out, images aren't always useful so I don't think images _should always_ have alt text, unless there is some internet standard I'm not aware of?

Custom selectors could be written but, at the same time, we're able to select other common element types without writing custom selectors, and <img> is a common element type, so I'd argue that it would make sense to give it a standard selector.

[bcarroll22]

Just wanted to chime in really quick that this would be opposed to the guiding principles. Users can't find an image on the page by its source, so neither should the core functionality of this library allow it.

That said though, if you're okay with you and your team finding images on the page by src, a custom query would definitely be the way to go. It's fairly simple to build custom queries, especially ones that are just looking for a string value of an attribute 👍

[Yorkshireman]

@bcarroll22 strictly speaking, you're correct, but images are images and imo it would be in alignment with the library's philosophy to have a standard selector for images because its a visual element on the page and I suspect the only reason that hasn't been implemented is because it's not possible to select an image by its visual content. If you agree with that then the best we can manage in the meantime is selection by src.

[Salstar24]

Thanks for the suggestion of custom queries, @eps1lon and @bcarroll22 - I wasn't aware you could do that and will definitely be writing one for our repo.

In terms of whether to have it as a core query - @bcarroll22 I would argue that, while it's true that users don't find an image by its source, they don't find images by the alt text either (usually). The src attribute seems to be the closest proxy available in the DOM to represent that the image is being displayed on your page (which is the way the user finds it). It's certainly a closer proxy than using data-test-id, although that's a decent fallback for non-common elements.

I agree it does increase the risk of this enabling inaccessible images, but wouldn't having both a *bySrc and a *byAltText function allow an opportunity in the docs to clearly explain the reasons behind each one (e.g. when it might be appropriate _not_ to use alt text for your images)?

[alexkrolick]

• Would a css background-image: url('xyz.png') would match the src selector, or an inlined data:png?
• If we implement this query, I'd be for enforcing alt text at the same time, similar to the JSX-a11y lint rule.

Found an element with  src="xyz.png":

    <img src="xyz.png" /> 

But it does not have an accessible alt-text attribute.
To ignore this check, add { requireAltText: false } to your query.

[eps1lon]

I don't think images should always have alt text,

I'm not sure to whom you are responding. I didn't say they should. Just that this change would allow inaccessible images that should have an alt attribute.

(which is the way the user finds it)

This neglects users of assistive technology entirely. This statement is solely concerned with users who identify images by their content from the source. Users don't even actively look for an image in every app. In this library we only want to assert that the image exists. We do this here mainly by expecting certain properties of the accessibility tree. If you are not concerned about this then visual regression tests seem like the better choice.

I agree it does increase the risk of this enabling inaccessible images, but wouldn't having both a *bySrc and a *byAltText function allow an opportunity in the docs to clearly explain the reasons behind each one (e.g. when it might be appropriate not to use alt text for your images)?

The argument was that this does add another non-trivial choice which is always dangerous. I would rather wait and see if others need this as well. So far the upvotes seem to come from brigading only.

[Salstar24]

@alexkrolick

• Would a css background-image: url('xyz.png') would match the src selector, or an inlined data:png?

I think it would be a good thing if it could? In that case, would it be better to call it something like *ByImagePath? It's not ideal.

• If we implement this query, I'd be for enforcing alt text at the same time, similar to the JSX-a11y lint rule.

Found an element with  src="xyz.png":

    <img src="xyz.png" /> 

But it does not have an accessible alt-text attribute.
To ignore this check, add { requireAltText: false } to your query.

I really like this idea - it could lessen some of the risk of people accidentally using inaccessible images when they didn't mean to. It would add a deliberate step that devs would have to take to confirm that they don't want to apply an alt text to certain images.

I'm interested in the wider question in the accessibility community, though, which is, why is it always a standard recommendation to include an alt-text attribute? Is there any debate about whether that should still be recommended as a blanket best practice? Like I was saying in my original post, it often seems relevant to leave images to be deliberately inaccessible, so that they don't distract from more important information when screen-readers are reading through a website.

[Salstar24]

@eps1lon

(which is the way the user finds it)

This neglects users of assistive technology entirely. This statement is solely concerned with users who identify images by their content from the source. Users don't even actively look for an image in every app. In this library we only want to assert that the image exists. We do this here mainly by expecting certain properties of the accessibility tree. If you are not concerned about this then visual regression tests seem like the better choice.

I apologise - I was summarising my point a little informally/clumsily. I hoped it was covered in my previous answers that in terms of users of assistive technology, I have the question I have asked in my comment above about whether _always_ having alt text is still the standard best-practice for these users. To me, it would be irritating to have a description of every image read out when most of them are only decoration.

For the cases where I want to deliberately not use alt text, but I still want to assert that the image exists, a *BySrc method seems useful. Personally, I would like to be able to check the logic of this image existing directly through my unit tests, as they are more reliable than visual regression tests. (Manual visual regression could easily miss things; automated visual regression tools often have issues like being slow and quite flakey.)

I agree it does increase the risk of this enabling inaccessible images, but wouldn't having both a *bySrc and a *byAltText function allow an opportunity in the docs to clearly explain the reasons behind each one (e.g. when it might be appropriate not to use alt text for your images)?

The argument was that this does add another non-trivial choice which is always dangerous. I would rather wait and see if others need this as well. So far the upvotes seem to come from brigading only.

I understand your hesitation about adding another non-trivial choice. However, could a warning like the one suggested by @alexkrolick above help to mitigate that? I have seen lots of users of the library resorting to using container.querySelector() in order to circumvent using any of the actual query selectors entirely, so maybe adding a couple more choices would help the selectors to seem more approachable?

You are correct that I did post the issue among my colleagues to ask them to add a thumbs-up. I won't ask anyone further. Perhaps another line in the issue template needs to be added to disencourage doing this if it skews the results?

One final thing, which I hesitate to voice since it goes against being purely technical/on-topic and it is so difficult to make sure that things come across correctly online - I know it is the norm in tech to debate things in quite an unemotional (? - closest word I can think of) way. I feel sure you didn't intend this effect, but when I read your latest reply I felt a little intimidated. This is one of my first forays into open source, and I am really enthusiastic about the whole testing-library project. I don't really know what I'm trying to say here, but I guess I'm curious if you know if the testing-library has any guiding principles around inclusivity?

[kentcdodds]

Hi @Salstar24,

Thank you for posting the issue. First, I'd like to address this:

I'm curious if you know if the testing-library has any guiding principles around inclusivity?

I apologize for any intimidation this discussion may have caused. As the maintainer of this project, I take inclusivity very seriously and we have and enforce a Code of Conduct. I expect no ill-will was intended and this is a general problem of online communication, but just to clear the air I want to remind everyone that the Testing Library community is one of friendliness and inclusion. I'd like to thank everyone for upholding that as well as we have thus far in the project and hope that we can continue to do so.

Again, I'm sorry @Salstar24, if that didn't show super well with this issue. I'm so glad that you have opened up this issue and participated in the open source discussion. You are absolutely welcome here.

You are correct that I did post the issue among my colleagues to ask them to add a thumbs-up.

That's fine with me. It doesn't bother me at all and I do that all the time (sometimes I'll tweet out for support of my issues and with the number of followers I have I think it would be pretty hypocritical of me to expect anyone else to do any different).

---

Now, to the issue at hand.

It's certainly a closer proxy than using data-test-id, although that's a decent fallback for non-common elements.

I definitely agree with this.

If we implement this query, I'd be for enforcing alt text at the same time, similar to the JSX-a11y lint rule.

I think this is a pretty good idea.

In general, I'm still undecided about including this as a built-in query. It's pretty easy to add it as a custom query. There are not very many cases I can think of on the web where a blind user wouldn't want to know something about the image that seeing users can experience. I think it's an inclusion/accessibility issue. And while not every image should necessarily have alt text, I think most of them should. By adding a *BySrc query, we're signaling that it's ok to not have alt text. Which I don't want to do.

[alexkrolick]

I could see a jest-dom matcher as well, that understands both src attributes and css background image urls:

expect(getByAltText('a dog sitting on the porch')).toHaveImageSrc('/img-dog.png')

[Salstar24]

Hello @kentcdodds,

Thanks for your reply. I do feel reassured around the inclusivity issue. As you say -

I expect no ill-will was intended and this is a general problem of online communication

And I agree. So much of online communication is perception and I think mainly I just got nervous that my points weren't being fully understood. @eps1lon my apologies for my momentary wobble in trusting to your good-will; the internet is a scary place sometimes (at least so it seems to me).

---

Now back to the actual issue:

There are not very many cases I can think of on the web where a blind user wouldn't want to know _something_ about the image that seeing users can experience. [...] And while not every image should necessarily have alt text, I think most of them should. By adding a *BySrc query, we're signaling that it's ok to not have alt text. Which I don't want to do.

This is definitely the heart of the matter. As I say, in my company I've been hearing it as recommended best practice regarding accessibility to only include alt text when the picture conveys functional information to the users. I asked my colleagues where this best practice comes from and found this excellent decision tree from the Web Accessibility Initiative about when and when not to use alt text.

From this, it certainly seems to be a common enough recommendation to deliberately make the alt text empty, and it strengthens my belief that this is a good reason to have a *BySrc query as a core part of the testing-library. Currently, the testing library's queries imply that one should _always_ use alt text (hence only providing the *ByAltText query as the standard way of testing images). This is not in line with inclusivity/accessibility best practices.

My current idea for a possible solution:

• The core library offers both a *BySrc and *ByAltText query
• When the *BySrc query is used during testing, a little 'lint warning'-style message pops up (as suggested by @alexkrolick above):

Found an element with src="xyz.png":

    <img src="xyz.png" /> 

But it does not have an accessible alt-text attribute.
This is recommended if your image is purely decorative.
Please check https://www.w3.org/WAI/tutorials/images/decision-tree/ if you are unsure 
whether your image contains functional information which should be included 
in an alt-text attribute.

• And a very similar message would pop up for when the *ByAltText query is used, something like:

Found an element with alt="This image shows xyz":

    <img src="xyz.png" alt="This image shows xyz" /> 

This is recommended if your image contains functional information.
Please check https://www.w3.org/WAI/tutorials/images/decision-tree/ if you are unsure 
whether your image is purely decorative, in which case an empty alt-text 
attribute is recommended.

My wording needs tightening up in these examples, and we probably want to include a way of turning this warning off, but I think this would nicely encourage a more nuanced awareness of accessibility best-practices via this test-library.

[kentcdodds]

Hi @Salstar24,

I think those suggestions are reasonable. The problem is it's really hard for us to know when to show the warning and when to know that the user is doing the right thing. But without it, it's hard for us to communicate the proper use of the query, which is important because it's a "dangerous" (for lack of a better term) query to rely on.

I'm leaning toward not including it in the core, and recommending people add it as a custom query to their custom utils. I realize this is not what you were hoping for, but hopefully it helps you with your goals.

It sounds like your app has a lot of decorative images, but my inclination is that most users of testing library have images that should have alt text. I just really want to avoid a situation where people start using a *BySrc query and not consider the fact that they should be adding alt text to their non-decorative images.

[eps1lon]

@Salstar24 I wanted to personally apologize for being intimidating. I totally understand that it isn't a very nice experience to open your first issue in open source only to be shut down. I hope this doesn't discourage you to keep on contributing to open source since you explained a lot and provided plenty of context. This isn't something you see a lot from people new to open source so I appreciate this from you even more 👍

As to the issue itself:

(hence only providing the *ByAltText query as the standard way of testing images)

Maybe we can solve this with better documentation. I don't think we wanted to imply you should only use ByAltText for images. You could also use ByRole('image') or ByTestId('my-decorative-image') or even container.querySelector('img[src="decorative.png"]'). These are all valid choices IMO.

The problem is that it is impossible? to make sure that you explicitly set the alt attribute to "" or if this was an accident i.e. how can we verify that an image should be decorative?

[Salstar24]

@kentcdodds @eps1lon Thank you both for your lovely replies. What you're saying makes total sense and I understand your decision not to put this into the core queries. I will make my own custom util for our own repo, and I may look into opening a PR on the documentation to add a paragraph about testing images/when and where to use alt text (probably linking to that decision tree I posted above). I might even see if there's somewhere appropriate in the docs to host the getBySrc custom util code as an example, even if that's as an external gist or something that can be linked to from the docs.

This has been a positive first experience in open source, so thanks again! I'll close the issue now. :)

[kentcdodds]

I think an addition to the docs as you describe would be fantastic. If you do end up doing that, it would definitely be appreciated :)