Skip to content

New rule: ambiguous-anchor-text #873

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ configuration file by mapping each custom component name to a DOM element type.
<!-- AUTO-GENERATED-CONTENT:START (LIST) -->

- [alt-text](https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/tree/HEAD/docs/rules/alt-text.md): Enforce all elements that require alternative text have meaningful information to relay back to end user.
- [anchor-ambiguous-text](https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/tree/HEAD/docs/rules/anchor-ambiguous-text.md): Enforce `<a>` text to not exactly match "click here", "here", "link", or "a link".
- [anchor-has-content](https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/tree/HEAD/docs/rules/anchor-has-content.md): Enforce all anchors to contain accessible content.
- [anchor-is-valid](https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/tree/HEAD/docs/rules/anchor-is-valid.md): Enforce all anchors are valid, navigable elements.
- [aria-activedescendant-has-tabindex](https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/tree/HEAD/docs/rules/aria-activedescendant-has-tabindex.md): Enforce elements with aria-activedescendant are tabbable.
Expand Down Expand Up @@ -155,6 +156,7 @@ configuration file by mapping each custom component name to a DOM element type.
| :--- | :--- | :--- |
| [accessible-emoji](https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/tree/HEAD/docs/rules/accessible-emoji.md) | off | off |
| [alt-text](https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/tree/HEAD/docs/rules/alt-text.md) | error | error |
| [anchor-ambiguous-text](https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/tree/HEAD/docs/rules/anchor-ambiguous-text.md) | off | off |
| [anchor-has-content](https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/tree/HEAD/docs/rules/anchor-has-content.md) | error | error |
| [anchor-is-valid](https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/tree/HEAD/docs/rules/anchor-is-valid.md) | error | error |
| [aria-activedescendant-has-tabindex](https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/tree/HEAD/docs/rules/aria-activedescendant-has-tabindex.md) | error | error |
Expand Down
97 changes: 97 additions & 0 deletions __tests__/src/rules/anchor-ambiguous-text-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/* eslint-env jest */
/**
* @fileoverview Enforce `<a>` text to not exactly match "click here", "here", "link", or "a link".
* @author Matt Wang
*/

// -----------------------------------------------------------------------------
// Requirements
// -----------------------------------------------------------------------------

import { RuleTester } from 'eslint';
import parserOptionsMapper from '../../__util__/parserOptionsMapper';
import rule from '../../../src/rules/anchor-ambiguous-text';

// -----------------------------------------------------------------------------
// Tests
// -----------------------------------------------------------------------------

const ruleTester = new RuleTester();

const DEFAULT_AMBIGUOUS_WORDS = [
'click here',
'here',
'link',
'a link',
'learn more',
];

const expectedErrorGenerator = (words) => ({
message: `Ambiguous text within anchor. Screenreader users rely on link text for context; the words "${words.join('", "')}" are ambiguous and do not provide enough context.`,
type: 'JSXOpeningElement',
});

const expectedError = expectedErrorGenerator(DEFAULT_AMBIGUOUS_WORDS);

ruleTester.run('anchor-ambiguous-text', rule, {
valid: [
{ code: '<a>documentation</a>;' },
{ code: '<a>${here}</a>;' },
{ code: '<a aria-label="tutorial on using eslint-plugin-jsx-a11y">click here</a>;' },
{ code: '<a><span aria-label="tutorial on using eslint-plugin-jsx-a11y">click here</span></a>;' },
{
code: '<a>click here</a>',
options: [{
words: ['disabling the defaults'],
}],
},
{
code: '<Link>documentation</Link>;',
settings: { 'jsx-a11y': { components: { Link: 'a' } } },
},
{
code: '<Link>${here}</Link>;',
settings: { 'jsx-a11y': { components: { Link: 'a' } } },
},
{
code: '<Link aria-label="tutorial on using eslint-plugin-jsx-a11y">click here</Link>;',
settings: { 'jsx-a11y': { components: { Link: 'a' } } },
},
{
code: '<Link>click here</Link>',
options: [{
words: ['disabling the defaults with components'],
}],
settings: { 'jsx-a11y': { components: { Link: 'a' } } },
},
].map(parserOptionsMapper),
invalid: [
{ code: '<a>here</a>;', errors: [expectedError] },
{ code: '<a>HERE</a>;', errors: [expectedError] },
{ code: '<a>click here</a>;', errors: [expectedError] },
{ code: '<a>learn more</a>;', errors: [expectedError] },
{ code: '<a>link</a>;', errors: [expectedError] },
{ code: '<a>a link</a>;', errors: [expectedError] },
{ code: '<a aria-label="click here">something</a>;', errors: [expectedError] },
{ code: '<a> a link </a>;', errors: [expectedError] },
{ code: '<a>a<i></i> link</a>;', errors: [expectedError] },
{ code: '<a><i></i>a link</a>;', errors: [expectedError] },
{ code: '<a><span>click</span> here</a>;', errors: [expectedError] },
{ code: '<a><span> click </span> here</a>;', errors: [expectedError] },
{ code: '<a><span aria-hidden>more text</span>learn more</a>;', errors: [expectedError] },
{ code: '<a><span aria-hidden="true">more text</span>learn more</a>;', errors: [expectedError] },
{ code: '<a><CustomElement>click</CustomElement> here</a>;', errors: [expectedError] },
{
code: '<Link>here</Link>',
errors: [expectedError],
settings: { 'jsx-a11y': { components: { Link: 'a' } } },
},
{
code: '<a>a disallowed word</a>',
errors: [expectedErrorGenerator(['a disallowed word'])],
options: [{
words: ['a disallowed word'],
}],
},
].map(parserOptionsMapper),
});
96 changes: 96 additions & 0 deletions __tests__/src/util/getAccessibleChildText-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import expect from 'expect';
import { elementType } from 'jsx-ast-utils';
import getAccessibleChildText from '../../../src/util/getAccessibleChildText';
import JSXAttributeMock from '../../../__mocks__/JSXAttributeMock';
import JSXElementMock from '../../../__mocks__/JSXElementMock';

describe('getAccessibleChildText', () => {
it('returns the aria-label when present', () => {
expect(getAccessibleChildText(JSXElementMock(
'a',
[JSXAttributeMock('aria-label', 'foo')],
), elementType)).toBe('foo');
});

it('returns the aria-label instead of children', () => {
expect(getAccessibleChildText(JSXElementMock(
'a',
[JSXAttributeMock('aria-label', 'foo')],
[{ type: 'JSXText', value: 'bar' }],
), elementType)).toBe('foo');
});

it('skips elements with aria-hidden=true', () => {
expect(getAccessibleChildText(JSXElementMock(
'a',
[JSXAttributeMock('aria-hidden', 'true')],
), elementType)).toBe('');
});

it('returns literal value for JSXText child', () => {
expect(getAccessibleChildText(JSXElementMock(
'a',
[],
[{ type: 'JSXText', value: 'bar' }],
), elementType)).toBe('bar');
});

it('returns literal value for JSXText child', () => {
expect(getAccessibleChildText(JSXElementMock(
'a',
[],
[{ type: 'Literal', value: 'bar' }],
), elementType)).toBe('bar');
});

it('returns recursive value for JSXElement child', () => {
expect(getAccessibleChildText(JSXElementMock(
'a',
[],
[JSXElementMock(
'span',
[],
[{ type: 'Literal', value: 'bar' }],
)],
), elementType)).toBe('bar');
});

it('skips children with aria-hidden-true', () => {
expect(getAccessibleChildText(JSXElementMock(
'a',
[],
[JSXElementMock(
'span',
[],
[JSXElementMock(
'span',
[JSXAttributeMock('aria-hidden', 'true')],
)],
)],
), elementType)).toBe('');
});

it('joins multiple children properly - no spacing', () => {
expect(getAccessibleChildText(JSXElementMock(
'a',
[],
[{ type: 'Literal', value: 'foo' }, { type: 'Literal', value: 'bar' }],
), elementType)).toBe('foo bar');
});

it('joins multiple children properly - with spacing', () => {
expect(getAccessibleChildText(JSXElementMock(
'a',
[],
[{ type: 'Literal', value: ' foo ' }, { type: 'Literal', value: ' bar ' }],
), elementType)).toBe('foo bar');
});

it('skips unknown elements', () => {
expect(getAccessibleChildText(JSXElementMock(
'a',
[],
[{ type: 'Literal', value: 'foo' }, { type: 'Unknown' }, { type: 'Literal', value: 'bar' }],
), elementType)).toBe('foo bar');
});
});
70 changes: 70 additions & 0 deletions docs/rules/anchor-ambiguous-text.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# anchor-ambiguous-text

Enforces `<a>` values are not exact matches for the phrases "click here", "here", "link", "a link", or "learn more". Screenreaders announce tags as links/interactive, but rely on values for context. Ambiguous anchor descriptions do not provide sufficient context for users.

## Rule details

This rule takes one optional object argument with the parameter `words`.

```json
{
"rules": {
"jsx-a11y/anchor-ambiguous-text": [2, {
"words": ["click this"],
}],
}
}
```

The `words` option allows users to modify the strings that can be checked for in the anchor text. Useful for specifying other words in other languages. The default value is set by `DEFAULT_AMBIGUOUS_WORDS`:

```js
const DEFAULT_AMBIGUOUS_WORDS = ['click here', 'here', 'link', 'a link', 'learn more'];
```

If an element has the `aria-label` property, its value is used instead of the inner text. Note that the rule still disallows ambiguous `aria-label`s. This rule also skips over elements with `aria-hidden="true"`.

Note that this rule is case-insensitive and trims whitespace. It only looks for **exact matches**.

### Succeed
```jsx
<a>read this tutorial</a> // passes since it is not one of the disallowed words
<a>${here}</a> // this is valid since 'here' is a variable name
<a aria-label="tutorial on using eslint-plugin-jsx-a11y">click here</a> // the aria-label supersedes the inner text
```

### Fail
```jsx
<a>here</a>
<a>HERE</a>
<a>click here</a>
<a>link</a>
<a>a link</a>
<a> a link </a>
<a><span> click </span> here</a> // goes through element children
<a>a<i></i> link</a>
<a><i></i>a link</a>
<a><span aria-hidden="true">more text</span>learn more</a> // skips over elements with aria-hidden=true
<a aria-label="click here">something</a> // the aria-label here is inaccessible
```

## Accessibility guidelines

Ensure anchor tags describe the content of the link, opposed to simply describing them as a link.

Compare

```jsx
<p><a href="#">click here</a> to read a tutorial by Foo Bar</p>
```

which can be more concise and accessible with

```jsx
<p>read <a href="#">a tutorial by Foo Bar</a></p>
```

### Resources

1. [WebAIM, Hyperlinks](https://webaim.org/techniques/hypertext/)
2. [Deque University, Link Checklist - 'Avoid "link" (or similar) in the link text'](https://dequeuniversity.com/checklists/web/links)
2 changes: 2 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module.exports = {
rules: {
'accessible-emoji': require('./rules/accessible-emoji'),
'alt-text': require('./rules/alt-text'),
'anchor-ambiguous-text': require('./rules/anchor-ambiguous-text'),
'anchor-has-content': require('./rules/anchor-has-content'),
'anchor-is-valid': require('./rules/anchor-is-valid'),
'aria-activedescendant-has-tabindex': require('./rules/aria-activedescendant-has-tabindex'),
Expand Down Expand Up @@ -51,6 +52,7 @@ module.exports = {
},
rules: {
'jsx-a11y/alt-text': 'error',
'jsx-a11y/anchor-ambiguous-text': 'off', // TODO: error
'jsx-a11y/anchor-has-content': 'error',
'jsx-a11y/anchor-is-valid': 'error',
'jsx-a11y/aria-activedescendant-has-tabindex': 'error',
Expand Down
71 changes: 71 additions & 0 deletions src/rules/anchor-ambiguous-text.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/**
* @fileoverview Enforce anchor text to not exactly match 'click here', 'here', 'link', 'learn more', and user-specified words.
* @author Matt Wang
* @flow
*/

// ----------------------------------------------------------------------------
// Rule Definition
// ----------------------------------------------------------------------------

import type { ESLintConfig, ESLintContext } from '../../flow/eslint';
import { arraySchema, generateObjSchema } from '../util/schemas';
import getAccessibleChildText from '../util/getAccessibleChildText';
import getElementType from '../util/getElementType';

const DEFAULT_AMBIGUOUS_WORDS = [
'click here',
'here',
'link',
'a link',
'learn more',
];

const schema = generateObjSchema({
words: arraySchema,
});

export default ({
meta: {
docs: {
url: 'https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/tree/HEAD/docs/rules/anchor-ambiguous-text.md',
description: 'Enforce `<a>` text to not exactly match "click here", "here", "link", or "a link".',
},
schema: [schema],
},

create: (context: ESLintContext) => {
const elementType = getElementType(context);

const typesToValidate = ['a'];

const options = context.options[0] || {};
const { words = DEFAULT_AMBIGUOUS_WORDS } = options;
const ambiguousWords = new Set(words);

return {
JSXOpeningElement: (node) => {
const nodeType = elementType(node);

// Only check anchor elements and custom types.
if (typesToValidate.indexOf(nodeType) === -1) {
return;
}

const nodeText = getAccessibleChildText(node.parent, elementType);

if (!ambiguousWords.has(nodeText)) { // check the value
return;
}

context.report({
node,
message: 'Ambiguous text within anchor. Screenreader users rely on link text for context; the words "{{wordsList}}" are ambiguous and do not provide enough context.',
data: {
wordsList: words.join('", "'),
},
});
},
};
},
}: ESLintConfig);
Loading