Skip to content

Commit

Permalink
feat: role-supports-aria-props rule (no aria-label-misuse)
Browse files Browse the repository at this point in the history
  • Loading branch information
smockle authored Jan 10, 2023
2 parents 3b7aefa + cd0a526 commit cc51b62
Show file tree
Hide file tree
Showing 18 changed files with 1,369 additions and 83 deletions.
44 changes: 21 additions & 23 deletions .devcontainer/devcontainer.json
Original file line number Diff line number Diff line change
@@ -1,32 +1,30 @@
// For format details, see https://aka.ms/devcontainer.json. For config options, see the README at:
// https://github.com/microsoft/vscode-dev-containers/tree/v0.222.0/containers/javascript-node
{
"name": "Node.js",
"build": {
"dockerfile": "Dockerfile",
// Update 'VARIANT' to pick a Node version: 16, 14, 12.
// Append -bullseye or -buster to pin to an OS version.
// Use -bullseye variants on local arm64/Apple Silicon.
"args": { "VARIANT": "16" }
},
"name": "Node.js",
"build": {
"dockerfile": "Dockerfile",
// Update 'VARIANT' to pick a Node version: 16, 14, 12.
// Append -bullseye or -buster to pin to an OS version.
// Use -bullseye variants on local arm64/Apple Silicon.
"args": {"VARIANT": "16"}
},

// Set *default* container specific settings.json values on container create.
"settings": {},
// Set *default* container specific settings.json values on container create.
"settings": {},

// Add the IDs of extensions you want installed when the container is created.
"extensions": [
"dbaeumer.vscode-eslint"
],
// Add the IDs of extensions you want installed when the container is created.
"extensions": ["dbaeumer.vscode-eslint"],

// Use 'forwardPorts' to make a list of ports inside the container available locally.
// "forwardPorts": [],
// Use 'forwardPorts' to make a list of ports inside the container available locally.
// "forwardPorts": [],

// Use 'postCreateCommand' to run commands after the container is created.
// "postCreateCommand": "yarn install",
// Use 'postCreateCommand' to run commands after the container is created.
// "postCreateCommand": "yarn install",

// Comment out to connect as root instead. More info: https://aka.ms/vscode-remote/containers/non-root.
"remoteUser": "node",
"features": {
"git": "latest"
}
// Comment out to connect as root instead. More info: https://aka.ms/vscode-remote/containers/non-root.
"remoteUser": "node",
"features": {
"git": "latest"
}
}
2 changes: 1 addition & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module.exports = {
root: true,
parserOptions: {
ecmaVersion: 2020,
ecmaVersion: 13,
},
env: {
es6: true,
Expand Down
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ For each component, you may specify a `default` and/or `props`. `default` may ma
"settings": {
"github": {
"components": {
"Box": { "default": "p" },
"Link": { "props": {"as": { "undefined": "a", "a": "a", "button": "button"}}},
"Box": {"default": "p"},
"Link": {"props": {"as": {"undefined": "a", "a": "a", "button": "button"}}}
}
}
}
Expand Down Expand Up @@ -94,3 +94,4 @@ This config will be interpreted in the following way:
#### Accessibility-focused rules (prefixed with a11y)

- [No Generic Link Text](./docs/rules/a11y-no-generic-link-text.md)
- [Role Supports ARIA Props](./docs/rules/role-supports-aria-props.md)
10 changes: 7 additions & 3 deletions docs/rules/a11y-no-generic-link-text.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,18 @@ Please perform browser tests and spot checks:
```

```jsx
<a href="github.com/about" aria-label="Why dogs are awesome">Read more</a>
<a href="github.com/about" aria-label="Why dogs are awesome">
Read more
</a>
```

```jsx
<a href="github.com/about" aria-describedby="element123">Read more</a>
<a href="github.com/about" aria-describedby="element123">
Read more
</a>
```

### **Correct** code for this rule 👍
### **Correct** code for this rule 👍

```jsx
<a href="github.com/about">Learn more about GitHub</a>
Expand Down
1 change: 1 addition & 0 deletions docs/rules/array-foreach.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ for (const el of els) {
```

Use [`entries()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/entries) to get access to the index:

```js
for (const [i, el] of els.entries()) {
el.name = `Element ${i}`
Expand Down
4 changes: 2 additions & 2 deletions docs/rules/authenticity-token.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ on('click', '.js-my-button', function (e) {

fetch(form.action, {
method: form.method,
body: new FormData(form)
body: new FormData(form),
}).then(function () {
alert('Success!')
})
Expand All @@ -52,7 +52,7 @@ An alternate, but less preferred approach is to include the a signed CSRF url in
on('click', '.js-my-button', function (e) {
csrfRequest(this.getAttribute('data-url'), {
method: 'PUT',
body: data
body: data,
}).then(function () {
alert('Success!')
})
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/no-useless-passive.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ window.addEventListener(
() => {
console.log('Scroll event fired!')
},
{passive: true}
{passive: true},
)
```

Expand Down
2 changes: 1 addition & 1 deletion docs/rules/prefer-observers.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ window.addEventListener('scroll', () => {
const isIntersecting = checkIfElementIntersects(
element.getBoundingClientRect(),
window.innerHeight,
document.clientHeight
document.clientHeight,
)
element.classList.toggle('intersecting', isIntersecting)
})
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/require-passive-events.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ window.addEventListener(
() => {
/* ... */
},
{passive: true}
{passive: true},
)
```

Expand Down
74 changes: 74 additions & 0 deletions docs/rules/role-supports-aria-props.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# Role Supports ARIA Props

## Rule Details

This rule enforces that elements with explicit or implicit roles defined contain only `aria-*` properties supported by that `role`.

For example, this rule aims to discourage common misuse of the `aria-label` and `aria-labelledby` attribute. `aria-label` and `aria-labelledby` support is only guaranteed on interactive elements like `button` or `a`, or on static elements like `div` and `span` with a permitted `role`. This rule will allow `aria-label` and `aria-labelledby` usage on `div` and `span` elements if it set to a role other than the ones listed in [WSC: a list of ARIA roles which cannot be named](https://w3c.github.io/aria/#namefromprohibited). This rule will never permit usage of `aria-label` and `aria-labelledby` on `h1`, `h2`, `h3`, `h4`, `h5`, `h6`, `strong`, `i`, `p`, `b`, or `code`.

### "Help! I'm trying to set a tooltip on a static element and this rule flagged it!"

Please do not use tooltips on static elements. It is a highly discouraged, inaccessible pattern.
See [Primer: Tooltip alternatives](https://primer.style/design/accessibility/tooltip-alternatives) for what to do instead.

### Resources

- [w3c/aria Consider prohibiting author naming certain roles #833](https://github.com/w3c/aria/issues/833)
- [Not so short note on aria-label usage - Big Table Edition](https://html5accessibility.com/stuff/2020/11/07/not-so-short-note-on-aria-label-usage-big-table-edition/)
- [Your tooltips are bogus](https://heydonworks.com/article/your-tooltips-are-bogus/)
- [Primer: Tooltip alternatives](https://primer.style/design/accessibility/tooltip-alternatives)

### Disclaimer

There are conflicting resources and opinions on what elements should support these naming attributes. For now, this rule will operate under a relatively simple heuristic aimed to minimize false positives. This may have room for future improvements. Learn more at [W3C Name Calcluation](https://w3c.github.io/aria/#namecalculation).

### **Incorrect** code for this rule 👎

```erb
<span class="tooltipped" aria-label="This is a tooltip">I am some text.</span>
```

```erb
<span aria-label="This is some content that will completely override the button content">Please be careful of the following.</span>
```

```erb
<div aria-labelledby="heading1">Goodbye</div>
```

```erb
<h1 aria-label="This will override the page title completely">Page title</h1>
```

### **Correct** code for this rule 👍

```erb
<button aria-label="Close">
<svg src="closeIcon"></svg>
</button>
```

```erb
<button aria-label="Bold" aria-describedby="tooltip1">
<svg src="boldIcon"></svg>
</button>
<p id="tooltip1" class="tooltip">Add bold text or turn selection into bold text</p>
```

```erb
<span>Hello</span>
```

```erb
<div>Goodbye</div>
```

```erb
<h1>Page title</h1>
```

```erb
<div role="dialog" aria-labelledby="dialogHeading">
<h1 id="dialogHeading">Heading</h1>
</div>
```
2 changes: 2 additions & 0 deletions lib/configs/react.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,7 @@ module.exports = {
extends: ['plugin:jsx-a11y/recommended'],
rules: {
'github/a11y-no-generic-link-text': 'error',
'github/role-supports-aria-props': 'error',
'jsx-a11y/role-supports-aria-props': 'off',
},
}
1 change: 1 addition & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ module.exports = {
'no-then': require('./rules/no-then'),
'no-useless-passive': require('./rules/no-useless-passive'),
'prefer-observers': require('./rules/prefer-observers'),
'role-supports-aria-props': require('./rules/role-supports-aria-props'),
'require-passive-events': require('./rules/require-passive-events'),
'unescaped-html-literal': require('./rules/unescaped-html-literal'),
},
Expand Down
101 changes: 101 additions & 0 deletions lib/rules/role-supports-aria-props.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
// @ts-check
const {aria, elementRoles, roles} = require('aria-query')
const {getProp, getPropValue, propName} = require('jsx-ast-utils')
const {getElementType} = require('../utils/get-element-type')
const ObjectMap = require('../utils/object-map')

// Clean-up `elementRoles` from `aria-query`
const elementRolesMap = new ObjectMap()
for (const [key, value] of elementRoles.entries()) {
// - Remove unused `constraints` key
delete key.constraints
key.attributes = key.attributes?.filter(attribute => !('constraints' in attribute))
// - Remove empty `attributes` key
if (!key.attributes || key.attributes?.length === 0) {
delete key.attributes
}
elementRolesMap.set(key, value)
}
// - Remove insufficiently-disambiguated `menuitem` entry
elementRolesMap.delete({name: 'menuitem'})
// - Disambiguate `menuitem` and `menu` roles by `type`
elementRolesMap.set({name: 'menuitem', attributes: [{name: 'type', value: 'command'}]}, ['menuitem'])
elementRolesMap.set({name: 'menuitem', attributes: [{name: 'type', value: 'radio'}]}, ['menuitemradio'])
elementRolesMap.set({name: 'menuitem', attributes: [{name: 'type', value: 'toolbar'}]}, ['toolbar'])
elementRolesMap.set({name: 'menu', attributes: [{name: 'type', value: 'toolbar'}]}, ['toolbar'])

module.exports = {
meta: {
docs: {
description:
'Enforce that elements with explicit or implicit roles defined contain only `aria-*` properties supported by that `role`.',
url: require('../url')(module),
},
schema: [],
},

create(context) {
return {
JSXOpeningElement(node) {
// Assemble a key for looking-up the element’s role in the `elementRolesMap`
// - Get the element’s name
const key = {name: getElementType(context, node)}
// - Get the element’s disambiguating attributes
for (const prop of ['aria-expanded', 'type', 'size', 'role', 'href', 'multiple', 'scope']) {
// - Only provide `aria-expanded` when it’s required for disambiguation
if (prop === 'aria-expanded' && key.name !== 'summary') continue
const value = getPropValue(getProp(node.attributes, prop))
if (value) {
if (!('attributes' in key)) {
key.attributes = []
}
if (prop === 'href') {
key.attributes.push({name: prop})
} else {
key.attributes.push({name: prop, value})
}
}
}
// Get the element’s explicit or implicit role
const role = getPropValue(getProp(node.attributes, 'role')) ?? elementRolesMap.get(key)?.[0]

// Return early if role could not be determined
if (!role) return

// Get allowed ARIA attributes:
// - From the role itself
let allowedProps = Object.keys(roles.get(role)?.props || {})
// - From parent roles
for (const parentRole of roles.get(role)?.superClass.flat() ?? []) {
allowedProps = allowedProps.concat(Object.keys(roles.get(parentRole)?.props || {}))
}
// Dedupe, for performance
allowedProps = Array.from(new Set(allowedProps))

// Get prohibited ARIA attributes:
// - From the role itself
let prohibitedProps = roles.get(role)?.prohibitedProps || []
// - From parent roles
for (const parentRole of roles.get(role)?.superClass.flat() ?? []) {
prohibitedProps = prohibitedProps.concat(roles.get(parentRole)?.prohibitedProps || [])
}
// - From comparing allowed vs all ARIA attributes
prohibitedProps = prohibitedProps.concat(aria.keys().filter(x => !allowedProps.includes(x)))
// Dedupe, for performance
prohibitedProps = Array.from(new Set(prohibitedProps))

for (const prop of node.attributes) {
// Return early if prohibited ARIA attribute is set to an ignorable value
if (getPropValue(prop) == null || prop.type === 'JSXSpreadAttribute') return

if (prohibitedProps?.includes(propName(prop))) {
context.report({
node,
message: `The attribute ${propName(prop)} is not supported by the role ${role}.`,
})
}
}
},
}
},
}
Loading

0 comments on commit cc51b62

Please sign in to comment.