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

Add more robust selector handling for getByLabelText #373

Merged
merged 11 commits into from
Mar 4, 2020
32 changes: 31 additions & 1 deletion src/__tests__/element-queries.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ test('can get elements labelled with aria-labelledby attribute', () => {
expect(getByLabelText('Section One').id).toBe('section-one')
})

test('can get sibling elements with aria-labelledby attrib ute', () => {
test('can get sibling elements with aria-labelledby attribute', () => {
const {getAllByLabelText} = render(`
<div>
<svg id="icon" aria-labelledby="icon-desc"></svg>
Expand All @@ -212,6 +212,36 @@ test('can get sibling elements with aria-labelledby attrib ute', () => {
expect(result[0].id).toBe('icon')
})

test('can filter results of label query based on selector', () => {
const {getAllByLabelText} = render(`
<div>
<label id="label1" for="input1">
Test Label
<input id="input2" />
</label>
<input id="input1" class="fancy-input" />
<span aria-labelledby="label1">Some hint text</span>
</div>
`)

const result = getAllByLabelText('Test Label', {selector: '.fancy-input'})
expect(result).toHaveLength(1)
expect(result[0].id).toBe('input1')
})

test('can find input when label text is in a span', () => {
const {getAllByLabelText} = render(`
<label>
<span>Test Label</span>
<input id="input1" />
</label>
`)

const result = getAllByLabelText('Test Label', {selector: 'input'})
expect(result).toHaveLength(1)
expect(result[0].id).toBe('input1')
})

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add another test for the situation I found myself in where we had two elements that had the same label text?

  const {getByLabelText} = render(`
    <label>
      Test Label
      <input />
    </label>
    <label>
      Test Label
      <textarea></textarea>
    </label>
  `)
  // select the input

Thanks!

test('get can get form controls by placeholder', () => {
const {getByPlaceholderText} = render(`
<input id="username-id" placeholder="username" />,
Expand Down
33 changes: 22 additions & 11 deletions src/queries/label-text.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ function queryAllByLabelText(
normalizer: matchNormalizer,
})
const labelledElements = labels
.map(label => {
.reduce((matchedElements, label) => {
const elementsForLabel = []
if (label.control) {
return label.control
elementsForLabel.push(label.control)
}
/* istanbul ignore if */
if (label.getAttribute('for')) {
Expand All @@ -51,21 +52,25 @@ function queryAllByLabelText(
// <label for="someId">text</label><input id="someId" />

// .control support has landed in jsdom (https://github.com/jsdom/jsdom/issues/2175)
return container.querySelector(`[id="${label.getAttribute('for')}"]`)
elementsForLabel.push(
container.querySelector(`[id="${label.getAttribute('for')}"]`),
)
}
if (label.getAttribute('id')) {
// <label id="someId">text</label><input aria-labelledby="someId" />
return container.querySelector(
`[aria-labelledby~="${label.getAttribute('id')}"]`,
)
container
.querySelectorAll(`[aria-labelledby~="${label.getAttribute('id')}"]`)
.forEach(element => elementsForLabel.push(element))
}
if (label.childNodes.length) {
// <label>text: <input /></label>
return label.querySelector(selector)
label
.querySelectorAll(selector)
.forEach(element => elementsForLabel.push(element))
}
return null
})
.filter(label => label !== null)
return matchedElements.concat(elementsForLabel)
}, [])
.filter(element => element !== null)
.concat(queryAllByAttribute('aria-label', container, text, {exact}))

const possibleAriaLabelElements = queryAllByText(container, text, {
Expand All @@ -89,7 +94,13 @@ function queryAllByLabelText(
[],
)

return Array.from(new Set([...labelledElements, ...ariaLabelledElements]))
const allMatches = Array.from(
new Set([...labelledElements, ...ariaLabelledElements]),
)
const elementsMatchingSelector = new Set(container.querySelectorAll(selector))
return allMatches.filter(matchingElement =>
elementsMatchingSelector.has(matchingElement),
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'd be better off with matches here: https://developer.mozilla.org/en-US/docs/Web/API/Element/matches

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a feeling this wasn't the best way. Probably could have done a little more digging haha.

}

// the getAll* query would normally look like this:
Expand Down