feat(ByAltText): Always include custom elements #1049
Conversation
|
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 3536716:
|
|
Sorry, there was a mixup on who was supposed to create a PR for this issue.. feel free to choose between this and #1048 |
src/queries/alt-text.ts
Outdated
| ).filter(node => | ||
| matcher(node.getAttribute('alt'), node, alt, matchNormalizer), | ||
| ) | ||
| return queryAllByAttribute('alt', container, alt, options) |
There was a problem hiding this comment.
If this PR only targets custom elements then let's restrict it to known and custom elements. Otherwise <p alt /> becomes queryable.
There was a problem hiding this comment.
I've added a filter.
Codecov Report
@@ Coverage Diff @@
## main #1049 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 25 25
Lines 920 919 -1
Branches 284 283 -1
=========================================
- Hits 920 919 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
@SantoJambit Could you review the docs change testing-library/testing-library-docs#946 ? |
eps1lon
changed the title
|
@eps1lon thanks for the review and the docs PR. I've reviewed the docs PR. |
|
@all-contributors add @SantoJambit for code |
|
I've put up a pull request to add @SantoJambit! 🎉 |
What: Made the ByAltText methods tag-agnostic
Why: ByAltText does currently not support
<amp-img>(and possibly other custom-elements).How: by using the existing
queryAllByAttributefunction and thus dropping the limitation to only 'img,input,area' tags.Checklist:
docs site: docs(ByAltText): Add notes about support for custom elements testing-library-docs#946
Note:
queryAllByAltTextandqueryAllByAttributeare mostly identical, so I chose to just callqueryAllByAttribute.However queryAllByAltText had an additional call to
checkContainerType, which I left in place. Not sure whyqueryAllByAttributedoes not have this check.Alternatively, I could leave the old logic in place and simply add the tag amp-img to the allowed list, but I think this way is better.
I will create an update PR for the docs when it's clear, that this will be merged.