Skip to content
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

Clean up docs for link lint rule #195

Merged
merged 3 commits into from
Jun 12, 2024
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
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
Loading