Skip to content

Commit 12084af

Browse files
authored
fix(header): ensure one banner role in condensed header (#30718)
Issue number: internal --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> As per accessibility guidelines, there should only be one banner landmark per page. A condensed header creates two banner landmarks since 2 `ion-header` components are required on the page. `ion-header` renders with a `role="banner"` by default (when not in `ion-menu`). The visual effect of the condensed header is achieved by rendering two distinct header components. Because both components exist in the code at the same time and both have `role="banner"`, they create a duplicate landmark announcement for screen readers. This leads to a violation with the accessibility guidelines. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - The role is updated to either `none` or `banner` based off the header's active state. - Added test. ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> [Preview](https://ionic-framework-git-fw-6767-ionic1.vercel.app/src/components/header/test/condense/?ionic%3Amode=ios)
1 parent add33c5 commit 12084af

File tree

3 files changed

+81
-1
lines changed

3 files changed

+81
-1
lines changed

core/src/components/header/header.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
handleToolbarIntersection,
1616
setHeaderActive,
1717
setToolbarBackgroundOpacity,
18+
getRoleType,
1819
} from './header.utils';
1920

2021
/**
@@ -208,9 +209,10 @@ export class Header implements ComponentInterface {
208209
const { translucent, inheritedAttributes } = this;
209210
const mode = getIonMode(this);
210211
const collapse = this.collapse || 'none';
212+
const isCondensed = collapse === 'condense';
211213

212214
// banner role must be at top level, so remove role if inside a menu
213-
const roleType = hostContext('ion-menu', this.el) ? 'none' : 'banner';
215+
const roleType = getRoleType(hostContext('ion-menu', this.el), isCondensed, mode);
214216

215217
return (
216218
<Host

core/src/components/header/header.utils.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import { readTask, writeTask } from '@stencil/core';
22
import { clamp } from '@utils/helpers';
33

44
const TRANSITION = 'all 0.2s ease-in-out';
5+
const ROLE_NONE = 'none';
6+
const ROLE_BANNER = 'banner';
57

68
interface HeaderIndex {
79
el: HTMLIonHeaderElement;
@@ -171,6 +173,7 @@ export const setHeaderActive = (headerIndex: HeaderIndex, active = true) => {
171173
const ionTitles = toolbars.map((toolbar) => toolbar.ionTitleEl);
172174

173175
if (active) {
176+
headerEl.setAttribute('role', ROLE_BANNER);
174177
headerEl.classList.remove('header-collapse-condense-inactive');
175178

176179
ionTitles.forEach((ionTitle) => {
@@ -179,6 +182,16 @@ export const setHeaderActive = (headerIndex: HeaderIndex, active = true) => {
179182
}
180183
});
181184
} else {
185+
/**
186+
* There can only be one banner landmark per page.
187+
* By default, all ion-headers have the banner role.
188+
* This causes an accessibility issue when using a
189+
* condensed header since there are two ion-headers
190+
* on the page at once (active and inactive).
191+
* To solve this, the role needs to be toggled
192+
* based on which header is active.
193+
*/
194+
headerEl.setAttribute('role', ROLE_NONE);
182195
headerEl.classList.add('header-collapse-condense-inactive');
183196

184197
/**
@@ -244,3 +257,28 @@ export const handleHeaderFade = (scrollEl: HTMLElement, baseEl: HTMLElement, con
244257
});
245258
});
246259
};
260+
261+
/**
262+
* Get the role type for the ion-header.
263+
*
264+
* @param isInsideMenu If ion-header is inside ion-menu.
265+
* @param isCondensed If ion-header has collapse="condense".
266+
* @param mode The current mode.
267+
* @returns 'none' if inside ion-menu or if condensed in md
268+
* mode, otherwise 'banner'.
269+
*/
270+
export const getRoleType = (isInsideMenu: boolean, isCondensed: boolean, mode: 'ios' | 'md') => {
271+
// If the header is inside a menu, it should not have the banner role.
272+
if (isInsideMenu) {
273+
return ROLE_NONE;
274+
}
275+
/**
276+
* Only apply role="none" to `md` mode condensed headers
277+
* since the large header is never shown.
278+
*/
279+
if (isCondensed && mode === 'md') {
280+
return ROLE_NONE;
281+
}
282+
// Default to banner role.
283+
return ROLE_BANNER;
284+
};

core/src/components/header/test/condense/header.e2e.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,5 +40,45 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, screenshot, c
4040

4141
await expect(smallTitle).toHaveAttribute('aria-hidden', 'true');
4242
});
43+
44+
test('should only have the banner role on the active header', async ({ page }) => {
45+
await page.goto('/src/components/header/test/condense', config);
46+
const largeTitleHeader = page.locator('#largeTitleHeader');
47+
const smallTitleHeader = page.locator('#smallTitleHeader');
48+
const content = page.locator('ion-content');
49+
50+
await expect(largeTitleHeader).toHaveAttribute('role', 'banner');
51+
await expect(smallTitleHeader).toHaveAttribute('role', 'none');
52+
53+
await content.evaluate(async (el: HTMLIonContentElement) => {
54+
await el.scrollToBottom();
55+
});
56+
await page.locator('#largeTitleHeader.header-collapse-condense-inactive').waitFor();
57+
58+
await expect(largeTitleHeader).toHaveAttribute('role', 'none');
59+
await expect(smallTitleHeader).toHaveAttribute('role', 'banner');
60+
});
61+
});
62+
});
63+
64+
configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
65+
test.describe(title('header: condense'), () => {
66+
test('should only have the banner role on the small header', async ({ page }) => {
67+
await page.goto('/src/components/header/test/condense', config);
68+
const largeTitleHeader = page.locator('#largeTitleHeader');
69+
const smallTitleHeader = page.locator('#smallTitleHeader');
70+
const content = page.locator('ion-content');
71+
72+
await expect(smallTitleHeader).toHaveAttribute('role', 'banner');
73+
await expect(largeTitleHeader).toHaveAttribute('role', 'none');
74+
75+
await content.evaluate(async (el: HTMLIonContentElement) => {
76+
await el.scrollToBottom();
77+
});
78+
await page.waitForChanges();
79+
80+
await expect(smallTitleHeader).toHaveAttribute('role', 'banner');
81+
await expect(largeTitleHeader).toHaveAttribute('role', 'none');
82+
});
4383
});
4484
});

0 commit comments

Comments
 (0)