fix: Don't recommend ByRole if role is generic (#664)

src/__tests__/element-queries.js

@@ -533,14 +533,14 @@ test('queryAllByRole returns semantic html elements', () => {
   expect(queryAllByRole(/listitem/i)).toHaveLength(3)
   // TODO: with https://github.com/A11yance/aria-query/pull/42
   // the actual value will match `toHaveLength(2)`
-  expect(queryAllByRole(/textbox/i)).toHaveLength(1)
+  expect(queryAllByRole(/textbox/i)).toHaveLength(2)
   expect(queryAllByRole(/checkbox/i)).toHaveLength(1)
   expect(queryAllByRole(/radio/i)).toHaveLength(1)
   expect(queryAllByRole('row')).toHaveLength(3)
   expect(queryAllByRole(/rowgroup/i)).toHaveLength(2)
   // TODO: with https://github.com/A11yance/aria-query/pull/42
   // the actual value will match `toHaveLength(3)`
-  expect(queryAllByRole(/(table)|(textbox)/i)).toHaveLength(2)
+  expect(queryAllByRole(/(table)|(textbox)/i)).toHaveLength(3)
   expect(queryAllByRole(/img/i)).toHaveLength(1)
   expect(queryAllByRole('meter')).toHaveLength(1)
   expect(queryAllByRole('progressbar')).toHaveLength(0)

src/__tests__/suggestions.js

@@ -74,7 +74,9 @@ test(`should not suggest if the suggestion would give different results`, () =>
 })
 
 test('should suggest by label over title', () => {
-  renderIntoDocument(`<label><span>bar</span><input title="foo" /></label>`)
+  renderIntoDocument(
+    `<label><span>bar</span><input type="password" title="foo" /></label>`,
+  )
 
   expect(() => screen.getByTitle('foo')).toThrowError(
     /getByLabelText\(\/bar\/i\)/,
@@ -181,7 +183,7 @@ test('escapes regular expressions in suggestion', () => {
 
 test('should suggest getByLabelText when no role available', () => {
   renderIntoDocument(
-    `<label for="foo">Username</label><input data-testid="foo" id="foo" />`,
+    `<label for="foo">Username</label><input type="password" data-testid="foo" id="foo" />`,
   )
   expect(() => screen.getByTestId('foo')).toThrowError(
     /getByLabelText\(\/username\/i\)/,
@@ -232,19 +234,21 @@ test.each([
 
 test(`should suggest label over placeholder text`, () => {
   renderIntoDocument(
-    `<label for="foo">Username</label><input id="foo" data-testid="foo" placeholder="Username" />`,
+    `<label for="foo">Password</label><input type="password" id="foo" data-testid="foo" placeholder="Password" />`,
   )
 
-  expect(() => screen.getByPlaceholderText('Username')).toThrowError(
-    /getByLabelText\(\/username\/i\)/,
+  expect(() => screen.getByPlaceholderText('Password')).toThrowError(
+    /getByLabelText\(\/password\/i\)/,
   )
 })
 
 test(`should suggest getByPlaceholderText`, () => {
-  renderIntoDocument(`<input data-testid="foo" placeholder="Username" />`)
+  renderIntoDocument(
+    `<input type="password" data-testid="foo" placeholder="Password" />`,
+  )
 
   expect(() => screen.getByTestId('foo')).toThrowError(
-    /getByPlaceholderText\(\/username\/i\)/,
+    /getByPlaceholderText\(\/password\/i\)/,
   )
 })
 
@@ -257,25 +261,27 @@ test(`should suggest getByText for simple elements`, () => {
 })
 
 test(`should suggest getByDisplayValue`, () => {
-  renderIntoDocument(`<input id="lastName" data-testid="lastName" />`)
+  renderIntoDocument(
+    `<input type="password" id="password" data-testid="password" />`,
+  )
 
-  document.getElementById('lastName').value = 'Prine' // RIP John Prine
+  document.getElementById('password').value = 'Prine' // RIP John Prine
 
-  expect(() => screen.getByTestId('lastName')).toThrowError(
+  expect(() => screen.getByTestId('password')).toThrowError(
     /getByDisplayValue\(\/prine\/i\)/,
   )
 })
 
 test(`should suggest getByAltText`, () => {
   renderIntoDocument(`
-    <input data-testid="input" alt="last name" />
+    <input type="password" data-testid="input" alt="password" />
     <map name="workmap">
       <area data-testid="area" shape="rect" coords="34,44,270,350" alt="Computer">
     </map>
     `)
 
   expect(() => screen.getByTestId('input')).toThrowError(
-    /getByAltText\(\/last name\/i\)/,
+    /getByAltText\(\/password\/i\)/,
   )
   expect(() => screen.getByTestId('area')).toThrowError(
     /getByAltText\(\/computer\/i\)/,

src/role-helpers.js

@@ -154,8 +154,9 @@ function getRoles(container, {hidden = false} = {}) {
 
 function prettyRoles(dom, {hidden}) {
   const roles = getRoles(dom, {hidden})
-
+  //We prefer to skip generic role, we don't reccomand it
   return Object.entries(roles)
+    .filter(([role]) => role !== 'generic')
     .map(([role, elements]) => {
       const delimiterBar = '-'.repeat(50)
       const elementsString = elements

src/suggestions.js

@@ -82,9 +82,10 @@ export function getSuggestedQuery(element, variant = 'get', method) {
     return undefined
   }
 
+  //We prefer to suggest something else if the role is generic
   const role =
     element.getAttribute('role') ?? getImplicitAriaRoles(element)?.[0]
-  if (canSuggest('Role', method, role)) {
+  if (role !== 'generic' && canSuggest('Role', method, role)) {
     return makeSuggestion('Role', role, {
       variant,
       name: computeAccessibleName(element),