get by label concat values #681
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,29 @@ import { | |
| wrapAllByQueryWithSuggestion, | ||
| wrapSingleQueryWithSuggestion, | ||
| } from './all-utils' | ||
|
|
||
| function queryAllLabels(container) { | ||
| return Array.from(container.querySelectorAll('label,input')) | ||
|
There was a problem hiding this comment. It occurs to me that an element can be labeled by anything, not just That said, I'm not sure how to handle "everything else." It seems to me that rather than searching for all possible elements that can label anything, we should be searching for everything that's labeled first, determine the contents of their label, and then return the elements whose label matches the query. There was a problem hiding this comment. For what it's worth, I think that searching for labelled items and then for their labels is more feasible than querying for all elements that could be labels. Like you said above! There was a problem hiding this comment. If you'd like to tack a whack at that, I think it would solve a lot of problems. There was a problem hiding this comment. For sure I would like to do it! |
||
| .map(node => { | ||
| let textToMatch = | ||
| node.tagName.toLowerCase() === 'label' | ||
| ? node.textContent | ||
| : node.value || null | ||
| // The children of a textarea are part of `textContent` as well. We | ||
| // need to remove them from the string so we can match it afterwards. | ||
| Array.from(node.querySelectorAll('textarea')).forEach(textarea => { | ||
| textToMatch = textToMatch.replace(textarea.value, '') | ||
| }) | ||
|
|
||
| // The children of a select are also part of `textContent`, so we | ||
| // need also to remove their text. | ||
| Array.from(node.querySelectorAll('select')).forEach(select => { | ||
| textToMatch = textToMatch.replace(select.textContent, '') | ||
| }) | ||
| return {node, textToMatch} | ||
| }) | ||
| .filter(({textToMatch}) => textToMatch !== null) | ||
| } | ||
|
|
||
| function queryAllLabelsByText( | ||
| container, | ||
|
|
@@ -19,23 +41,25 @@ function queryAllLabelsByText( | |
| ) { | ||
| const matcher = exact ? matches : fuzzyMatches | ||
| const matchNormalizer = makeNormalizer({collapseWhitespace, trim, normalizer}) | ||
|
|
||
| const textToMatchByLabels = queryAllLabels(container) | ||
|
|
||
| return textToMatchByLabels | ||
| .filter(({node, textToMatch}) => | ||
| matcher(textToMatch, node, text, matchNormalizer), | ||
| ) | ||
| .map(({node}) => node) | ||
| } | ||
|
|
||
| function getLabelContent(label) { | ||
| let labelContent = label.getAttribute('value') || label.textContent | ||
| Array.from(label.querySelectorAll('textarea')).forEach(textarea => { | ||
| labelContent = labelContent.replace(textarea.value, '') | ||
| }) | ||
| Array.from(label.querySelectorAll('select')).forEach(select => { | ||
| labelContent = labelContent.replace(select.textContent, '') | ||
| }) | ||
| return labelContent | ||
| } | ||
|
|
||
| function queryAllByLabelText( | ||
|
|
@@ -45,74 +69,69 @@ function queryAllByLabelText( | |
| ) { | ||
| checkContainerType(container) | ||
|
|
||
| const matcher = exact ? matches : fuzzyMatches | ||
| const matchNormalizer = makeNormalizer({collapseWhitespace, trim, normalizer}) | ||
| const matchingLabelledElements = Array.from(container.querySelectorAll('*')) | ||
| .filter( | ||
| element => element.labels || element.hasAttribute('aria-labelledby'), | ||
| ) | ||
| .reduce((labelledElements, labelledElement) => { | ||
| const labelsId = labelledElement.getAttribute('aria-labelledby') | ||
| ? labelledElement.getAttribute('aria-labelledby').split(' ') | ||
| : [] | ||
| const labelsValue = labelsId.length | ||
| ? labelsId.map(labelId => { | ||
|
There was a problem hiding this comment. What happens if an element had both a .labels property and the aria-labelledby? I don't know if that's valid or not. Is it? There was a problem hiding this comment. aria-labelledby has precedence: https://www.w3.org/TR/accname-1.1/#step2B 2B is for aria-labelledby, 2D for There was a problem hiding this comment. In that case, I think this PR is good to go! Anyone have other thoughts? |
||
| const labellingElement = container.querySelector(`[id=${labelId}]`) | ||
| return getLabelContent(labellingElement) | ||
| }) | ||
| : Array.from(labelledElement.labels).map(label => { | ||
| const textToMatch = getLabelContent(label) | ||
| const formControlSelector = | ||
| 'button, input, meter, output, progress, select, textarea' | ||
| const labelledFormControl = Array.from( | ||
| label.querySelectorAll(formControlSelector), | ||
| ).filter(element => element.matches(selector))[0] | ||
| if (labelledFormControl) { | ||
| if ( | ||
| matcher(textToMatch, labelledFormControl, text, matchNormalizer) | ||
| ) | ||
| labelledElements.push(labelledFormControl) | ||
| } | ||
| return textToMatch | ||
| }) | ||
| if ( | ||
| matcher(labelsValue.join(' '), labelledElement, text, matchNormalizer) | ||
| ) | ||
| labelledElements.push(labelledElement) | ||
| if (labelsValue.length > 1) { | ||
| labelsValue.forEach((labelValue, index) => { | ||
| if (matcher(labelValue, labelledElement, text, matchNormalizer)) | ||
| labelledElements.push(labelledElement) | ||
|
|
||
| const labelsFiltered = [...labelsValue] | ||
| labelsFiltered.splice(index, 1) | ||
|
|
||
| if (labelsFiltered.length > 1) { | ||
| if ( | ||
| matcher( | ||
| labelsFiltered.join(' '), | ||
| labelledElement, | ||
| text, | ||
| matchNormalizer, | ||
| ) | ||
| ) | ||
| labelledElements.push(labelledElement) | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| return labelledElements | ||
| }, []) | ||
| .concat(queryAllByAttribute('aria-label', container, text, {exact})) | ||
|
|
||
| return Array.from(new Set(matchingLabelledElements)).filter(element => | ||
| element.matches(selector), | ||
| ) | ||
| } | ||
|
|
||
| // the getAll* query would normally look like this: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should find
textarea. And it does so when usinglabel.controlThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I am not understanding, you are saying that the right test to be done here (at line 374) was that the labeled element is
textareainstead of that noinputelement labeled was found?getByLabelText(/alone/)would have found thetextareaelement, I added the selector to make it return null.Please tell me if I am missing something.