Skip to content

Commit 978ea4b

Browse files
jessebeachjessebeach
committed
Responding to review comments
1 parent d123ff7 commit 978ea4b

File tree

6 files changed

+269
-149
lines changed

6 files changed

+269
-149
lines changed

‎.eslintrc

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,6 @@
88
"flowtype"
99
],
1010
rules: {
11-
'no-template-curly-in-string': 'off',
12-
'no-restricted-syntax': [
13-
'error',
14-
'ForInStatement',
15-
'LabeledStatement',
16-
'WithStatement',
17-
]
11+
'no-template-curly-in-string': 'off'
1812
}
1913
}

‎__mocks__/genInteractives.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ const interactiveElementsMap = {
4646
menuitem:[],
4747
option: [],
4848
select: [],
49+
// Whereas ARIA makes a distinction between cell and gridcell, the AXObject
50+
// treats them both as CellRole and since gridcell is interactice, we consider
51+
// cell interactive as well.
52+
// td: [],
4953
th: [],
5054
tr: [],
5155
textarea: [],

‎__tests__/src/util/attributesComparator-test.js

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
/* eslint-env mocha */
2+
import expect from 'expect';
3+
import attributesComparator from '../../../src/util/attributesComparator';
4+
import JSXAttributeMock from '../../../__mocks__/JSXAttributeMock';
5+
6+
describe('attributesComparator', () => {
7+
describe('base attributes', () => {
8+
let baseAttributes;
9+
let attributes;
10+
describe('are undefined', () => {
11+
describe('and attributes are undefined', () => {
12+
it('should return true', () => {
13+
expect(attributesComparator()).toBe(true);
14+
});
15+
});
16+
});
17+
describe('are empty', () => {
18+
beforeEach(() => {
19+
baseAttributes = [];
20+
});
21+
describe('and attributes', () => {
22+
describe('are empty', () => {
23+
attributes = [];
24+
it('should return true', () => {
25+
expect(attributesComparator(baseAttributes, attributes))
26+
.toBe(true);
27+
});
28+
});
29+
describe('have values', () => {
30+
attributes = [
31+
JSXAttributeMock('foo', 0),
32+
JSXAttributeMock('bar', 'baz'),
33+
];
34+
it('should return true', () => {
35+
expect(attributesComparator(baseAttributes, attributes))
36+
.toBe(true);
37+
});
38+
});
39+
});
40+
});
41+
describe('have values', () => {
42+
beforeEach(() => {
43+
baseAttributes = [
44+
{
45+
name: 'biz',
46+
value: 1,
47+
}, {
48+
name: 'fizz',
49+
value: 'pop',
50+
}, {
51+
name: 'fuzz',
52+
value: 'lolz',
53+
},
54+
];
55+
});
56+
describe('and attributes', () => {
57+
describe('are empty', () => {
58+
attributes = [];
59+
it('should return false', () => {
60+
expect(attributesComparator(baseAttributes, attributes))
61+
.toBe(false);
62+
});
63+
});
64+
describe('have values', () => {
65+
describe('and the values are the different', () => {
66+
it('should return false', () => {
67+
attributes = [
68+
JSXAttributeMock('biz', 2),
69+
JSXAttributeMock('ziff', 'opo'),
70+
JSXAttributeMock('far', 'lolz'),
71+
];
72+
expect(attributesComparator(baseAttributes, attributes))
73+
.toBe(false);
74+
});
75+
});
76+
describe('and the values are a subset', () => {
77+
it('should return true', () => {
78+
attributes = [
79+
JSXAttributeMock('biz', 1),
80+
JSXAttributeMock('fizz', 'pop'),
81+
JSXAttributeMock('goo', 'gazz'),
82+
];
83+
expect(attributesComparator(baseAttributes, attributes))
84+
.toBe(false);
85+
});
86+
});
87+
describe('and the values are the same', () => {
88+
it('should return true', () => {
89+
attributes = [
90+
JSXAttributeMock('biz', 1),
91+
JSXAttributeMock('fizz', 'pop'),
92+
JSXAttributeMock('fuzz', 'lolz'),
93+
];
94+
expect(attributesComparator(baseAttributes, attributes))
95+
.toBe(true);
96+
});
97+
});
98+
describe('and the values are a superset', () => {
99+
it('should return true', () => {
100+
attributes = [
101+
JSXAttributeMock('biz', 1),
102+
JSXAttributeMock('fizz', 'pop'),
103+
JSXAttributeMock('fuzz', 'lolz'),
104+
JSXAttributeMock('dar', 'tee'),
105+
];
106+
expect(attributesComparator(baseAttributes, attributes))
107+
.toBe(true);
108+
});
109+
});
110+
});
111+
});
112+
});
113+
});
114+
});

‎src/util/attributesComparator.js

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/**
2+
* @flow
3+
*/
4+
5+
import {
6+
getLiteralPropValue,
7+
propName,
8+
} from 'jsx-ast-utils';
9+
import type { Node } from 'ast-types-flow';
10+
11+
/**
12+
* Returns true if all items in baseAttributes are found in attributes. Always
13+
* returns true if baseAttributes is empty.
14+
*/
15+
function attributesComparator(
16+
baseAttributes: Array<{[key: string]: mixed}> = [],
17+
attributes: Array<Node> = [],
18+
): boolean {
19+
return baseAttributes.every(
20+
(baseAttr): boolean => attributes.some(
21+
(attribute): boolean => {
22+
// Guard against non-JSXAttribute nodes like JSXSpreadAttribute
23+
if (attribute.type !== 'JSXAttribute') {
24+
return false;
25+
}
26+
// Attribute matches.
27+
if (baseAttr.name !== propName(attribute)) {
28+
return false;
29+
}
30+
// Value exists and does not match.
31+
if (
32+
baseAttr.value
33+
&& baseAttr.value !== getLiteralPropValue(attribute)
34+
) {
35+
return false;
36+
}
37+
return true;
38+
},
39+
),
40+
);
41+
}
42+
43+
export default attributesComparator;

‎src/util/isInteractiveElement.js

Lines changed: 52 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,22 @@ import {
1111
AXObjects,
1212
elementAXObjects,
1313
} from 'axobject-query';
14-
import {
15-
getLiteralPropValue,
16-
propName,
17-
} from 'jsx-ast-utils';
14+
import attributesComparator from './attributesComparator';
1815

1916
const roleKeys = [...roles.keys()];
17+
const elementRoleEntries = [...elementRoles];
2018

2119
const nonInteractiveRoles = new Set(
2220
roleKeys
23-
.filter(name => !roles.get(name).abstract)
24-
.filter(name => !roles.get(name).superClass.some(
25-
klasses => klasses.includes('widget')),
26-
),
21+
.filter((name) => {
22+
const role = roles.get(name);
23+
return (
24+
!role.abstract
25+
&& !role.superClass.some(
26+
classes => classes.includes('widget'),
27+
)
28+
);
29+
}),
2730
);
2831

2932
const interactiveRoles = new Set(
@@ -33,120 +36,97 @@ const interactiveRoles = new Set(
3336
// aria-activedescendant, thus in practice we treat it as a widget.
3437
'toolbar',
3538
)
36-
.filter(name => !roles.get(name).abstract)
37-
.filter(name => roles.get(name).superClass.some(
38-
klasses => klasses.includes('widget')),
39-
),
39+
.filter((name) => {
40+
const role = roles.get(name);
41+
return (
42+
!role.abstract
43+
&& role.superClass.some(
44+
classes => classes.includes('widget'),
45+
)
46+
);
47+
}),
4048
);
4149

4250

43-
const nonInteractiveElementRoles = [...elementRoles.entries()]
51+
const nonInteractiveElementRoleSchemas = elementRoleEntries
4452
.reduce((
4553
accumulator,
4654
[
4755
elementSchema,
4856
roleSet,
4957
],
5058
) => {
51-
if ([...roleSet.keys()].every(
59+
if ([...roleSet].every(
5260
(role): boolean => nonInteractiveRoles.has(role),
5361
)) {
54-
accumulator.set(elementSchema, roleSet);
62+
accumulator.push(elementSchema);
5563
}
5664
return accumulator;
57-
}, new Map([]));
65+
}, []);
5866

59-
const interactiveElementRoles = [...elementRoles.entries()]
67+
const interactiveElementRoleSchemas = elementRoleEntries
6068
.reduce((
6169
accumulator,
6270
[
6371
elementSchema,
6472
roleSet,
6573
],
6674
) => {
67-
if ([...roleSet.keys()].some(
75+
if ([...roleSet].some(
6876
(role): boolean => interactiveRoles.has(role),
6977
)) {
70-
accumulator.set(elementSchema, roleSet);
78+
accumulator.push(elementSchema);
7179
}
7280
return accumulator;
73-
}, new Map([]));
81+
}, []);
7482

7583
const interactiveAXObjects = new Set(
7684
[...AXObjects.keys()]
77-
.filter(name => ['widget'].includes(AXObjects.get(name).type)),
85+
.filter(name => AXObjects.get(name).type === 'widget'),
7886
);
7987

80-
const interactiveElementAXObjects = [...elementAXObjects.entries()]
88+
const interactiveElementAXObjectSchemas = [...elementAXObjects]
8189
.reduce((
8290
accumulator,
8391
[
8492
elementSchema,
8593
AXObjectSet,
8694
],
8795
) => {
88-
if ([...AXObjectSet.keys()].every(
96+
if ([...AXObjectSet].every(
8997
(role): boolean => interactiveAXObjects.has(role),
9098
)) {
91-
accumulator.set(elementSchema, AXObjectSet);
99+
accumulator.push(elementSchema);
92100
}
93101
return accumulator;
94-
}, new Map([]));
95-
96-
function attributesComparator(baseAttributes = [], attributes = []): boolean {
97-
return baseAttributes.every(
98-
(baseAttr): boolean => attributes.some(
99-
(attribute): boolean => {
100-
// Guard against non-JSXAttribute nodes like JSXSpreadAttribute
101-
if (attribute.type !== 'JSXAttribute') {
102-
return false;
103-
}
104-
let attrMatches = false;
105-
let valueMatches = true;
106-
// Attribute matches.
107-
if (baseAttr.name === propName(attribute).toLowerCase()) {
108-
attrMatches = true;
109-
}
110-
// Value exists and matches.
111-
if (baseAttr.value) {
112-
valueMatches = baseAttr.value === getLiteralPropValue(attribute);
113-
}
114-
return attrMatches && valueMatches;
115-
},
116-
),
117-
);
118-
}
102+
}, []);
119103

120104
function checkIsInteractiveElement(tagName, attributes): boolean {
121-
// Check in configuration for an override.
122-
123-
// Check in elementRoles for inherent interactive role associations for
124-
// this element.
125-
for (const [elementSchema] of interactiveElementRoles) {
126-
if (
105+
function elementSchemaMatcher(elementSchema) {
106+
return (
127107
tagName === elementSchema.name
128108
&& attributesComparator(elementSchema.attributes, attributes)
129-
) {
130-
return true;
131-
}
109+
);
132110
}
133-
134111
// Check in elementRoles for inherent interactive role associations for
135112
// this element.
136-
for (const [elementSchema] of nonInteractiveElementRoles) {
137-
if (
138-
tagName === elementSchema.name
139-
&& attributesComparator(elementSchema.attributes, attributes)
140-
) {
141-
return false;
142-
}
113+
const isInherentInteractiveElement = interactiveElementRoleSchemas
114+
.some(elementSchemaMatcher);
115+
if (isInherentInteractiveElement) {
116+
return true;
143117
}
144-
145-
// Check in elementAXObjects for AX Tree associations for this elements.
146-
for (const [elementSchema] of interactiveElementAXObjects) {
147-
if (tagName === elementSchema.name) {
148-
return attributesComparator(elementSchema.attributes, attributes);
149-
}
118+
// Check in elementRoles for inherent non-interactive role associations for
119+
// this element.
120+
const isInherentNonInteractiveElement = nonInteractiveElementRoleSchemas
121+
.some(elementSchemaMatcher);
122+
if (isInherentNonInteractiveElement) {
123+
return false;
124+
}
125+
// Check in elementAXObjects for AX Tree associations for this element.
126+
const isInteractiveAXElement = interactiveElementAXObjectSchemas
127+
.some(elementSchemaMatcher);
128+
if (isInteractiveAXElement) {
129+
return true;
150130
}
151131

152132
return false;

0 commit comments

Comments
 (0)