From 8617a66b2afe7e4e7a8608dbf5744999fffd9140 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arma=C4=9Fan?= Date: Thu, 18 Jul 2024 14:44:42 +1000 Subject: [PATCH] Add a11y-use-next-tooltip rule (#103) * 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 --- .changeset/silly-phones-jog.md | 5 + README.md | 1 + docs/rules/a11y-use-next-tooltip.md | 33 +++++ src/configs/recommended.js | 1 + src/index.js | 1 + .../__tests__/a11y-use-next-tooltip.test.js | 61 +++++++++ src/rules/a11y-use-next-tooltip.js | 128 ++++++++++++++++++ 7 files changed, 230 insertions(+) create mode 100644 .changeset/silly-phones-jog.md create mode 100644 docs/rules/a11y-use-next-tooltip.md create mode 100644 src/rules/__tests__/a11y-use-next-tooltip.test.js create mode 100644 src/rules/a11y-use-next-tooltip.js diff --git a/.changeset/silly-phones-jog.md b/.changeset/silly-phones-jog.md new file mode 100644 index 00000000..1ad58b5f --- /dev/null +++ b/.changeset/silly-phones-jog.md @@ -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) diff --git a/README.md b/README.md index b2a22db4..eb90a910 100644 --- a/README.md +++ b/README.md @@ -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) diff --git a/docs/rules/a11y-use-next-tooltip.md b/docs/rules/a11y-use-next-tooltip.md new file mode 100644 index 00000000..18659934 --- /dev/null +++ b/docs/rules/a11y-use-next-tooltip.md @@ -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 ( + + + + ) +} +``` diff --git a/src/configs/recommended.js b/src/configs/recommended.js index 7b4aa489..916be1fa 100644 --- a/src/configs/recommended.js +++ b/src/configs/recommended.js @@ -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: { diff --git a/src/index.js b/src/index.js index 5ec93def..20e1fb01 100644 --- a/src/index.js +++ b/src/index.js @@ -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'), diff --git a/src/rules/__tests__/a11y-use-next-tooltip.test.js b/src/rules/__tests__/a11y-use-next-tooltip.test.js new file mode 100644 index 00000000..64115c7c --- /dev/null +++ b/src/rules/__tests__/a11y-use-next-tooltip.test.js @@ -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`, + errors: [{messageId: 'useNextTooltip'}, {messageId: 'useTextProp'}], + output: `import {Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/next';\n`, + }, + { + code: `import {ActionList, ActionMenu, Tooltip, Button} from '@primer/react';\n`, + errors: [ + {messageId: 'useNextTooltip', line: 1}, + {messageId: 'useTextProp', line: 2}, + ], + output: `import {ActionList, ActionMenu, Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/next';\n`, + }, + { + code: `import {ActionList, ActionMenu, Button, Tooltip} from '@primer/react';\n`, + errors: [ + {messageId: 'useNextTooltip', line: 1}, + {messageId: 'useTextProp', line: 2}, + ], + output: `import {ActionList, ActionMenu, Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/next';\n`, + }, + { + code: `import {ActionList, ActionMenu, Tooltip, Button} from '@primer/react';\n`, + 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`, + }, + ], +}) diff --git a/src/rules/a11y-use-next-tooltip.js b/src/rules/a11y-use-next-tooltip.js new file mode 100644 index 00000000..ab1a6e3c --- /dev/null +++ b/src/rules/a11y-use-next-tooltip.js @@ -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) + }, + }) + } + }, + } + }, +}