feat: upgrade aria-query to 5.3.0 (#1241)

* feat: upgrade aria-query to 5.3.0 * fix: correctly handle img with empty alt * feat: pin `aria-query` version * chore: add accessibility-alt-text-bot (#1240) * chore: add accessibility-alt-text-bot * fix formatting issues * test: add coverage for footer to confirm it's expected implicit role as contentinfo --------- Co-authored-by: Sebastian Silbermann <[email protected]> Co-authored-by: Matan Borenkraout <[email protected]>
testing-library · Sep 14, 2023 · 2c57055 · 2c57055
1 parent 452097b
commit 2c57055
Show file tree

Hide file tree

Showing 7 changed files with 112 additions and 53 deletions.
diff --git a/.github/workflows/accessibility-alt-text.yml b/.github/workflows/accessibility-alt-text.yml
@@ -0,0 +1,28 @@
+name: Accessibility-alt-text-bot
+on:
+  issues:
+    types: [opened, edited]
+  pull_request:
+    types: [opened, edited]
+  issue_comment:
+    types: [created, edited]
+  discussion:
+    types: [created, edited]
+  discussion_comment:
+    types: [created, edited]
+
+permissions:
+  issues: write
+  pull-requests: write
+  discussions: write
+
+jobs:
+  accessibility_alt_text_bot:
+    name: Check alt text is set on issue or pull requests
+    runs-on: ubuntu-latest
+    if:
+      ${{ github.event.issue || github.event.pull_request ||
+      github.event.discussion }}
+    steps:
+      - name: Get action 'github/accessibility-alt-text-bot'
+        uses: github/[email protected] # Set to latest
diff --git a/package.json b/package.json
@@ -62,7 +62,7 @@
     "@babel/code-frame": "^7.10.4",
     "@babel/runtime": "^7.12.5",
     "@types/aria-query": "^5.0.1",
-    "aria-query": "5.1.3",
+    "aria-query": "5.3.0",
     "chalk": "^4.1.0",
     "dom-accessibility-api": "^0.5.9",
     "lz-string": "^1.5.0",

diff --git a/src/__tests__/__snapshots__/role-helpers.js.snap b/src/__tests__/__snapshots__/role-helpers.js.snap
@@ -52,32 +52,6 @@ Name "":
   data-testid="a-article"
 />
 
---------------------------------------------------
-command:
-
-Name "":
-<menuitem
-  data-testid="a-menuitem-1"
-/>
-
-Name "":
-<menuitem
-  data-testid="a-menuitem-2"
-/>
-
---------------------------------------------------
-menuitem:
-
-Name "":
-<menuitem
-  data-testid="a-menuitem-1"
-/>
-
-Name "":
-<menuitem
-  data-testid="a-menuitem-2"
-/>
-
 --------------------------------------------------
 list:
 
@@ -216,5 +190,31 @@ Name "":
   data-testid="a-dd"
 />
 
+--------------------------------------------------
+img:
+
+Name "":
+<img
+  data-testid="a-img-1"
+  src="http://example.com/image.png"
+/>
+
+Name "a meaningful description":
+<img
+  alt="a meaningful description"
+  data-testid="a-img-3"
+  src="http://example.com/image.png"
+/>
+
+--------------------------------------------------
+presentation:
+
+Name "":
+<img
+  alt=""
+  data-testid="a-img-2"
+  src="http://example.com/image.png"
+/>
+
 --------------------------------------------------
 `;
diff --git a/src/__tests__/role-helpers.js b/src/__tests__/role-helpers.js
@@ -19,7 +19,7 @@ function setup() {
 <header data-testid="a-header">Banner header</header>
 <section aria-label="a region" data-testid='named-section'>
   <a href="http://whatever.com" data-testid="a-link">link</a>
-  <a>invalid link</a>
+  <a data-testid="invalid-link">invalid link</a>
 
   <nav data-testid='a-nav' />
   
@@ -28,14 +28,6 @@ function setup() {
   <h3 data-testid='a-h3'>Tertiary Heading</h3>
 
   <article data-testid='a-article'>
-    <!-- menuitem is currently deprecated, but is the only 
-         tag currently that aria-query returns multiple roles for
-         (roles: command, menuitem).
-         It's used here in case a future tag also has multiple 
-         roles -->
-    <menuitem data-testid='a-menuitem-1'>1</menuitem> 
-    <menuitem data-testid='a-menuitem-2'>2</menuitem>
-
     <ul data-testid='a-list'>
       <li data-testid='a-list-item-1'>Item 1</li>
       <li data-testid='a-list-item-2'>Item 2</li>
@@ -66,12 +58,17 @@ function setup() {
 
     <form data-testid="a-form" />
     <section data-testid="a-section" />
-   </article>
-   <dl>
+  </article>
+  <dl>
     <dt data-testid="a-dt">Term</dt>
     <dd data-testid="a-dd">Definition</dd>
-   </dl>
+  </dl>
+
+  <img src="http://example.com/image.png" data-testid='a-img-1'/>
+  <img alt="" src="http://example.com/image.png" data-testid='a-img-2'/>
+  <img alt="a meaningful description" src="http://example.com/image.png" data-testid='a-img-3'/>
 </section>
+<footer data-testid="a-footer">Contentinfo footer</footer>
   `)
 
   return {
@@ -83,8 +80,6 @@ function setup() {
     h3: getByTestId('a-h3'),
     nav: getByTestId('a-nav'),
     article: getByTestId('a-article'),
-    menuItem: getByTestId('a-menuitem-1'),
-    menuItem2: getByTestId('a-menuitem-2'),
     aUl: getByTestId('a-list'),
     aLi1: getByTestId('a-list-item-1'),
     aLi2: getByTestId('a-list-item-2'),
@@ -107,6 +102,11 @@ function setup() {
     dt: getByTestId('a-dt'),
     dd: getByTestId('a-dd'),
     header: getByTestId('a-header'),
+    invalidAnchor: getByTestId('invalid-link'),
+    unnamedImg: getByTestId('a-img-1'),
+    presentationImg: getByTestId('a-img-2'),
+    namedImg: getByTestId('a-img-3'),
+    footer: getByTestId('a-footer'),
   }
 }
 
@@ -118,8 +118,6 @@ test('getRoles returns expected roles for various dom nodes', () => {
     h3,
     nav,
     article,
-    menuItem,
-    menuItem2,
     aUl,
     aLi1,
     aLi2,
@@ -142,6 +140,12 @@ test('getRoles returns expected roles for various dom nodes', () => {
     dd,
     dt,
     header,
+    invalidAnchor,
+    unnamedSection,
+    unnamedImg,
+    presentationImg,
+    namedImg,
+    footer,
   } = setup()
 
   expect(getRoles(namedSection)).toEqual({
@@ -157,16 +161,20 @@ test('getRoles returns expected roles for various dom nodes', () => {
     cell: [td1, td2, td3],
     textbox: [input, input2, textarea],
     rowgroup: [tbody],
-    command: [menuItem, menuItem2],
-    menuitem: [menuItem, menuItem2],
     form: [namedForm],
     region: [namedSection],
     term: [dt],
     definition: [dd],
+    generic: [invalidAnchor, unnamedSection],
+    img: [unnamedImg, namedImg],
+    presentation: [presentationImg],
   })
   expect(getRoles(header)).toEqual({
     banner: [header],
   })
+  expect(getRoles(footer)).toEqual({
+    contentinfo: [footer],
+  })
 })
 
 test('logRoles calls console.log with output from prettyRoles', () => {

diff --git a/src/__tests__/role.js b/src/__tests__/role.js
@@ -248,6 +248,12 @@ test('accessible name comparison is case sensitive', () => {
       <h1 />
 
       --------------------------------------------------
+      emphasis:
+
+      Name "":
+      <em />
+
+      --------------------------------------------------
 
     Ignored nodes: comments, script, style
     <div>
@@ -296,6 +302,12 @@ test('TextMatch serialization in error message', () => {
       <h1 />
 
       --------------------------------------------------
+      emphasis:
+
+      Name "":
+      <em />
+
+      --------------------------------------------------
 
     Ignored nodes: comments, script, style
     <div>
@@ -320,6 +332,12 @@ test('TextMatch serialization in error message', () => {
       <h1 />
 
       --------------------------------------------------
+      emphasis:
+
+      Name "":
+      <em />
+
+      --------------------------------------------------
 
     Ignored nodes: comments, script, style
     <div>

diff --git a/src/__tests__/suggestions.js b/src/__tests__/suggestions.js
@@ -196,9 +196,9 @@ test('escapes regular expressions in suggestion', () => {
   const {container} = renderIntoDocument(`
       <label for="superInput">inp-t lab^l w{th c+ars to esc\\pe</label>
       <input id="superInput" type="text" value="my super string +-('{}^$)" placeholder="should escape +-'(/" />
-      <p>
+      <span>
         Loading ... (1)
-      </p>
+      </span>
       <img src="foo.png" alt="The Problem (picture of a question mark)" data-testid="foo" />
     `)
 
@@ -214,7 +214,7 @@ test('escapes regular expressions in suggestion', () => {
     ).toString(),
   ).toEqual(`getByAltText(/the problem \\(picture of a question mark\\)/i)`)
 
-  expect(getSuggestedQuery(container.querySelector('p')).toString()).toEqual(
+  expect(getSuggestedQuery(container.querySelector('span')).toString()).toEqual(
     `getByText(/loading \\.\\.\\. \\(1\\)/i)`,
   )
 

diff --git a/src/role-helpers.js b/src/role-helpers.js
@@ -82,13 +82,18 @@ function buildElementRoleList(elementRolesMap) {
     return `${name}${attributes
       .map(({name: attributeName, value, constraints = []}) => {
         const shouldNotExist = constraints.indexOf('undefined') !== -1
-        if (shouldNotExist) {
-          return `:not([${attributeName}])`
-        } else if (value) {
+        const shouldBeNonEmpty = constraints.indexOf('set') !== -1
+        const hasExplicitValue = typeof value !== 'undefined'
+
+        if (hasExplicitValue) {
           return `[${attributeName}="${value}"]`
-        } else {
-          return `[${attributeName}]`
+        } else if (shouldNotExist) {
+          return `:not([${attributeName}])`
+        } else if (shouldBeNonEmpty) {
+          return `[${attributeName}]:not([${attributeName}=""])`
         }
+
+        return `[${attributeName}]`
       })
       .join('')}`
   }