Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(alert): use correct heading structure for subHeader when header exists #29964

Merged
merged 10 commits into from
Oct 25, 2024
17 changes: 13 additions & 4 deletions core/src/components/alert/alert.tsx
brandyscarney marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chained ternary expressions like this are so gross 🤮


return (
<Host
Expand Down Expand Up @@ -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)}
Expand Down
37 changes: 33 additions & 4 deletions core/src/components/alert/test/a11y/alert.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 }) => {
Expand Down
19 changes: 13 additions & 6 deletions core/src/components/alert/test/a11y/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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'],
});
}
Expand All @@ -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'],
});
}
Expand Down
Loading