Skip to content

Commit

Permalink
fix(rule): don't check keyup events for some elements (#772)
Browse files Browse the repository at this point in the history
  • Loading branch information
mohammedzamakhan authored and mgechev committed Mar 2, 2019
1 parent 865ec3b commit a9c4ae9
Show file tree
Hide file tree
Showing 10 changed files with 377 additions and 8 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
"dependencies": {
"app-root-path": "^2.1.0",
"aria-query": "^3.0.0",
"axobject-query": "^2.0.2",
"css-selector-tokenizer": "^0.7.0",
"cssauron": "^1.4.0",
"damerau-levenshtein": "^1.0.4",
Expand Down
21 changes: 21 additions & 0 deletions src/templateClickEventsHaveKeyEventsRule.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import { ElementAst } from '@angular/compiler';
import { dom } from 'aria-query';
import { IRuleMetadata, RuleFailure, Rules } from 'tslint/lib';
import { SourceFile } from 'typescript/lib/typescript';
import { NgWalker } from './angular/ngWalker';
import { BasicTemplateAstVisitor } from './angular/templates/basicTemplateAstVisitor';
import { isInteractiveElement } from './util/isInteractiveElement';
import { isPresentationRole } from './util/isPresentationRole';
import { isHiddenFromScreenReader } from './util/isHiddenFromScreenReader';

const domElements = new Set(dom.keys());

export class Rule extends Rules.AbstractRule {
static readonly metadata: IRuleMetadata = {
Expand Down Expand Up @@ -37,6 +43,21 @@ class TemplateClickEventsHaveKeyEventsVisitor extends BasicTemplateAstVisitor {
if (!hasClick) {
return;
}

if (!domElements.has(el.name)) {
// Do not test components, as we do not know what
// low-level DOM element this maps to.
return;
}

if (isPresentationRole(el) || isHiddenFromScreenReader(el)) {
return;
}

if (isInteractiveElement(el)) {
return;
}

const hasKeyEvent = el.outputs.some(output => output.name === 'keyup' || output.name === 'keydown' || output.name === 'keypress');

if (hasKeyEvent) {
Expand Down
24 changes: 24 additions & 0 deletions src/util/attributesComparator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { ElementAst, AttrAst, BoundElementPropertyAst } from '@angular/compiler';
import { getAttributeValue } from './getAttributeValue';
import { getLiteralValue } from './getLiteralValue';

export function attributesComparator(baseAttributes: any = [], el: ElementAst): boolean {
const attributes: Array<AttrAst | BoundElementPropertyAst> = [...el.attrs, ...el.inputs];
return baseAttributes.every(
(baseAttr): boolean =>
attributes.some(
(attribute): boolean => {
if (el.name === 'a' && attribute.name === 'routerLink') {
return true;
}
if (baseAttr.name !== attribute.name) {
return false;
}
if (baseAttr.value && baseAttr.value !== getLiteralValue(getAttributeValue(el, baseAttr.name))) {
return false;
}
return true;
}
)
);
}
13 changes: 10 additions & 3 deletions src/util/getAttributeValue.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
import { ElementAst } from '@angular/compiler';
import { ElementAst, ASTWithSource, LiteralPrimitive } from '@angular/compiler';
import { PROPERTY } from './isHiddenFromScreenReader';

export const getAttributeValue = (element: ElementAst, property: string) => {
const attr = element.attrs.find(attr => attr.name === property);
const input = element.inputs.find(input => input.name === property);
if (attr) {
return attr.value;
}
if (input) {
return (<any>input.value).ast.value;
if (!input || !(input.value instanceof ASTWithSource)) {
return undefined;
}

if (input.value.ast instanceof LiteralPrimitive) {
return input.value.ast.value;
}

return PROPERTY;
};
8 changes: 8 additions & 0 deletions src/util/getLiteralValue.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export const getLiteralValue = value => {
if (value === 'true') {
return true;
} else if (value === 'false') {
return false;
}
return value;
};
24 changes: 24 additions & 0 deletions src/util/isHiddenFromScreenReader.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { ElementAst } from '@angular/compiler';
import { getAttributeValue } from './getAttributeValue';
import { getLiteralValue } from './getLiteralValue';

export const PROPERTY = ['PROPERTY'];
/**
* Returns boolean indicating that the aria-hidden prop
* is present or the value is true. Will also return true if
* there is an input with type='hidden'.
*
* <div aria-hidden /> is equivalent to the DOM as <div aria-hidden=true />.
*/
export const isHiddenFromScreenReader = (el: ElementAst) => {
if (el.name.toUpperCase() === 'INPUT') {
const hidden = getAttributeValue(el, 'type');

if (hidden && hidden.toUpperCase() === 'HIDDEN') {
return true;
}
}

const ariaHidden = getLiteralValue(getAttributeValue(el, 'aria-hidden'));
return ariaHidden === PROPERTY || ariaHidden === true;
};
94 changes: 94 additions & 0 deletions src/util/isInteractiveElement.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import { dom, elementRoles, roles } from 'aria-query';

import { AXObjects, elementAXObjects } from 'axobject-query';

import { attributesComparator } from './attributesComparator';
import { ElementAst } from '@angular/compiler';

const domKeys = <string[]>Array.from(dom.keys());
const roleKeys: any = <string[]>Array.from(roles.keys());
const elementRoleEntries = Array.from(elementRoles);

const nonInteractiveRoles = new Set(
roleKeys.filter(name => {
const role = roles.get(name);
return !role.abstract && !role.superClass.some(classes => classes.indexOf('widget') !== 0);
})
);

const interactiveRoles: any = new Set(
[
...roleKeys,
// 'toolbar' does not descend from widget, but it does support
// aria-activedescendant, thus in practice we treat it as a widget.
'toolbar'
].filter(name => {
const role = roles.get(name);
return !role.abstract && role.superClass.some(classes => classes.indexOf('widget') !== 0);
})
);

const nonInteractiveElementRoleSchemas = elementRoleEntries.reduce((accumulator: any, [elementSchema, roleSet]: any) => {
if (Array.from(roleSet).every((role): boolean => nonInteractiveRoles.has(role))) {
accumulator.push(elementSchema);
}
return accumulator;
}, []);

const interactiveElementRoleSchemas = elementRoleEntries.reduce((accumulator: any, [elementSchema, roleSet]: any) => {
if (Array.from(roleSet).some((role): boolean => interactiveRoles.has(role))) {
accumulator.push(elementSchema);
}
return accumulator;
}, []);

const interactiveAXObjects = new Set(Array.from(AXObjects.keys()).filter(name => AXObjects.get(name).type === 'widget'));

const interactiveElementAXObjectSchemas = Array.from(elementAXObjects).reduce((accumulator: any, [elementSchema, AXObjectSet]: any) => {
if (Array.from(AXObjectSet).every((role): boolean => interactiveAXObjects.has(role))) {
accumulator.push(elementSchema);
}
return accumulator;
}, []);

function checkIsInteractiveElement(el: ElementAst): boolean {
function elementSchemaMatcher(elementSchema) {
return el.name === elementSchema.name && attributesComparator(elementSchema.attributes, el);
}
// Check in elementRoles for inherent interactive role associations for
// this element.
const isInherentInteractiveElement = interactiveElementRoleSchemas.some(elementSchemaMatcher);

if (isInherentInteractiveElement) {
return true;
}
// Check in elementRoles for inherent non-interactive role associations for
// this element.
const isInherentNonInteractiveElement = nonInteractiveElementRoleSchemas.some(elementSchemaMatcher);

if (isInherentNonInteractiveElement) {
return false;
}
// Check in elementAXObjects for AX Tree associations for this element.
const isInteractiveAXElement = interactiveElementAXObjectSchemas.some(elementSchemaMatcher);

if (isInteractiveAXElement) {
return true;
}

return false;
}

/**
* Returns boolean indicating whether the given element is
* interactive on the DOM or not. Usually used when an element
* has a dynamic handler on it and we need to discern whether or not
* it's intention is to be interacted with on the DOM.
*/
export const isInteractiveElement = (el: ElementAst): boolean => {
if (domKeys.indexOf(el.name) === -1) {
return false;
}

return checkIsInteractiveElement(el);
};
7 changes: 7 additions & 0 deletions src/util/isPresentationRole.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { getAttributeValue } from './getAttributeValue';
import { ElementAst } from '@angular/compiler';
import { PROPERTY } from './isHiddenFromScreenReader';

const presentationRoles = new Set(['presentation', 'none', PROPERTY]);

export const isPresentationRole = (el: ElementAst) => presentationRoles.has(getAttributeValue(el, 'role'));
Loading

0 comments on commit a9c4ae9

Please sign in to comment.