From dd60cdd31c58da3252754f8fbc21f0a3217e1773 Mon Sep 17 00:00:00 2001
From: Ben Monro <ben.monro@gmail.com>
Date: Tue, 17 Nov 2020 12:06:47 -0800
Subject: [PATCH] fix(suggestions): only warn about inaccessible elements when
 actually showing the suggestion

---
 src/__tests__/suggestions.js | 39 +++++++++++++++++++++++++++++++-----
 src/suggestions.js           |  4 +++-
 2 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/src/__tests__/suggestions.js b/src/__tests__/suggestions.js
index 318917a2..5cbb94a0 100644
--- a/src/__tests__/suggestions.js
+++ b/src/__tests__/suggestions.js
@@ -551,16 +551,44 @@ test('should get the first label with aria-labelledby contains multiple ids', ()
   })
 })
 
-test('should suggest hidden option if element is not in the accessibilty tree', () => {
+test('should not suggest or warn about hidden element when suggested query is already used.', () => {
   console.warn.mockImplementation(() => {})
 
-  const {container} = renderIntoDocument(`
+  renderIntoDocument(`
     <input type="text" aria-hidden=true />
   `)
 
-  expect(
-    getSuggestedQuery(container.querySelector('input'), 'get', 'role'),
-  ).toMatchObject({
+  expect(() => screen.getByRole('textbox', {hidden: true})).not.toThrowError()
+  expect(console.warn).not.toHaveBeenCalled()
+})
+test('should suggest and warn about if element is not in the accessibility tree', () => {
+  console.warn.mockImplementation(() => {})
+
+  renderIntoDocument(`
+    <input type="text" data-testid="foo" aria-hidden=true />
+  `)
+
+  expect(() => screen.getByTestId('foo', {hidden: true})).toThrowError(
+    /getByRole\('textbox', \{ hidden: true \}\)/,
+  )
+  expect(console.warn).toHaveBeenCalledWith(
+    expect.stringContaining(`Element is inaccessible.`),
+  )
+})
+
+test('should suggest hidden option if element is not in the accessibility tree', () => {
+  console.warn.mockImplementation(() => {})
+
+  const {container} = renderIntoDocument(`
+    <input type="text" data-testid="foo" aria-hidden=true />
+  `)
+
+  const suggestion = getSuggestedQuery(
+    container.querySelector('input'),
+    'get',
+    'role',
+  )
+  expect(suggestion).toMatchObject({
     queryName: 'Role',
     queryMethod: 'getByRole',
     queryArgs: ['textbox', {hidden: true}],
@@ -569,6 +597,7 @@ test('should suggest hidden option if element is not in the accessibilty tree',
     If you are using the aria-hidden prop, make sure this is the right choice for your case.
     `,
   })
+  suggestion.toString()
 
   expect(console.warn.mock.calls).toMatchInlineSnapshot(`
     Array [
diff --git a/src/suggestions.js b/src/suggestions.js
index b3725069..ba5a1f29 100644
--- a/src/suggestions.js
+++ b/src/suggestions.js
@@ -33,7 +33,6 @@ function makeSuggestion(queryName, element, content, {variant, name}) {
     warning = `Element is inaccessible. This means that the element and all its children are invisible to screen readers.
     If you are using the aria-hidden prop, make sure this is the right choice for your case.
     `
-    console.warn(warning)
   }
   if (Object.keys(queryOptions).length > 0) {
     queryArgs.push(queryOptions)
@@ -48,6 +47,9 @@ function makeSuggestion(queryName, element, content, {variant, name}) {
     variant,
     warning,
     toString() {
+      if (warning) {
+        console.warn(warning)
+      }
       let [text, options] = queryArgs
 
       text = typeof text === 'string' ? `'${text}'` : text