-
Notifications
You must be signed in to change notification settings - Fork 59
Add accessibility rule and React config #282
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
Changes from all commits
ff98741
fa7dd7e
b69e44f
ac1edd7
de77ce7
57dab5d
19eca61
e39e9c6
78b7eeb
c1733dc
af98a18
cd9d7eb
eb5d423
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
# No Generic Link Text | ||
|
||
## Rule Details | ||
|
||
Avoid setting generic link text like, "Click here", "Read more", and "Learn more" which do not make sense when read out of context. | ||
|
||
Screen reader users often tab through links on a page to quickly find content without needing to listen to the full page. When link text is too generic, it becomes difficult to quickly identify the destination of the link. While it is possible to provide a more specific link text by setting the `aria-label`, this results in divergence between the label and the text and is not an ideal, future-proof solution. | ||
|
||
Additionally, generic link text can also problematic for heavy zoom users where the link context is out of view. | ||
|
||
Ensure that your link text is descriptive and the purpose of the link is clear even when read out of context of surrounding text. | ||
Learn more about how to write descriptive link text at [Access Guide: Write descriptive link text](https://www.accessguide.io/guide/descriptive-link-text) | ||
|
||
### Use of ARIA attributes | ||
|
||
If you _must_ use ARIA to replace the visible link text, include the visible text at the beginning. | ||
|
||
For example, on a pricing plans page, the following are good: | ||
|
||
- Visible text: `Learn more` | ||
- Accessible label: `Learn more about GitHub pricing plans` | ||
|
||
Accessible ✅ | ||
|
||
```html | ||
<a href="..." aria-label="Learn more about GitHub pricing plans">Learn more</a> | ||
``` | ||
|
||
Inaccessible 🚫 | ||
|
||
```html | ||
<a href="..." aria-label="GitHub pricing plans">Learn more</a> | ||
``` | ||
|
||
Including the visible text in the ARIA label satisfies [SC 2.5.3: Label in Name](https://www.w3.org/WAI/WCAG21/Understanding/label-in-name.html). | ||
|
||
#### False negatives | ||
|
||
Caution: because of the restrictions of static code analysis, we may not catch all violations. | ||
|
||
Please perform browser tests and spot checks: | ||
|
||
- when `aria-label` is set dynamically | ||
- when using `aria-labelledby` | ||
|
||
## Resources | ||
|
||
- [Primer: Links](https://primer.style/design/accessibility/links) | ||
- [Understanding Success Criterion 2.4.4: Link Purpose (In Context)](https://www.w3.org/WAI/WCAG21/Understanding/link-purpose-in-context.html) | ||
- [WebAim: Links and Hypertext](https://webaim.org/techniques/hypertext/) | ||
- [Deque: Use link text that make sense when read out of context](https://dequeuniversity.com/tips/link-text) | ||
|
||
## Examples | ||
|
||
### **Incorrect** code for this rule 👎 | ||
|
||
```jsx | ||
<a href="github.com/about">Learn more</a> | ||
``` | ||
|
||
```jsx | ||
<a href="github.com/about">Read more</a> | ||
``` | ||
|
||
```jsx | ||
<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> | ||
``` | ||
|
||
### **Correct** code for this rule 👍 | ||
|
||
```jsx | ||
<a href="github.com/about">Learn more about GitHub</a> | ||
``` | ||
|
||
```jsx | ||
<a href="github.com/new">Create a new repository</a> | ||
``` | ||
|
||
## Version |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
module.exports = { | ||
env: { | ||
browser: true | ||
}, | ||
plugins: ['github', 'jsx-a11y'], | ||
extends: ['plugin:jsx-a11y/recommended'], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm assuming that any config we want to recommend as part of "react" belongs here, so I added the |
||
rules: { | ||
'github/a11y-no-generic-link-text': 'error' | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
const {elementType, getProp, getPropValue} = require('jsx-ast-utils') | ||
|
||
const bannedLinkText = ['read more', 'here', 'click here', 'learn more', 'more'] | ||
|
||
/* Downcase and strip extra whitespaces and punctuation */ | ||
const stripAndDowncaseText = text => { | ||
return text | ||
.toLowerCase() | ||
.replace(/[.,/#!$%^&*;:{}=\-_`~()]/g, '') | ||
.replace(/\s{2,}/g, ' ') | ||
.trim() | ||
} | ||
|
||
module.exports = { | ||
meta: { | ||
docs: { | ||
description: 'disallow generic link text', | ||
url: require('../url')(module) | ||
}, | ||
schema: [] | ||
}, | ||
|
||
create(context) { | ||
return { | ||
JSXOpeningElement: node => { | ||
if (elementType(node) !== 'a') return | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In general we will probably use the I don't think we can catch all of the possible cases just using a linter, but it might be more thorough to apply this check to any element with an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a follow up PR to address this lack of support for linting custom components. I'd love your feedback there -- #283. |
||
if (getProp(node.attributes, 'aria-labelledby')) return | ||
|
||
let cleanTextContent // text content we can reliably fetch | ||
|
||
const parent = node.parent | ||
let jsxTextNode | ||
if (parent.children && parent.children.length > 0 && parent.children[0].type === 'JSXText') { | ||
jsxTextNode = parent.children[0] | ||
cleanTextContent = stripAndDowncaseText(parent.children[0].value) | ||
} | ||
|
||
const ariaLabel = getPropValue(getProp(node.attributes, 'aria-label')) | ||
const cleanAriaLabelValue = ariaLabel && stripAndDowncaseText(ariaLabel) | ||
|
||
if (ariaLabel) { | ||
if (bannedLinkText.includes(cleanAriaLabelValue)) { | ||
context.report({ | ||
node, | ||
message: | ||
'Avoid setting generic link text like `Here`, `Click here`, `Read more`. Make sure that your link text is both descriptive and concise.' | ||
}) | ||
} | ||
if (cleanTextContent && !cleanAriaLabelValue.includes(cleanTextContent)) { | ||
context.report({ | ||
node, | ||
message: 'When using ARIA to set a more descriptive text, it must fully contain the visible label.' | ||
}) | ||
} | ||
} else { | ||
if (cleanTextContent) { | ||
if (!bannedLinkText.includes(cleanTextContent)) return | ||
context.report({ | ||
node: jsxTextNode, | ||
message: | ||
'Avoid setting generic link text like `Here`, `Click here`, `Read more`. Make sure that your link text is both descriptive and concise.' | ||
}) | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I should distinguish this config as being accessibility rules for react....like
github/jsx-a11y
. (We don't currently have any other React rules so it's currently justgithub/react
.)thoughts? @github/accessibility-reviewers . also open to other suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for future rules github/react is kinda nice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like having a11y-specific rules! But if it's too early to make that distinction until there are more react rules & accessibility rules, I am fine with this for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'm not sure either... I'm down to keep this for now and revisit later!