fix(item): only add click event listener to items with inputs (#22352)

This stops screen readers, such as NVDA, from reading every item as clickable even when it is text only.

fixes #22011
This commit is contained in:
Brandy Carney
2020-11-04 16:19:38 -05:00
committed by GitHub
parent 9752cd6371
commit 9659ad6334
8 changed files with 58 additions and 14 deletions

View File

@ -139,8 +139,7 @@ export class Alert implements ComponentInterface, OverlayInterface {
if ( if (
!inputTypes.has('radio') !inputTypes.has('radio')
|| (ev.target && !this.el.contains(ev.target)) || (ev.target && !this.el.contains(ev.target))
|| ev.target.classList.contains('alert-button')) || ev.target.classList.contains('alert-button')) {
{
return; return;
} }
@ -170,7 +169,7 @@ export class Alert implements ComponentInterface, OverlayInterface {
if (['ArrowUp', 'ArrowLeft'].includes(ev.key)) { if (['ArrowUp', 'ArrowLeft'].includes(ev.key)) {
nextEl = (index === 0) nextEl = (index === 0)
? radios[radios.length - 1] ? radios[radios.length - 1]
: radios[index - 1] : radios[index - 1];
} }
if (nextEl && radios.includes(nextEl)) { if (nextEl && radios.includes(nextEl)) {

View File

@ -285,7 +285,7 @@ export class Datetime implements ComponentInterface {
if (data.name !== 'ampm' && this.datetimeValue.ampm !== undefined) { if (data.name !== 'ampm' && this.datetimeValue.ampm !== undefined) {
changeData['ampm'] = { changeData['ampm'] = {
value: this.datetimeValue.ampm value: this.datetimeValue.ampm
} };
} }
this.updateDatetimeValue(changeData); this.updateDatetimeValue(changeData);

View File

@ -30,6 +30,7 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
private labelColorStyles = {}; private labelColorStyles = {};
private itemStyles = new Map<string, CssClassMap>(); private itemStyles = new Map<string, CssClassMap>();
private clickListener?: (ev: Event) => void;
@Element() el!: HTMLIonItemElement; @Element() el!: HTMLIonItemElement;
@ -152,7 +153,33 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
} }
} }
componentDidUpdate() {
// Do not use @Listen here to avoid making all items
// appear as clickable to screen readers
// https://github.com/ionic-team/ionic-framework/issues/22011
const input = this.getFirstInput();
if (input && !this.clickListener) {
this.clickListener = (ev: Event) => this.delegateFocus(ev, input);
this.el.addEventListener('click', this.clickListener);
}
}
disconnectedCallback() {
const input = this.getFirstInput();
if (input && this.clickListener) {
this.el.removeEventListener('click', this.clickListener);
this.clickListener = undefined;
}
}
componentDidLoad() { componentDidLoad() {
this.setMultipleInputs();
}
// If the item contains multiple clickable elements and/or inputs, then the item
// should not have a clickable input cover over the entire item to prevent
// interfering with their individual click events
private setMultipleInputs() {
// The following elements have a clickable cover that is relative to the entire item // The following elements have a clickable cover that is relative to the entire item
const covers = this.el.querySelectorAll('ion-checkbox, ion-datetime, ion-select, ion-radio'); const covers = this.el.querySelectorAll('ion-checkbox, ion-datetime, ion-select, ion-radio');
@ -199,22 +226,20 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
// clicking on the left padding of an item is not focusing the input // clicking on the left padding of an item is not focusing the input
// but is opening the keyboard. It will no longer be needed with // but is opening the keyboard. It will no longer be needed with
// iOS 14. // iOS 14.
@Listen('click') private delegateFocus(ev: Event, input: HTMLIonInputElement | HTMLIonTextareaElement) {
delegateFocus(ev: Event) {
const clickedItem = (ev.target as HTMLElement).tagName === 'ION-ITEM'; const clickedItem = (ev.target as HTMLElement).tagName === 'ION-ITEM';
const input = this.getFirstInput();
let firstActive = false; let firstActive = false;
// If the first input is the same as the active element we need // If the first input is the same as the active element we need
// to focus the first input again, but if the active element // to focus the first input again, but if the active element
// is another input inside of the item we shouldn't switch focus // is another input inside of the item we shouldn't switch focus
if (input && document.activeElement) { if (document.activeElement) {
firstActive = input.querySelector('input, textarea') === document.activeElement; firstActive = input.querySelector('input, textarea') === document.activeElement;
} }
// Only focus the first input if we clicked on an ion-item // Only focus the first input if we clicked on an ion-item
// and the first input exists // and the first input exists
if (clickedItem && input && firstActive) { if (clickedItem && firstActive) {
input.fireFocusEvents = false; input.fireFocusEvents = false;
input.setBlur(); input.setBlur();
input.setFocus(); input.setFocus();
@ -239,6 +264,11 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
rel, rel,
target target
}; };
// Only set onClick if the item is clickable to prevent screen
// readers from reading all items as clickable
const clickFn = clickable ? {
onClick: (ev: Event) => {openURL(href, ev, routerDirection, routerAnimation); }
} : {};
const showDetail = detail !== undefined ? detail : mode === 'ios' && clickable; const showDetail = detail !== undefined ? detail : mode === 'ios' && clickable;
this.itemStyles.forEach(value => { this.itemStyles.forEach(value => {
Object.assign(childStyles, value); Object.assign(childStyles, value);
@ -267,7 +297,7 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
class="item-native" class="item-native"
part="native" part="native"
disabled={disabled} disabled={disabled}
onClick={(ev: Event) => openURL(href, ev, routerDirection, routerAnimation)} {...clickFn}
> >
<slot name="start"></slot> <slot name="start"></slot>
<div class="item-inner"> <div class="item-inner">

View File

@ -123,6 +123,10 @@
</ion-label> </ion-label>
</ion-list-header> </ion-list-header>
<ion-list class="multiple"> <ion-list class="multiple">
<ion-item id="delayedInputItem">
<ion-label>Delayed input</ion-label>
</ion-item>
<ion-item> <ion-item>
<ion-label>Multiple inputs</ion-label> <ion-label>Multiple inputs</ion-label>
<ion-input placeholder="Input 1"></ion-input> <ion-input placeholder="Input 1"></ion-input>
@ -200,6 +204,15 @@
clickableItem.color = color === undefined ? 'primary' : undefined; clickableItem.color = color === undefined ? 'primary' : undefined;
}); });
const delayedInputItem = document.querySelector('#delayedInputItem');
const delayedInput = document.createElement('ion-input');
delayedInput.type = 'number';
delayedInput.value = 34;
setTimeout(() => {
delayedInputItem.appendChild(delayedInput);
}, 3000);
const inputs = document.querySelectorAll('ion-input, ion-textarea'); const inputs = document.querySelectorAll('ion-input, ion-textarea');
for (var i = 0; i < inputs.length; i++) { for (var i = 0; i < inputs.length; i++) {

View File

@ -126,7 +126,7 @@ export class RadioGroup implements ComponentInterface {
if (['ArrowUp', 'ArrowLeft'].includes(ev.key)) { if (['ArrowUp', 'ArrowLeft'].includes(ev.key)) {
next = (index === 0) next = (index === 0)
? radios[radios.length - 1] ? radios[radios.length - 1]
: radios[index - 1] : radios[index - 1];
} }
if (next && radios.includes(next)) { if (next && radios.includes(next)) {

View File

@ -299,7 +299,7 @@ const focusPreviousElementOnDismiss = async (overlayEl: any) => {
await overlayEl.onDidDismiss(); await overlayEl.onDidDismiss();
previousElement.focus(); previousElement.focus();
} };
export const dismiss = async ( export const dismiss = async (
overlay: OverlayInterface, overlay: OverlayInterface,

View File

@ -21,7 +21,7 @@
}, },
"scripts": { "scripts": {
"build": "npm run clean && npm run compile", "build": "npm run clean && npm run compile",
"clean": "rm -rf dist dist-transpiled", "clean": "rimraf dist dist-transpiled",
"compile": "npm run tsc && rollup -c", "compile": "npm run tsc && rollup -c",
"release": "np --any-branch --no-cleanup", "release": "np --any-branch --no-cleanup",
"lint": "tslint --project .", "lint": "tslint --project .",
@ -65,6 +65,7 @@
"react-dom": "^16.9.0", "react-dom": "^16.9.0",
"react-router": "^5.0.1", "react-router": "^5.0.1",
"react-router-dom": "^5.0.1", "react-router-dom": "^5.0.1",
"rimraf": "^3.0.2",
"rollup": "^2.26.4", "rollup": "^2.26.4",
"rollup-plugin-sourcemaps": "^0.6.2", "rollup-plugin-sourcemaps": "^0.6.2",
"ts-jest": "^26.1.1", "ts-jest": "^26.1.1",

View File

@ -21,7 +21,7 @@
}, },
"scripts": { "scripts": {
"build": "npm run clean && npm run copy && npm run compile", "build": "npm run clean && npm run copy && npm run compile",
"clean": "rm -rf dist && rm -rf dist-transpiled && rm -rf routing", "clean": "rimraf dist && rimraf dist-transpiled && rimraf routing",
"compile": "npm run tsc && rollup -c", "compile": "npm run tsc && rollup -c",
"release": "np --any-branch --yolo --no-release-draft", "release": "np --any-branch --yolo --no-release-draft",
"lint": "tslint --project .", "lint": "tslint --project .",
@ -61,6 +61,7 @@
"np": "^6.4.0", "np": "^6.4.0",
"react": "^16.9.0", "react": "^16.9.0",
"react-dom": "^16.9.0", "react-dom": "^16.9.0",
"rimraf": "^3.0.2",
"rollup": "^2.26.4", "rollup": "^2.26.4",
"rollup-plugin-sourcemaps": "^0.6.2", "rollup-plugin-sourcemaps": "^0.6.2",
"ts-jest": "^26.1.1", "ts-jest": "^26.1.1",