From 019a1c32d5b9b075c64719f65aa6c8a50b0fedd9 Mon Sep 17 00:00:00 2001 From: airjp73 Date: Sun, 6 Oct 2019 09:47:00 -0400 Subject: [PATCH 1/8] test: Add test cases for more robust label filtering --- src/__tests__/element-queries.js | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/src/__tests__/element-queries.js b/src/__tests__/element-queries.js index 6d2becac..e8904770 100644 --- a/src/__tests__/element-queries.js +++ b/src/__tests__/element-queries.js @@ -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(`
@@ -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(` +
+ + + Some hint text +
+ `) + + 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(` + + `) + + const result = getAllByLabelText('Test Label', {selector: 'input'}) + expect(result).toHaveLength(1) + expect(result[0].id).toBe('input1') +}) + test('get can get form controls by placeholder', () => { const {getByPlaceholderText} = render(` , From 2a832cdf6c8d3c29e18944972be6ea5bf5aa946c Mon Sep 17 00:00:00 2001 From: airjp73 Date: Sun, 6 Oct 2019 09:47:33 -0400 Subject: [PATCH 2/8] feat: Add more robust label filtering --- src/queries/label-text.js | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/queries/label-text.js b/src/queries/label-text.js index 3669db20..b2c30373 100644 --- a/src/queries/label-text.js +++ b/src/queries/label-text.js @@ -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')) { @@ -51,21 +52,25 @@ function queryAllByLabelText( // // .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')) { // - return container.querySelector( - `[aria-labelledby~="${label.getAttribute('id')}"]`, - ) + container + .querySelectorAll(`[aria-labelledby~="${label.getAttribute('id')}"]`) + .forEach(element => elementsForLabel.push(element)) } if (label.childNodes.length) { // - 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, { @@ -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), + ) } // the getAll* query would normally look like this: From 7b41148b613fd26272ba1e5b545df8440a765734 Mon Sep 17 00:00:00 2001 From: airjp73 Date: Sun, 6 Oct 2019 10:25:30 -0400 Subject: [PATCH 3/8] test: Add test case for multiple labels with the same text Passes with no changes to production code --- src/__tests__/element-queries.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/__tests__/element-queries.js b/src/__tests__/element-queries.js index e8904770..516e0e72 100644 --- a/src/__tests__/element-queries.js +++ b/src/__tests__/element-queries.js @@ -242,6 +242,22 @@ test('can find input when label text is in a span', () => { expect(result[0].id).toBe('input1') }) +test('can find the correct element when there are multiple matching labels', () => { + const {getByLabelText} = render(` + + + `) + + const result = getByLabelText('Test Label', {selector: 'input'}) + expect(result.nodeName).toBe('INPUT') +}) + test('get can get form controls by placeholder', () => { const {getByPlaceholderText} = render(` , From 581c45140ee0f4c9c8b9c43b2175c546c7a52fc3 Mon Sep 17 00:00:00 2001 From: airjp73 Date: Sun, 6 Oct 2019 10:25:49 -0400 Subject: [PATCH 4/8] Use matches instead of performing another query --- src/queries/label-text.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/queries/label-text.js b/src/queries/label-text.js index b2c30373..466256e8 100644 --- a/src/queries/label-text.js +++ b/src/queries/label-text.js @@ -94,13 +94,9 @@ function queryAllByLabelText( [], ) - const allMatches = Array.from( + return Array.from( new Set([...labelledElements, ...ariaLabelledElements]), - ) - const elementsMatchingSelector = new Set(container.querySelectorAll(selector)) - return allMatches.filter(matchingElement => - elementsMatchingSelector.has(matchingElement), - ) + ).filter(matchingElement => matchingElement.matches(selector)) } // the getAll* query would normally look like this: From 696f30979472bae6059fb2cf46981111a5560299 Mon Sep 17 00:00:00 2001 From: airjp73 Date: Sun, 6 Oct 2019 10:42:18 -0400 Subject: [PATCH 5/8] Change variable name matchingElement.matches sounds a little weird --- src/queries/label-text.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/queries/label-text.js b/src/queries/label-text.js index 466256e8..a2f7c2da 100644 --- a/src/queries/label-text.js +++ b/src/queries/label-text.js @@ -96,7 +96,7 @@ function queryAllByLabelText( return Array.from( new Set([...labelledElements, ...ariaLabelledElements]), - ).filter(matchingElement => matchingElement.matches(selector)) + ).filter(element => element.matches(selector)) } // the getAll* query would normally look like this: From d7a9c2acff4e642032bbdaebee3be12332c0271a Mon Sep 17 00:00:00 2001 From: airjp73 Date: Mon, 7 Oct 2019 07:58:21 -0400 Subject: [PATCH 6/8] Tweak logic for getting label children --- src/__tests__/element-queries.js | 26 +++++++++++++++++++++----- src/queries/label-text.js | 2 +- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/__tests__/element-queries.js b/src/__tests__/element-queries.js index 516e0e72..032c8018 100644 --- a/src/__tests__/element-queries.js +++ b/src/__tests__/element-queries.js @@ -229,17 +229,33 @@ test('can filter results of label query based on selector', () => { expect(result[0].id).toBe('input1') }) -test('can find input when label text is in a span', () => { +test('can find input or textarea when label text is inside other elements', () => { const {getAllByLabelText} = render(`