-
Notifications
You must be signed in to change notification settings - Fork 467
Description
Hello all :)
Thanks for the library 🔥
@testing-library/domversion: v10.4.0
Relevant code or config:
I made this branch with a test https://github.com/romain-trotard/dom-testing-library/tree/labelledby-multiple-repro and some comments on the code I think we should change.
const {getByLabelText} = render(`
<div id="frameworks-row" role="row" aria-label="Frameworks">
<button aria-labelledby="select frameworks-row" id="select" aria-label="Select" type="button">Select</button>
<span>Frameworks</span>
<span>Other information</span>
</div>
`)
// Do not find `Select Frameworks` because it takes element.textContent
// and not `aria-label` for the element with id `frameworks-row`
expect(getByLabelText('Select Frameworks').id).toBe('select')
expect(getByLabelText('Frameworks').id).toBe('frameworks-row')What you did:
There are some use cases where we want to labelled an element with aria-labelledby that references an element that has aria-label.
For example, in the template above the button element has a aria-labelledby that references himself (that has aria-label) and the div with id frameworks-row (that has aria-label)
What happened:
The selector getByLabelText('Select Frameworks') does not work because when getting the element with id: frameworks-row, it's not the aria-label that is retrieved but the textContent (here Select Frameworks Other information)
Reproduction:
https://github.com/romain-trotard/dom-testing-library/tree/labelledby-multiple-repro
npm run test
Problem description:
The selector getByLabelText('Select Frameworks') does not work because when getting the element with id: frameworks-row, it's not the aria-label that is retrieved but the textContent (here Select Frameworks Other information)
Suggested solution:
Here
dom-testing-library/src/label-helpers.ts
Line 32 in a86c54c
| textContent = (element as HTMLInputElement).value || element.textContent |
We should get the
aria-label attribute before getting textContent, like this:
textContent = (element as HTMLInputElement).value || element.getAttribute('aria-label') || element.textContentBut we also need to find a way to concat these labels (when working with aria-labelledby) here:
dom-testing-library/src/queries/label-text.ts
Lines 67 to 86 in a86c54c
| const labelList = getLabels(container, labelledElement, {selector}) | |
| labelList | |
| .filter(label => Boolean(label.formControl)) | |
| .forEach(label => { | |
| if ( | |
| matcher(label.content, label.formControl, text, matchNormalizer) && | |
| label.formControl | |
| ) { | |
| labelledElements.push(label.formControl) | |
| } | |
| }) | |
| const labelsValue = labelList | |
| .filter(label => Boolean(label.content)) | |
| .map(label => label.content) | |
| if ( | |
| matcher(labelsValue.join(' '), labelledElement, text, matchNormalizer) | |
| ) { | |
| labelledElements.push(labelledElement) | |
| } | |
| if (labelsValue.length > 1) { |
Note: For the moment, I haven't given the solution much thought.
If the bug is acknowledged, I would be happy to make a fix ;)
Thanks :)
