Skip to content

Commit

Permalink
Add a11y-use-next-tooltip rule (#103)
Browse files Browse the repository at this point in the history
* Add use-next-tooltip rule

* format

* add a fix to replace the path

* add a fix to replace aria-label with text

* fix linting

* update the entry point and add additional fixes

* fix attribute checker to only look for in Tooltip

* rename the rule to a11y-use-next-tooltip

* Create silly-phones-jog.md

* experimental->next

* Add docs link

* docs

* docs

* linting
  • Loading branch information
broccolinisoup authored Jul 18, 2024
1 parent d433ee7 commit 8617a66
Show file tree
Hide file tree
Showing 7 changed files with 230 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .changeset/silly-phones-jog.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-primer-react": minor
---

Add a11y-use-next-tooltip rule that warns against using Tooltip v1 (Not the accessible one)
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,4 @@ ESLint rules for Primer React
- [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)
- [a11y-remove-disable-tooltip](https://github.com/primer/eslint-plugin-primer-react/blob/main/docs/rules/a11y-remove-disable-tooltip.md)
- [a11y-use-next-tooltip](https://github.com/primer/eslint-plugin-primer-react/blob/main/docs/rules/a11y-use-next-tooltip.md)
33 changes: 33 additions & 0 deletions docs/rules/a11y-use-next-tooltip.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Recommends to use the `next` tooltip instead of the current stable one.

## Rule details

This rule recommends using the tooltip that is imported from `@primer/react/next` instead of the main entrypoint. The components that are exported from `@primer/react/next` are recommended over the main entrypoint ones because `next` components are reviewed and remediated for accessibility issues.
👎 Examples of **incorrect** code for this rule:

```tsx
import {Tooltip} from '@primer/react'
```

👍 Examples of **correct** code for this rule:

```tsx
import {Tooltip} from '@primer/react/next'
```

## Icon buttons and tooltips

Even though the below code is perfectly valid, since icon buttons now come with tooltips by default, it is not required to explicitly use the Tooltip component on icon buttons.

```jsx
import {IconButton} from '@primer/react'
import {Tooltip} from '@primer/react/next'

function ExampleComponent() {
return (
<Tooltip text="Search" direction="e">
<IconButton icon={SearchIcon} aria-label="Search" />
</Tooltip>
)
}
```
1 change: 1 addition & 0 deletions src/configs/recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ module.exports = {
'primer-react/a11y-explicit-heading': 'error',
'primer-react/no-deprecated-props': 'warn',
'primer-react/a11y-remove-disable-tooltip': 'error',
'primer-react/a11y-use-next-tooltip': 'error',
},
settings: {
github: {
Expand Down
1 change: 1 addition & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ module.exports = {
'no-deprecated-props': require('./rules/no-deprecated-props'),
'a11y-link-in-text-block': require('./rules/a11y-link-in-text-block'),
'a11y-remove-disable-tooltip': require('./rules/a11y-remove-disable-tooltip'),
'a11y-use-next-tooltip': require('./rules/a11y-use-next-tooltip'),
},
configs: {
recommended: require('./configs/recommended'),
Expand Down
61 changes: 61 additions & 0 deletions src/rules/__tests__/a11y-use-next-tooltip.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
const rule = require('../a11y-use-next-tooltip')
const {RuleTester} = require('eslint')

const ruleTester = new RuleTester({
parserOptions: {
ecmaVersion: 'latest',
sourceType: 'module',
ecmaFeatures: {
jsx: true,
},
},
})

ruleTester.run('a11y-use-next-tooltip', rule, {
valid: [
`import {Tooltip} from '@primer/react/next';`,
`import {UnderlineNav, Button} from '@primer/react';
import {Tooltip} from '@primer/react/next';`,
`import {UnderlineNav, Button} from '@primer/react';
import {Tooltip, SelectPanel} from '@primer/react/next';`,
],
invalid: [
{
code: `import {Tooltip} from '@primer/react';`,
errors: [{messageId: 'useNextTooltip'}],
output: `import {Tooltip} from '@primer/react/next';`,
},
{
code: `import {Tooltip, Button} from '@primer/react';\n<Tooltip aria-label="tooltip text"><Button>Button</Button></Tooltip>`,
errors: [{messageId: 'useNextTooltip'}, {messageId: 'useTextProp'}],
output: `import {Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/next';\n<Tooltip text="tooltip text"><Button>Button</Button></Tooltip>`,
},
{
code: `import {ActionList, ActionMenu, Tooltip, Button} from '@primer/react';\n<Tooltip aria-label="tooltip text"><Button>Button</Button></Tooltip>`,
errors: [
{messageId: 'useNextTooltip', line: 1},
{messageId: 'useTextProp', line: 2},
],
output: `import {ActionList, ActionMenu, Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/next';\n<Tooltip text="tooltip text"><Button>Button</Button></Tooltip>`,
},
{
code: `import {ActionList, ActionMenu, Button, Tooltip} from '@primer/react';\n<Tooltip aria-label="tooltip text"><Button aria-label="test">Button</Button></Tooltip>`,
errors: [
{messageId: 'useNextTooltip', line: 1},
{messageId: 'useTextProp', line: 2},
],
output: `import {ActionList, ActionMenu, Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/next';\n<Tooltip text="tooltip text"><Button aria-label="test">Button</Button></Tooltip>`,
},
{
code: `import {ActionList, ActionMenu, Tooltip, Button} from '@primer/react';\n<Tooltip aria-label="tooltip text" noDelay={true} wrap={true} align="left"><Button>Button</Button></Tooltip>`,
errors: [
{messageId: 'useNextTooltip', line: 1},
{messageId: 'useTextProp', line: 2},
{messageId: 'noDelayRemoved', line: 2},
{messageId: 'wrapRemoved', line: 2},
{messageId: 'alignRemoved', line: 2},
],
output: `import {ActionList, ActionMenu, Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/next';\n<Tooltip text="tooltip text" ><Button>Button</Button></Tooltip>`,
},
],
})
128 changes: 128 additions & 0 deletions src/rules/a11y-use-next-tooltip.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
'use strict'
const url = require('../url')
const {getJSXOpeningElementAttribute} = require('../utils/get-jsx-opening-element-attribute')
const {getJSXOpeningElementName} = require('../utils/get-jsx-opening-element-name')

module.exports = {
meta: {
type: 'suggestion',
docs: {
description: 'recommends the use of @primer/react/next Tooltip component',
category: 'Best Practices',
recommended: true,
url: url(module),
},
fixable: true,
schema: [],
messages: {
useNextTooltip: 'Please use @primer/react/next Tooltip component that has accessibility improvements',
useTextProp: 'Please use the text prop instead of aria-label',
noDelayRemoved: 'noDelay prop is removed. Tooltip now has no delay by default',
wrapRemoved: 'wrap prop is removed. Tooltip now wraps by default',
alignRemoved: 'align prop is removed. Please use the direction prop instead.',
},
},
create(context) {
return {
ImportDeclaration(node) {
if (node.source.value !== '@primer/react') {
return
}
const hasTooltip = node.specifiers.some(
specifier => specifier.imported && specifier.imported.name === 'Tooltip',
)

const hasOtherImports = node.specifiers.length > 1
if (!hasTooltip) {
return
}
context.report({
node,
messageId: 'useNextTooltip',
fix(fixer) {
// If Tooltip is the only import, replace the whole import statement
if (!hasOtherImports) {
return fixer.replaceText(node.source, `'@primer/react/next'`)
} else {
// Otherwise, remove Tooltip from the import statement and add a new import statement with the correct path
const tooltipSpecifier = node.specifiers.find(
specifier => specifier.imported && specifier.imported.name === 'Tooltip',
)

const tokensToRemove = [' ', ',']
const tooltipIsFirstImport = tooltipSpecifier === node.specifiers[0]
const tooltipIsLastImport = tooltipSpecifier === node.specifiers[node.specifiers.length - 1]
const tooltipIsNotFirstOrLastImport = !tooltipIsFirstImport && !tooltipIsLastImport

const sourceCode = context.getSourceCode()
const canRemoveBefore = tooltipIsNotFirstOrLastImport
? false
: tokensToRemove.includes(sourceCode.getTokenBefore(tooltipSpecifier).value)
const canRemoveAfter = tokensToRemove.includes(sourceCode.getTokenAfter(tooltipSpecifier).value)
const start = canRemoveBefore
? sourceCode.getTokenBefore(tooltipSpecifier).range[0]
: tooltipSpecifier.range[0]
const end = canRemoveAfter
? sourceCode.getTokenAfter(tooltipSpecifier).range[1] + 1
: tooltipSpecifier.range[1]
return [
// remove tooltip specifier and the space and comma after it
fixer.removeRange([start, end]),
fixer.insertTextAfterRange(
[node.range[1], node.range[1]],
`\nimport {Tooltip} from '@primer/react/next';`,
),
]
}
},
})
},
JSXOpeningElement(node) {
const openingElName = getJSXOpeningElementName(node)
if (openingElName !== 'Tooltip') {
return
}
const ariaLabel = getJSXOpeningElementAttribute(node, 'aria-label')
if (ariaLabel !== undefined) {
context.report({
node,
messageId: 'useTextProp',
fix(fixer) {
return fixer.replaceText(ariaLabel.name, 'text')
},
})
}
const noDelay = getJSXOpeningElementAttribute(node, 'noDelay')
if (noDelay !== undefined) {
context.report({
node,
messageId: 'noDelayRemoved',
fix(fixer) {
return fixer.remove(noDelay)
},
})
}
const wrap = getJSXOpeningElementAttribute(node, 'wrap')
if (wrap !== undefined) {
context.report({
node,
messageId: 'wrapRemoved',
fix(fixer) {
return fixer.remove(wrap)
},
})
}
const align = getJSXOpeningElementAttribute(node, 'align')
if (align !== undefined) {
context.report({
node,
messageId: 'alignRemoved',
fix(fixer) {
return fixer.remove(align)
},
})
}
},
}
},
}

0 comments on commit 8617a66

Please sign in to comment.