Merge branch 'master' into pr/add-config-fields

Author: thepelkus-too

.all-contributorsrc

@@ -1145,6 +1145,16 @@
         "code",
         "test"
       ]
+    },
+    {
+      "login": "ipap360",
+      "name": "Ioannis Papadopoulos",
+      "avatar_url": "https://avatars2.githubusercontent.com/u/11017666?v=4",
+      "profile": "https://github.com/ipap360",
+      "contributions": [
+        "code",
+        "test"
+      ]
     }
   ],
   "repoHost": "https://github.com"

README.md

@@ -281,6 +281,7 @@ Thanks goes to these people ([emoji key][emojis]):
     <td align="center"><a href="https://www.linkedin.com/in/deniz-susman-92b40a145/"><img src="https://avatars1.githubusercontent.com/u/39295979?v=4" width="100px;" alt=""/><br /><sub><b>Deniz Susman</b></sub></a><br /><a href="https://github.com/testing-library/dom-testing-library/commits?author=DenrizSusam" title="Code">💻</a></td>
     <td align="center"><a href="https://github.com/delca85"><img src="https://avatars1.githubusercontent.com/u/4076043?v=4" width="100px;" alt=""/><br /><sub><b>Bianca Del Carretto</b></sub></a><br /><a href="https://github.com/testing-library/dom-testing-library/commits?author=delca85" title="Tests">⚠️</a> <a href="https://github.com/testing-library/dom-testing-library/commits?author=delca85" title="Code">💻</a></td>
     <td align="center"><a href="https://www.linkedin.com/in/joshlalonde3/"><img src="https://avatars3.githubusercontent.com/u/9097492?v=4" width="100px;" alt=""/><br /><sub><b>Josh Lalonde</b></sub></a><br /><a href="https://github.com/testing-library/dom-testing-library/commits?author=ryuuji3" title="Code">💻</a> <a href="https://github.com/testing-library/dom-testing-library/commits?author=ryuuji3" title="Tests">⚠️</a></td>
+    <td align="center"><a href="https://github.com/ipap360"><img src="https://avatars2.githubusercontent.com/u/11017666?v=4" width="100px;" alt=""/><br /><sub><b>Ioannis Papadopoulos</b></sub></a><br /><a href="https://github.com/testing-library/dom-testing-library/commits?author=ipap360" title="Code">💻</a> <a href="https://github.com/testing-library/dom-testing-library/commits?author=ipap360" title="Tests">⚠️</a></td>
   </tr>
 </table>
 

src/__tests__/element-queries.js

@@ -352,17 +352,19 @@ test('label with no form control', () => {
 `)
 })
 
-test('label with no form control and fuzzy matcher', () => {
+test('label with "for" attribute but no form control and fuzzy matcher', () => {
   const {getByLabelText, queryByLabelText} = render(
-    `<label>All alone label</label>`,
+    `<label for="foo">All alone label</label>`,
   )
   expect(queryByLabelText('alone', {exact: false})).toBeNull()
   expect(() => getByLabelText('alone', {exact: false}))
     .toThrowErrorMatchingInlineSnapshot(`
 "Found a label with the text of: alone, however no form control was found associated to that label. Make sure you're using the "for" attribute or "aria-labelledby" attribute correctly.
 
 <div>
-  <label>
+  <label
+    for="foo"
+  >
     All alone label
   </label>
 </div>"
@@ -407,6 +409,114 @@ test('label with children with no form control', () => {
 `)
 })
 
+test('label with non-labellable element', () => {
+  const {getByLabelText, queryByLabelText} = render(`
+  <div>
+    <label for="div1">Label 1</label>
+    <div id="div1">
+      Hello
+    </div>
+  </div>
+  `)
+
+  expect(queryByLabelText(/Label/)).toBeNull()
+  expect(() => getByLabelText(/Label/)).toThrowErrorMatchingInlineSnapshot(`
+"Found a label with the text of: /Label/, however the element associated with this label (<div />) is non-labellable [https://html.spec.whatwg.org/multipage/forms.html#category-label]. If you really need to label a <div />, you can use aria-label or aria-labelledby instead.
+
+<div>
+  
+  
+  <div>
+    
+    
+    <label
+      for="div1"
+    >
+      Label 1
+    </label>
+    
+    
+    <div
+      id="div1"
+    >
+      
+      Hello
+    
+    </div>
+    
+  
+  </div>
+  
+  
+</div>"
+`)
+})
+
+test('multiple labels with non-labellable elements', () => {
+  const {getAllByLabelText, queryAllByLabelText} = render(`
+  <div>
+    <label for="span1">Label 1</label>
+    <span id="span1">
+      Hello
+    </span>
+    <label for="p1">Label 2</label>
+    <p id="p1">
+      World
+    </p>
+  </div>
+  `)
+
+  expect(queryAllByLabelText(/Label/)).toEqual([])
+  expect(() => getAllByLabelText(/Label/)).toThrowErrorMatchingInlineSnapshot(`
+"Found a label with the text of: /Label/, however the element associated with this label (<span />) is non-labellable [https://html.spec.whatwg.org/multipage/forms.html#category-label]. If you really need to label a <span />, you can use aria-label or aria-labelledby instead.
+
+Found a label with the text of: /Label/, however the element associated with this label (<p />) is non-labellable [https://html.spec.whatwg.org/multipage/forms.html#category-label]. If you really need to label a <p />, you can use aria-label or aria-labelledby instead.
+
+<div>
+  
+  
+  <div>
+    
+    
+    <label
+      for="span1"
+    >
+      Label 1
+    </label>
+    
+    
+    <span
+      id="span1"
+    >
+      
+      Hello
+    
+    </span>
+    
+    
+    <label
+      for="p1"
+    >
+      Label 2
+    </label>
+    
+    
+    <p
+      id="p1"
+    >
+      
+      World
+    
+    </p>
+    
+  
+  </div>
+  
+  
+</div>"
+`)
+})
+
 test('totally empty label', () => {
   const {getByLabelText, queryByLabelText} = render(`<label />`)
   expect(queryByLabelText('')).toBeNull()

src/queries/label-text.js

@@ -149,10 +149,27 @@ const getAllByLabelText = (container, text, ...rest) => {
   if (!els.length) {
     const labels = queryAllLabelsByText(container, text, ...rest)
     if (labels.length) {
-      throw getConfig().getElementError(
-        `Found a label with the text of: ${text}, however no form control was found associated to that label. Make sure you're using the "for" attribute or "aria-labelledby" attribute correctly.`,
-        container,
-      )
+      const tagNames = labels
+        .map(label =>
+          getTagNameOfElementAssociatedWithLabelViaFor(container, label),
+        )
+        .filter(tagName => !!tagName)
+      if (tagNames.length) {
+        throw getConfig().getElementError(
+          tagNames
+            .map(
+              tagName =>
+                `Found a label with the text of: ${text}, however the element associated with this label (<${tagName} />) is non-labellable [https://html.spec.whatwg.org/multipage/forms.html#category-label]. If you really need to label a <${tagName} />, you can use aria-label or aria-labelledby instead.`,
+            )
+            .join('\n\n'),
+          container,
+        )
+      } else {
+        throw getConfig().getElementError(
+          `Found a label with the text of: ${text}, however no form control was found associated to that label. Make sure you're using the "for" attribute or "aria-labelledby" attribute correctly.`,
+          container,
+        )
+      }
     } else {
       throw getConfig().getElementError(
         `Unable to find a label with the text of: ${text}`,
@@ -163,6 +180,16 @@ const getAllByLabelText = (container, text, ...rest) => {
   return els
 }
 
+function getTagNameOfElementAssociatedWithLabelViaFor(container, label) {
+  const htmlFor = label.getAttribute('for')
+  if (!htmlFor) {
+    return null
+  }
+
+  const element = container.querySelector(`[id="${htmlFor}"]`)
+  return element ? element.tagName.toLowerCase() : null
+}
+
 // the reason mentioned above is the same reason we're not using buildQueries
 const getMultipleError = (c, text) =>
   `Found multiple elements with the text of: ${text}`