Skip to content

Commit

Permalink
Add a rule that warn against removing unsafeDisableTooltip prop fro…
Browse files Browse the repository at this point in the history
…m `IconButton` (#185)

* add a rule to remove unsafeDisableTooltip prop

* add changeset

* Update src/rules/a11y-remove-disable-tooltip.js

Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com>

* unsafeDisableTooltip={false} is also invalid - the prop should be removed.

* export the rule - duh

---------

Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com>
  • Loading branch information
broccolinisoup and khiga8 committed Jun 13, 2024
1 parent 2ff10af commit fea642d
Show file tree
Hide file tree
Showing 7 changed files with 130 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .changeset/slow-numbers-invite.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-primer-react": minor
---

Add a rule that warns against removing `unsafeDisableTooltip` prop
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,4 @@ ESLint rules for Primer React
- [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)
- [a11y-remove-disable-tooltip](https://github.com/primer/eslint-plugin-primer-react/blob/main/docs/rules/a11y-remove-disable-tooltip.md)
25 changes: 25 additions & 0 deletions docs/rules/a11y-remove-disable-tooltip.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
## Rule Details

This rule enforces to remove the `unsafeDisableTooltip` from `IconButton` component so that they have a tooltip by default. `unsafeDisableTooltip` prop is created for an incremental migration and should be removed once all icon buttons have a tooltip.

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

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

const App = () => (
<IconButton icon={SearchIcon} aria-label="Search" unsafeDisableTooltip />
// OR
<IconButton icon={SearchIcon} aria-label="Search" unsafeDisableTooltip={true} />
// OR
<IconButton icon={SearchIcon} aria-label="Search" unsafeDisableTooltip={false} /> // This is incorrect because it should be removed
)
```

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

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

const App = () => <IconButton icon={SearchIcon} aria-label="Search" />
```
1 change: 1 addition & 0 deletions src/configs/recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ module.exports = {
'primer-react/new-color-css-vars': 'error',
'primer-react/a11y-explicit-heading': 'error',
'primer-react/no-deprecated-props': 'warn',
'primer-react/a11y-remove-disable-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 @@ -8,6 +8,7 @@ module.exports = {
'a11y-explicit-heading': require('./rules/a11y-explicit-heading'),
'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'),
},
configs: {
recommended: require('./configs/recommended'),
Expand Down
50 changes: 50 additions & 0 deletions src/rules/__tests__/a11y-remove-disable-tooltip.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
'use strict'

const {RuleTester} = require('eslint')
const rule = require('../a11y-remove-disable-tooltip')

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

ruleTester.run('a11y-remove-disable-tooltip', rule, {
valid: [
`import {IconButton} from '@primer/react';
<IconButton icon={SearchIcon} aria-label="Search" />`,
],
invalid: [
{
code: `<IconButton icon={SearchIcon} aria-label="Search" unsafeDisableTooltip />`,
output: `<IconButton icon={SearchIcon} aria-label="Search" />`,
errors: [
{
messageId: 'removeDisableTooltipProp',
},
],
},
{
code: `<IconButton icon={SearchIcon} aria-label="Search" unsafeDisableTooltip={true} />`,
output: `<IconButton icon={SearchIcon} aria-label="Search" />`,
errors: [
{
messageId: 'removeDisableTooltipProp',
},
],
},
{
code: `<IconButton icon={SearchIcon} aria-label="Search" unsafeDisableTooltip={false} />`,
output: `<IconButton icon={SearchIcon} aria-label="Search" />`,
errors: [
{
messageId: 'removeDisableTooltipProp',
},
],
},
],
})
47 changes: 47 additions & 0 deletions src/rules/a11y-remove-disable-tooltip.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
'use strict'
const {getJSXOpeningElementAttribute} = require('../utils/get-jsx-opening-element-attribute')
const {getJSXOpeningElementName} = require('../utils/get-jsx-opening-element-name')

/**
* @type {import('eslint').Rule.RuleModule}
*/
module.exports = {
meta: {
type: 'error',
docs: {
description:
'Icon buttons should have tooltip by default. Please remove `unsafeDisableTooltip` prop from `IconButton` component to enable the tooltip and help making icon button more accessible.',
recommended: true,
},
fixable: 'code',
schema: [],
messages: {
removeDisableTooltipProp:
'Please remove `unsafeDisableTooltip` prop from `IconButton` component to enable the tooltip and help make icon button more accessible.',
},
},
create(context) {
return {
JSXOpeningElement(node) {
const openingElName = getJSXOpeningElementName(node)
if (openingElName !== 'IconButton') {
return
}
const unsafeDisableTooltip = getJSXOpeningElementAttribute(node, 'unsafeDisableTooltip')
if (unsafeDisableTooltip !== undefined) {
context.report({
node,
messageId: 'removeDisableTooltipProp',
fix(fixer) {
const start = unsafeDisableTooltip.range[0]
const end = unsafeDisableTooltip.range[1]
return [
fixer.removeRange([start - 1, end]), // remove the space before unsafeDisableTooltip as well
]
},
})
}
},
}
},
}

0 comments on commit fea642d

Please sign in to comment.