Skip to content

Commit 49f96d7

Browse files
joselrioIonitronbrandyscarney
authored
fix(overlays,picker): remove invalid aria-hidden attribute (#30563)
Issue number: resolves #30040 --------- ## What is the current behavior? - The usage of `aria-hidden="true"` attributes both on overlay and picker components was causing some console error/warnings messages. - This was being caused due the fact of the activeElement (focused element) was inside those elements with this attribute that's equal to true in all cases. ![d13215a4-a610-4b1f-81d8-ce3ae05df790](https://github.com/user-attachments/assets/1b79c8bf-b23a-4165-8fb4-894be27bbad4) ## What is the new behavior? - There is no need of making usage of this attribute due the facts: - 1. Once overlay is closed the focus will be redirect to the element that triggers the overlay, this way screen readers will be also redirected to the same context of focused element. - 2. After overlay is closed, it will be set as a `display: none;` through the selector `:host(.overlay-hidden)`, which by itself will disable overlay content for the screen readers. - Removed overlay tests since they were basically checking about `aria-hidden` attribute. - Updated PickerColumn and PickerColumnOption structure in order to fit the a11y needs. - Added a focus management system to better drive users when making usage of keyboard navigation inside picker. - Skip A11Y test validation when reported violation is color contrast related. ### ⚠️ NOTE: - Reported devTools issue/warning when selecting picker is **from now on** expected due to focus an input that's set with `tabIndex="-1"` and `aria-hidden="true"` - Which turns into an A11Y violation when it gets focused. It happens that it gets focused dynamically in order to open the native numeric keyboard **once user selects highlighted picker values zone**, in order to allow users to insert numeric values through the keyboard. If this `aria-hidden` and `tabindex` are removed/updated, the existing functionality will be lost since ScreenReaders will start to ignore the updated value through the PickerChange and will be focused onto the focused input. This mentioned input has an onChange event that's used to update the `aria-valuetext` on each `picker-column` which is being capture by the ScreenReader. That said, this new devTools reported issue/warning is a false positive since A11Y behaviour is being covered through a different perspective. ## Does this introduce a breaking change? - [ ] Yes - [X] No ## Testing: - ✅ [ActionSheet Preview](https://ionic-framework-git-rou-11368-to-main-ionic1.vercel.app/src/components/action-sheet/test/a11y) - ✅ [Alert Preview](https://ionic-framework-git-rou-11368-to-main-ionic1.vercel.app/src/components/alert/test/a11y) - ✅ [DateTime Preview](https://ionic-framework-git-rou-11368-to-main-ionic1.vercel.app/src/components/datetime/test/basic) - ✅ [DateTime Button Preview](https://ionic-framework-git-rou-11368-to-main-ionic1.vercel.app/src/components/datetime-button/test/basic) - ✅ [Modal Preview](https://ionic-framework-git-rou-11368-to-main-ionic1.vercel.app/src/components/modal/test/a11y) - ✅ [Popover Preview](https://ionic-framework-git-rou-11368-to-main-ionic1.vercel.app/src/components/popover/test/basic) - ✅ [Select Preview](https://ionic-framework-git-rou-11368-to-main-ionic1.vercel.app/src/components/select/test/basic) - ✅ [Picker, PickerColumn and PickerColumnOption Preview](https://ionic-framework-git-rou-11368-to-main-ionic1.vercel.app/src/components/picker/test/basic) - ✅ [Toast Preview](https://ionic-framework-git-rou-11368-to-main-ionic1.vercel.app/src/components/toast/test/a11y) ## Other Information Dev build: `8.7.4-dev.11756388042.1a103a79` --------- Co-authored-by: ionitron <hi@ionicframework.com> Co-authored-by: Brandy Smith <6577830+brandyscarney@users.noreply.github.com> Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
1 parent b49ba6b commit 49f96d7

File tree

46 files changed

+208
-486
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

46 files changed

+208
-486
lines changed

core/src/components/datetime/datetime.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { Component, Element, Event, Host, Method, Prop, State, Watch, h, writeTa
33
import { startFocusVisible } from '@utils/focus-visible';
44
import { getElementRoot, raf, renderHiddenInput } from '@utils/helpers';
55
import { printIonError, printIonWarning } from '@utils/logging';
6+
import { FOCUS_TRAP_DISABLE_CLASS } from '@utils/overlays';
67
import { isRTL } from '@utils/rtl';
78
import { createColorClasses } from '@utils/theme';
89
import { caretDownSharp, caretUpSharp, chevronBack, chevronDown, chevronForward } from 'ionicons/icons';
@@ -1598,7 +1599,7 @@ export class Datetime implements ComponentInterface {
15981599
forcePresentation === 'time-date'
15991600
? [this.renderTimePickerColumns(forcePresentation), this.renderDatePickerColumns(forcePresentation)]
16001601
: [this.renderDatePickerColumns(forcePresentation), this.renderTimePickerColumns(forcePresentation)];
1601-
return <ion-picker>{renderArray}</ion-picker>;
1602+
return <ion-picker class={FOCUS_TRAP_DISABLE_CLASS}>{renderArray}</ion-picker>;
16021603
}
16031604

16041605
private renderDatePickerColumns(forcePresentation: string) {

core/src/components/menu/menu.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,9 @@ export class Menu implements ComponentInterface, MenuI {
418418
*/
419419
@Method()
420420
setOpen(shouldOpen: boolean, animated = true, role?: string): Promise<boolean> {
421+
// Blur the active element to prevent it from being kept focused inside an element that will be set with aria-hidden="true"
422+
(document.activeElement as HTMLElement)?.blur();
423+
421424
return menuController._setOpen(this, shouldOpen, animated, role);
422425
}
423426

core/src/components/picker-column-option/picker-column-option.scss

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// Picker Column
44
// --------------------------------------------------
55

6-
button {
6+
.picker-column-option-button {
77
@include padding(0);
88
@include margin(0);
99

@@ -40,6 +40,6 @@ button {
4040
opacity: 0.4;
4141
}
4242

43-
:host(.option-disabled) button {
43+
:host(.option-disabled) .picker-column-option-button {
4444
cursor: default;
4545
}

core/src/components/picker-column-option/picker-column-option.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,9 @@ export class PickerColumnOption implements ComponentInterface {
124124
['option-disabled']: disabled,
125125
})}
126126
>
127-
<button tabindex="-1" aria-label={ariaLabel} disabled={disabled} onClick={() => this.onClick()}>
127+
<div class={'picker-column-option-button'} role="button" aria-label={ariaLabel} onClick={() => this.onClick()}>
128128
<slot></slot>
129-
</button>
129+
</div>
130130
</Host>
131131
);
132132
}

core/src/components/picker-column-option/test/a11y/picker-column-option.e2e.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ configs({ directions: ['ltr'] }).forEach(({ config, title }) => {
1010
test('should not have accessibility violations', async ({ page }) => {
1111
await page.goto(`/src/components/picker-column-option/test/a11y`, config);
1212

13-
const results = await new AxeBuilder({ page }).analyze();
13+
const results = await new AxeBuilder({ page }).disableRules('color-contrast').analyze();
1414

1515
expect(results.violations).toEqual([]);
1616
});
84 Bytes
Loading
170 Bytes
Loading
94 Bytes
Loading
86 Bytes
Loading
209 Bytes
Loading

0 commit comments

Comments
 (0)