fix(alert): use correct heading structure for subHeader when header e…

…xists (#29964) - The `header` will continue to always render as an `<h2>` element. - If there is no `header` defined, the `subHeader` will continue to render as an `<h2>` element. - If there is a `header` defined, the `subHeader` will render as an `<h3>` element. - Updates `ariaLabelledBy` to include both `header` and `subHeader` ids when both are defined. - Updates the `a11y` e2e test to use new values & check for tag names.
ionic-team · Nov 4, 2024 · 0fdcb32 · 0fdcb32
1 parent ee2fa19
commit 0fdcb32
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 14 deletions.
diff --git a/core/src/components/alert/alert.tsx b/core/src/components/alert/alert.tsx
@@ -730,10 +730,12 @@ export class Alert implements ComponentInterface, OverlayInterface {
     const role = this.inputs.length > 0 || this.buttons.length > 0 ? 'alertdialog' : 'alert';
 
     /**
-     * If the header is defined, use that. Otherwise, fall back to the subHeader.
-     * If neither is defined, don't set aria-labelledby.
+     * Use both the header and subHeader ids if they are defined.
+     * If only the header is defined, use the header id.
+     * If only the subHeader is defined, use the subHeader id.
+     * If neither are defined, do not set aria-labelledby.
      */
-    const ariaLabelledBy = header ? hdrId : subHeader ? subHdrId : null;
+    const ariaLabelledBy = header && subHeader ? `${hdrId} ${subHdrId}` : header ? hdrId : subHeader ? subHdrId : null;
 
     return (
       <Host
@@ -766,11 +768,18 @@ export class Alert implements ComponentInterface, OverlayInterface {
                 {header}
               </h2>
             )}
-            {subHeader && (
+            {/* If no header exists, subHeader should be the highest heading level, h2 */}
+            {subHeader && !header && (
               <h2 id={subHdrId} class="alert-sub-title">
                 {subHeader}
               </h2>
             )}
+            {/* If a header exists, subHeader should be one level below it, h3 */}
+            {subHeader && header && (
+              <h3 id={subHdrId} class="alert-sub-title">
+                {subHeader}
+              </h3>
+            )}
           </div>
 
           {this.renderAlertMessage(msgId)}

diff --git a/core/src/components/alert/test/a11y/alert.e2e.ts b/core/src/components/alert/test/a11y/alert.e2e.ts
@@ -17,6 +17,27 @@ const testAria = async (
 
   const alert = page.locator('ion-alert');
 
+  const header = alert.locator('.alert-title');
+  const subHeader = alert.locator('.alert-sub-title');
+
+  // If a header exists, it should be an h2 element
+  if ((await header.count()) > 0) {
+    const headerTagName = await header.evaluate((el) => el.tagName);
+    expect(headerTagName).toBe('H2');
+  }
+
+  // If a header and subHeader exist, the subHeader should be an h3 element
+  if ((await header.count()) > 0 && (await subHeader.count()) > 0) {
+    const subHeaderTagName = await subHeader.evaluate((el) => el.tagName);
+    expect(subHeaderTagName).toBe('H3');
+  }
+
+  // If a subHeader exists without a header, the subHeader should be an h2 element
+  if ((await header.count()) === 0 && (await subHeader.count()) > 0) {
+    const subHeaderTagName = await subHeader.evaluate((el) => el.tagName);
+    expect(subHeaderTagName).toBe('H2');
+  }
+
   /**
    * expect().toHaveAttribute() can't check for a null value, so grab and check
    * the values manually instead.
@@ -124,16 +145,24 @@ configs({ directions: ['ltr'] }).forEach(({ config, title }) => {
       await page.goto(`/src/components/alert/test/a11y`, config);
     });
 
-    test('should have aria-labelledby when header is set', async ({ page }) => {
-      await testAria(page, 'noMessage', 'alert-1-hdr', null);
+    test('should have aria-labelledby set to both when header and subHeader are set', async ({ page }) => {
+      await testAria(page, 'bothHeadersOnly', 'alert-1-hdr alert-1-sub-hdr', null);
+    });
+
+    test('should have aria-labelledby set when only header is set', async ({ page }) => {
+      await testAria(page, 'headerOnly', 'alert-1-hdr', null);
+    });
+
+    test('should fall back to subHeader for aria-labelledby if header is not defined', async ({ page }) => {
+      await testAria(page, 'subHeaderOnly', 'alert-1-sub-hdr', null);
     });
 
     test('should have aria-describedby when message is set', async ({ page }) => {
       await testAria(page, 'noHeaders', null, 'alert-1-msg');
     });
 
-    test('should fall back to subHeader for aria-labelledby if header is not defined', async ({ page }) => {
-      await testAria(page, 'subHeaderOnly', 'alert-1-sub-hdr', 'alert-1-msg');
+    test('should have aria-labelledby and aria-describedby when headers and message are set', async ({ page }) => {
+      await testAria(page, 'headersAndMessage', 'alert-1-hdr alert-1-sub-hdr', 'alert-1-msg');
     });
 
     test('should allow for manually specifying aria attributes', async ({ page }) => {

diff --git a/core/src/components/alert/test/a11y/index.html b/core/src/components/alert/test/a11y/index.html
@@ -19,10 +19,11 @@
     <main class="ion-padding">
       <h1>Alert - A11y</h1>
 
-      <button class="expand" id="bothHeaders" onclick="presentBothHeaders()">Both Headers</button>
+      <button class="expand" id="bothHeadersOnly" onclick="presentBothHeadersOnly()">Both Headers Only</button>
+      <button class="expand" id="headerOnly" onclick="presentHeaderOnly()">Header Only</button>
       <button class="expand" id="subHeaderOnly" onclick="presentSubHeaderOnly()">Subheader Only</button>
       <button class="expand" id="noHeaders" onclick="presentNoHeaders()">No Headers</button>
-      <button class="expand" id="noMessage" onclick="presentNoMessage()">No Message</button>
+      <button class="expand" id="headersAndMessage" onclick="presentHeadersAndMessage()">Headers and Message</button>
       <button class="expand" id="customAria" onclick="presentCustomAria()">Custom Aria</button>
       <button class="expand" id="ariaLabelButton" onclick="presentAriaLabelButton()">Aria Label Button</button>
       <button class="expand" id="checkbox" onclick="presentAlertCheckbox()">Checkbox</button>
@@ -34,19 +35,24 @@ <h1>Alert - A11y</h1>
         await alert.present();
       }
 
-      function presentBothHeaders() {
+      function presentBothHeadersOnly() {
         openAlert({
           header: 'Header',
           subHeader: 'Subtitle',
-          message: 'This is an alert message.',
+          buttons: ['OK'],
+        });
+      }
+
+      function presentHeaderOnly() {
+        openAlert({
+          header: 'Header',
           buttons: ['OK'],
         });
       }
 
       function presentSubHeaderOnly() {
         openAlert({
           subHeader: 'Subtitle',
-          message: 'This is an alert message.',
           buttons: ['OK'],
         });
       }
@@ -58,10 +64,11 @@ <h1>Alert - A11y</h1>
         });
       }
 
-      function presentNoMessage() {
+      function presentHeadersAndMessage() {
         openAlert({
           header: 'Header',
           subHeader: 'Subtitle',
+          message: 'This is an alert message.',
           buttons: ['OK'],
         });
       }