From 55327610c73c96c10b7ee5151afd3b7d7af3368f Mon Sep 17 00:00:00 2001
From: amandaesmith3 <amanda.s@ionic.io>
Date: Tue, 23 Aug 2022 10:21:54 -0500
Subject: [PATCH 1/3] fix(alert): use aria-labelledby and aria-describedby
 instead of aria-label

---
 core/src/components/alert/alert.tsx           | 14 +++++--
 .../components/alert/test/a11y/alert.e2e.ts   | 42 +++++++++++++------
 .../src/components/alert/test/a11y/index.html | 38 ++++++++++-------
 3 files changed, 62 insertions(+), 32 deletions(-)

diff --git a/core/src/components/alert/alert.tsx b/core/src/components/alert/alert.tsx
index af023a167de..33b729b6fa2 100644
--- a/core/src/components/alert/alert.tsx
+++ b/core/src/components/alert/alert.tsx
@@ -578,20 +578,26 @@ export class Alert implements ComponentInterface, OverlayInterface {
   }
 
   render() {
-    const { overlayIndex, header, subHeader, htmlAttributes } = this;
+    const { overlayIndex, header, subHeader, message, htmlAttributes } = this;
     const mode = getIonMode(this);
     const hdrId = `alert-${overlayIndex}-hdr`;
     const subHdrId = `alert-${overlayIndex}-sub-hdr`;
     const msgId = `alert-${overlayIndex}-msg`;
     const role = this.inputs.length > 0 || this.buttons.length > 0 ? 'alertdialog' : 'alert';
-    const defaultAriaLabel = header || subHeader || 'Alert';
+
+    /**
+     * If the header is defined, use that. Otherwise, fall back to the subHeader.
+     * If neither is defined, don't set aria-labelled by.
+     */
+    const ariaLabelledBy = header ? hdrId : subHeader ? subHdrId : null;
 
     return (
       <Host
         role={role}
         aria-modal="true"
+        aria-labelledby={ariaLabelledBy}
+        aria-describedby={message ? msgId : null}
         tabindex="-1"
-        aria-label={defaultAriaLabel}
         {...(htmlAttributes as any)}
         style={{
           zIndex: `${20000 + overlayIndex}`,
@@ -623,7 +629,7 @@ export class Alert implements ComponentInterface, OverlayInterface {
             )}
           </div>
 
-          <div id={msgId} class="alert-message" innerHTML={sanitizeDOMString(this.message)}></div>
+          <div id={msgId} class="alert-message" innerHTML={sanitizeDOMString(message)}></div>
 
           {this.renderAlertInputs()}
           {this.renderAlertButtons()}
diff --git a/core/src/components/alert/test/a11y/alert.e2e.ts b/core/src/components/alert/test/a11y/alert.e2e.ts
index 74746af7cc7..645a7e735bf 100644
--- a/core/src/components/alert/test/a11y/alert.e2e.ts
+++ b/core/src/components/alert/test/a11y/alert.e2e.ts
@@ -3,12 +3,30 @@ import { expect } from '@playwright/test';
 import type { E2EPage } from '@utils/test/playwright';
 import { test } from '@utils/test/playwright';
 
-const testAriaLabel = async (page: E2EPage, buttonID: string, expectedAriaLabel: string) => {
+const testAria = async (
+  page: E2EPage,
+  buttonID: string,
+  expectedAriaLabelledBy: string | null,
+  expectedAriaDescribedBy: string | null
+) => {
   const button = page.locator(`#${buttonID}`);
   await button.click();
 
   const alert = page.locator('ion-alert');
-  await expect(alert).toHaveAttribute('aria-label', expectedAriaLabel);
+
+  /**
+   * expect().toHaveAttribute() can't check for a null value, so grab and check
+   * the values manually instead.
+   */
+  const { ariaLabelledBy, ariaDescribedBy } = await alert.evaluate((el: HTMLIonAlertElement) => {
+    return {
+      ariaLabelledBy: el.getAttribute('aria-labelledby'),
+      ariaDescribedBy: el.getAttribute('aria-describedby'),
+    };
+  });
+
+  expect(ariaLabelledBy).toBe(expectedAriaLabelledBy);
+  expect(ariaDescribedBy).toBe(expectedAriaDescribedBy);
 };
 
 test.describe('alert: a11y', () => {
@@ -17,27 +35,27 @@ test.describe('alert: a11y', () => {
     await page.goto(`/src/components/alert/test/a11y`);
   });
 
-  test('should not have accessibility violations', async ({ page }) => {
-    const button = page.locator('#customHeader');
+  test('should not have accessibility violations when header and message are defined', async ({ page }) => {
+    const button = page.locator('#bothHeaders');
     await button.click();
 
     const results = await new AxeBuilder({ page }).analyze();
     expect(results.violations).toEqual([]);
   });
 
-  test('should have fallback aria-label when no header or subheader is specified', async ({ page }) => {
-    await testAriaLabel(page, 'noHeader', 'Alert');
+  test('should have aria-labelledby when header is set', async ({ page }) => {
+    await testAria(page, 'noMessage', 'alert-1-hdr', null);
   });
 
-  test('should inherit aria-label from header', async ({ page }) => {
-    await testAriaLabel(page, 'customHeader', 'Header');
+  test('should have aria-describedby when message is set', async ({ page }) => {
+    await testAria(page, 'noHeaders', null, 'alert-1-msg');
   });
 
-  test('should inherit aria-label from subheader if no header is specified', async ({ page }) => {
-    await testAriaLabel(page, 'subHeaderOnly', 'Subtitle');
+  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 allow for manually specifying aria-label', async ({ page }) => {
-    await testAriaLabel(page, 'customAriaLabel', 'Custom alert');
+  test('should allow for manually specifying aria attributes', async ({ page }) => {
+    await testAria(page, 'customAria', 'Custom title', 'Custom description');
   });
 });
diff --git a/core/src/components/alert/test/a11y/index.html b/core/src/components/alert/test/a11y/index.html
index a457eef9e2f..b73fdae7f7f 100644
--- a/core/src/components/alert/test/a11y/index.html
+++ b/core/src/components/alert/test/a11y/index.html
@@ -19,14 +19,11 @@
     <main class="ion-padding">
       <h1>Alert - A11y</h1>
 
-      <ion-button id="noHeader" expand="block" onclick="presentNoHeader()">Alert With No Header</ion-button>
-      <ion-button id="customHeader" expand="block" onclick="presentCustomHeader()">Alert With Custom Header</ion-button>
-      <ion-button id="subHeaderOnly" expand="block" onclick="presentSubHeaderOnly()"
-        >Alert With Subheader Only</ion-button
-      >
-      <ion-button id="customAriaLabel" expand="block" onclick="presentCustomAriaLabel()"
-        >Alert With Custom Aria Label</ion-button
-      >
+      <ion-button id="bothHeaders" expand="block" onclick="presentBothHeaders()">Both Headers</ion-button>
+      <ion-button id="subHeaderOnly" expand="block" onclick="presentSubHeaderOnly()">Subheader Only</ion-button>
+      <ion-button id="noHeaders" expand="block" onclick="presentNoHeaders()">No Headers</ion-button>
+      <ion-button id="noMessage" expand="block" onclick="presentNoMessage()">No Message</ion-button>
+      <ion-button id="customAria" expand="block" onclick="presentCustomAria()">Custom Aria</ion-button>
     </main>
 
     <script>
@@ -35,38 +32,47 @@ <h1>Alert - A11y</h1>
         await alert.present();
       }
 
-      function presentNoHeader() {
+      function presentBothHeaders() {
         openAlert({
+          header: 'Header',
+          subHeader: 'Subtitle',
           message: 'This is an alert message.',
           buttons: ['OK'],
         });
       }
 
-      function presentCustomHeader() {
+      function presentSubHeaderOnly() {
         openAlert({
-          header: 'Header',
           subHeader: 'Subtitle',
           message: 'This is an alert message.',
           buttons: ['OK'],
         });
       }
 
-      function presentSubHeaderOnly() {
+      function presentNoHeaders() {
         openAlert({
-          subHeader: 'Subtitle',
           message: 'This is an alert message.',
           buttons: ['OK'],
         });
       }
 
-      function presentCustomAriaLabel() {
+      function presentNoMessage() {
+        openAlert({
+          header: 'Header',
+          subHeader: 'Subtitle',
+          buttons: ['OK'],
+        });
+      }
+
+      function presentCustomAria() {
         openAlert({
           header: 'Header',
           subHeader: 'Subtitle',
-          message: 'This is an alert message with a custom aria-label.',
+          message: 'This is an alert message with custom aria attributes.',
           buttons: ['OK'],
           htmlAttributes: {
-            'aria-label': 'Custom alert',
+            'aria-labelledby': 'Custom title',
+            'aria-describedby': 'Custom description',
           },
         });
       }

From 47e87d30d83c0e1c7c7599a8fe6097efb736fbd0 Mon Sep 17 00:00:00 2001
From: amandaesmith3 <amanda.s@ionic.io>
Date: Tue, 23 Aug 2022 11:00:30 -0500
Subject: [PATCH 2/3] test(alert): improve test flakiness

---
 core/src/components/alert/test/a11y/alert.e2e.ts | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/core/src/components/alert/test/a11y/alert.e2e.ts b/core/src/components/alert/test/a11y/alert.e2e.ts
index 645a7e735bf..692a6690739 100644
--- a/core/src/components/alert/test/a11y/alert.e2e.ts
+++ b/core/src/components/alert/test/a11y/alert.e2e.ts
@@ -9,8 +9,11 @@ const testAria = async (
   expectedAriaLabelledBy: string | null,
   expectedAriaDescribedBy: string | null
 ) => {
+  const didPresent = await page.spyOnEvent('ionAlertDidPresent');
   const button = page.locator(`#${buttonID}`);
+
   await button.click();
+  await didPresent.next();
 
   const alert = page.locator('ion-alert');
 
@@ -18,12 +21,8 @@ const testAria = async (
    * expect().toHaveAttribute() can't check for a null value, so grab and check
    * the values manually instead.
    */
-  const { ariaLabelledBy, ariaDescribedBy } = await alert.evaluate((el: HTMLIonAlertElement) => {
-    return {
-      ariaLabelledBy: el.getAttribute('aria-labelledby'),
-      ariaDescribedBy: el.getAttribute('aria-describedby'),
-    };
-  });
+  const ariaLabelledBy = await alert.getAttribute('aria-labelledby');
+  const ariaDescribedBy = await alert.getAttribute('aria-describedby');
 
   expect(ariaLabelledBy).toBe(expectedAriaLabelledBy);
   expect(ariaDescribedBy).toBe(expectedAriaDescribedBy);

From 5be0596a3ebac3034af73ee274af38f17a04866e Mon Sep 17 00:00:00 2001
From: Amanda Johnston <90629384+amandaejohnston@users.noreply.github.com>
Date: Tue, 23 Aug 2022 11:23:01 -0500
Subject: [PATCH 3/3] Update core/src/components/alert/alert.tsx

Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
---
 core/src/components/alert/alert.tsx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/src/components/alert/alert.tsx b/core/src/components/alert/alert.tsx
index 33b729b6fa2..95c30e3ec30 100644
--- a/core/src/components/alert/alert.tsx
+++ b/core/src/components/alert/alert.tsx
@@ -587,7 +587,7 @@ export class Alert implements ComponentInterface, OverlayInterface {
 
     /**
      * If the header is defined, use that. Otherwise, fall back to the subHeader.
-     * If neither is defined, don't set aria-labelled by.
+     * If neither is defined, don't set aria-labelledby.
      */
     const ariaLabelledBy = header ? hdrId : subHeader ? subHdrId : null;