Skip to content

Commit

Permalink
Clean up docs for link lint rule (#195)
Browse files Browse the repository at this point in the history
* Clean up docs, make clear that this rule is experimental

* Fix lint issue

* Create lemon-turtles-talk.md
  • Loading branch information
khiga8 committed Jun 12, 2024
1 parent 5fc19f6 commit ccc2e99
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 10 deletions.
5 changes: 5 additions & 0 deletions .changeset/lemon-turtles-talk.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-primer-react": patch
---

Clean up docs for link lint rule
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ ESLint rules for Primer React

- [direct-slot-children](https://github.com/primer/eslint-plugin-primer-react/blob/main/docs/rules/direct-slot-children.md)
- [no-system-props](https://github.com/primer/eslint-plugin-primer-react/blob/main/docs/rules/no-system-props.md)
- [a11y-tooltip-interactive-trigger](https://github.com/primer/eslint-plugin-primer-react/blob/main/docs/rules/a11y-tooltip-interactive-trigger.md)
- [a11y-explicit-heading](https://github.com/primer/eslint-plugin-primer-react/blob/main/docs/rules/a11y-explicit-heading.md)
- [new-css-color-vars](https://github.com/primer/eslint-plugin-primer-react/blob/main/docs/rules/new-css-color-vars.md)
- [no-deprecated-props](https://github.com/primer/eslint-plugin-primer-react/blob/main/docs/rules/no-deprecated-props.md)
- [a11y-tooltip-interactive-trigger](https://github.com/primer/eslint-plugin-primer-react/blob/main/docs/rules/a11y-tooltip-interactive-trigger.md)
- [a11y-explicit-heading](https://github.com/primer/eslint-plugin-primer-react/blob/main/docs/rules/a11y-explicit-heading.md)
- [a11y-link-in-text-block](https://github.com/primer/eslint-plugin-primer-react/blob/main/docs/rules/a11y-link-in-text-block.md)
27 changes: 20 additions & 7 deletions docs/rules/a11y-link-in-text-block.md
Original file line number Diff line number Diff line change
@@ -1,22 +1,25 @@
## Require `inline` prop on `<Link>` component inside a text block
# EXPERIMENTAL: Require `inline` prop on `<Link>` in text block

The `Link` component should have the `inline` prop when it is used inside of a text block.
This is an experimental rule. If you suspect any false positives reported by this rule, please file an issue so we can make this rule better.

## Rule Details

This rule enforces setting `inline` on the `<Link>` component when a `<Link>` is detected inside of a text block without distiguishable styling.
The `Link` component should have the `inline` prop when it is used within a text block and has no styles (aside from color) to distinguish itself from surrounding plain text.

The lint rule will essentially flag any `<Link>` without the `inline` property (equal to `true`) detected with string nodes on either side.
Related: [WCAG 1.4.1 Use of Color issues](https://www.w3.org/WAI/WCAG21/Understanding/use-of-color.html)

This rule will not catch all instances of link in text block due to the limitations of static analysis, so be sure to also have in-browser checks in place such as the [link-in-text-block Axe rule](https://dequeuniversity.com/rules/axe/4.9/link-in-text-block) for additional coverage.
The lint rule will flag any `<Link>` without the `inline` property (equal to `true`) detected with string nodes on either side.

The edge cases that the linter skips to avoid false positives will include:
There are certain edge cases that the linter skips to avoid false positives including:

- `<Link className="...">` because there may be distinguishing styles applied.
- `<Link sx={{fontWeight:...}}>` or `<Link sx={{fontFamily:...}}>` because these technically may provide sufficient distinguishing styling.
- `<Link>` where the only adjacent text is a period, since that can't really be considered a text block.
- `<Link>` where the children is a JSX component, rather than a string literal, because then it might be an icon link rather than a text link.
- `<Link>` that are nested inside of headings as these have often been breadcrumbs.

This rule will not catch all instances of link in text block due to the limitations of static analysis, so be sure to also have in-browser checks in place such as the [link-in-text-block Axe rule](https://dequeuniversity.com/rules/axe/4.9/link-in-text-block) for additional coverage.

👎 Examples of **incorrect** code for this rule

```jsx
Expand Down Expand Up @@ -93,8 +96,18 @@ function ExampleComponent() {
}
```

This rule will skip `Link`s with a `className`.

```jsx
function ExampleComponent() {
return (
Learn more at <Link className={styles.someDistinguishingStyle}>GitHub</Link>
)
}
```

## Options

- `skipImportCheck` (default: `false`)

By default, the `a11y-explicit-heading` rule will only check for `<Heading>` components imported directly from `@primer/react`. You can disable this behavior by setting `skipImportCheck` to `true`.
By default, the `a11y-link-in-text-block` rule will only check for `<Link>` components imported directly from `@primer/react`. You can disable this behavior by setting `skipImportCheck` to `true`.
6 changes: 5 additions & 1 deletion src/rules/a11y-link-in-text-block.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ const {getJSXOpeningElementAttribute} = require('../utils/get-jsx-opening-elemen

module.exports = {
meta: {
docs: {
url: require('../url')(module),
},
type: 'problem',
schema: [
{
Expand All @@ -15,7 +18,8 @@ module.exports = {
},
],
messages: {
linkInTextBlock: '<Link>s that are used within a text block should have the inline prop.',
linkInTextBlock:
'Links should have the inline prop if it appear in a text block and only uses color to distinguish itself from surrounding text.',
},
},
create(context) {
Expand Down

0 comments on commit ccc2e99

Please sign in to comment.