From bb2bff6af3f82e57b94703ea99fcc13058eea168 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 6 Nov 2024 15:29:30 +0800 Subject: [PATCH 1/7] Update the tooltip eslint rule as we now export the new one from main bundle and the old one from deprecated --- README.md | 2 +- ...ltip.md => a11y-use-accessible-tooltip.md} | 7 +- src/configs/recommended.js | 2 +- src/index.js | 2 +- .../a11y-use-accessible-tooltip.test.js | 59 +++++++ .../__tests__/a11y-use-next-tooltip.test.js | 61 ------- src/rules/a11y-use-accessible-tooltip.js | 162 ++++++++++++++++++ src/rules/a11y-use-next-tooltip.js | 128 -------------- 8 files changed, 227 insertions(+), 196 deletions(-) rename docs/rules/{a11y-use-next-tooltip.md => a11y-use-accessible-tooltip.md} (88%) create mode 100644 src/rules/__tests__/a11y-use-accessible-tooltip.test.js delete mode 100644 src/rules/__tests__/a11y-use-next-tooltip.test.js create mode 100644 src/rules/a11y-use-accessible-tooltip.js delete mode 100644 src/rules/a11y-use-next-tooltip.js diff --git a/README.md b/README.md index eb90a910..03a4c86b 100644 --- a/README.md +++ b/README.md @@ -39,4 +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) +- [a11y-use-accessible-tooltip](https://github.com/primer/eslint-plugin-primer-react/blob/main/docs/rules/a11y-use-accessible-tooltip.md) diff --git a/docs/rules/a11y-use-next-tooltip.md b/docs/rules/a11y-use-accessible-tooltip.md similarity index 88% rename from docs/rules/a11y-use-next-tooltip.md rename to docs/rules/a11y-use-accessible-tooltip.md index 18659934..785711d8 100644 --- a/docs/rules/a11y-use-next-tooltip.md +++ b/docs/rules/a11y-use-accessible-tooltip.md @@ -6,13 +6,13 @@ This rule recommends using the tooltip that is imported from `@primer/react/next 👎 Examples of **incorrect** code for this rule: ```tsx -import {Tooltip} from '@primer/react' +import {Tooltip} from '@primer/react/deprecated' ``` 👍 Examples of **correct** code for this rule: ```tsx -import {Tooltip} from '@primer/react/next' +import {Tooltip} from '@primer/react' ``` ## Icon buttons and tooltips @@ -20,8 +20,7 @@ import {Tooltip} from '@primer/react/next' 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' +import {IconButton, Tooltip} from '@primer/react' function ExampleComponent() { return ( diff --git a/src/configs/recommended.js b/src/configs/recommended.js index 3bb02d23..b351bb00 100644 --- a/src/configs/recommended.js +++ b/src/configs/recommended.js @@ -17,7 +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', + 'primer-react/a11y-use-accessible-tooltip': 'error', 'primer-react/no-unnecessary-components': 'error', 'primer-react/prefer-action-list-item-onselect': 'error', }, diff --git a/src/index.js b/src/index.js index bc284b02..e74e08ee 100644 --- a/src/index.js +++ b/src/index.js @@ -9,7 +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'), + 'a11y-use-accessible-tooltip': require('./rules/a11y-use-accessible-tooltip'), 'use-deprecated-from-deprecated': require('./rules/use-deprecated-from-deprecated'), 'no-wildcard-imports': require('./rules/no-wildcard-imports'), 'no-unnecessary-components': require('./rules/no-unnecessary-components'), diff --git a/src/rules/__tests__/a11y-use-accessible-tooltip.test.js b/src/rules/__tests__/a11y-use-accessible-tooltip.test.js new file mode 100644 index 00000000..bc99ebb3 --- /dev/null +++ b/src/rules/__tests__/a11y-use-accessible-tooltip.test.js @@ -0,0 +1,59 @@ +const rule = require('../a11y-use-accessible-tooltip') +const {RuleTester} = require('eslint') + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 'latest', + sourceType: 'module', + ecmaFeatures: { + jsx: true, + }, + }, +}) + +ruleTester.run('a11y-use-accessible-tooltip', rule, { + valid: [`import {Tooltip} from '@primer/react';`], + invalid: [ + // Single import from deprecated + { + code: `import {Tooltip} from '@primer/react/deprecated';`, + errors: [{messageId: 'useAccessibleTooltip'}], + output: `import {Tooltip} from '@primer/react';`, + }, + // Multiple imports from deprecated + { + code: `import {Tooltip, Button} from '@primer/react/deprecated';`, + errors: [{messageId: 'useAccessibleTooltip'}], + output: `import {Button} from '@primer/react/deprecated';\nimport {Tooltip} from '@primer/react';`, + }, + // Multiple imports from deprecated + { + code: `import {Button, Tooltip, Stack} from '@primer/react/deprecated';`, + errors: [{messageId: 'useAccessibleTooltip'}], + output: `import {Button, Stack} from '@primer/react/deprecated';\nimport {Tooltip} from '@primer/react';`, + }, + // Single import from deprecated with an existing import from @primer/react + { + code: `import {Tooltip} from '@primer/react/deprecated';import {ActionList, ActionMenu} from '@primer/react';`, + errors: [{messageId: 'useAccessibleTooltip', line: 1}], + output: `import {ActionList, ActionMenu, Tooltip} from '@primer/react';`, + }, + // Multiple imports from deprecated with an existing import from @primer/react + { + code: `import {Dialog, Tooltip} from '@primer/react/deprecated';import {ActionList, ActionMenu} from '@primer/react';`, + errors: [{messageId: 'useAccessibleTooltip', line: 1}], + output: `import {Dialog} from '@primer/react/deprecated';import {ActionList, ActionMenu, Tooltip} from '@primer/react';`, + }, + { + code: `import {ActionList, ActionMenu, Tooltip, Button} from '@primer/react/deprecated';\n`, + errors: [ + {messageId: 'useAccessibleTooltip', 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/deprecated';\nimport {Tooltip} from '@primer/react';\n`, + }, + ], +}) diff --git a/src/rules/__tests__/a11y-use-next-tooltip.test.js b/src/rules/__tests__/a11y-use-next-tooltip.test.js deleted file mode 100644 index 64115c7c..00000000 --- a/src/rules/__tests__/a11y-use-next-tooltip.test.js +++ /dev/null @@ -1,61 +0,0 @@ -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-accessible-tooltip.js b/src/rules/a11y-use-accessible-tooltip.js new file mode 100644 index 00000000..f415e721 --- /dev/null +++ b/src/rules/a11y-use-accessible-tooltip.js @@ -0,0 +1,162 @@ +'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 Tooltip component', + category: 'Best Practices', + recommended: true, + url: url(module), + }, + fixable: true, + schema: [], + messages: { + useAccessibleTooltip: 'Please use @primer/react 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/deprecated') { + return + } + const hasTooltip = node.specifiers.some( + specifier => specifier.imported && specifier.imported.name === 'Tooltip', + ) + + if (!hasTooltip) { + return + } + + const hasOtherImports = node.specifiers.length > 1 + + const sourceCode = context.getSourceCode() + // Checking to see if there is an existing root (@primer/react) import + const rootImport = sourceCode.ast.body.filter(statement => { + return statement.type === 'ImportDeclaration' && statement.source.value === '@primer/react' + }) + + const tooltipSpecifier = node.specifiers.find( + specifier => specifier.imported && specifier.imported.name === 'Tooltip', + ) + + const hasRootImport = rootImport.length >= 1 + + context.report({ + node, + messageId: 'useAccessibleTooltip', + fix(fixer) { + const fixes = [] + // If Tooltip is the only import and no existing @primer/react import, replace the whole import statement + if (!hasOtherImports && !hasRootImport) { + fixes.push(fixer.replaceText(node.source, `'@primer/react'`)) + } else if (hasOtherImports && !hasRootImport) { + // There are other imports from the deprecated bundle but no existing @primer/react import, so remove the Tooltip import and add a new import statement with the correct path. + const previousToken = sourceCode.getTokenBefore(tooltipSpecifier) + const nextToken = sourceCode.getTokenAfter(tooltipSpecifier) + const hasTrailingComma = nextToken && nextToken.value === ',' + const hasLeadingComma = previousToken && previousToken.value === ',' + + let rangeToRemove + + if (hasTrailingComma) { + rangeToRemove = [tooltipSpecifier.range[0], nextToken.range[1] + 1] + } else if (hasLeadingComma) { + rangeToRemove = [previousToken.range[0], tooltipSpecifier.range[1]] + } else { + rangeToRemove = [tooltipSpecifier.range[0], tooltipSpecifier.range[1]] + } + + fixes.push(fixer.removeRange(rangeToRemove)) + fixes.push(fixer.insertTextAfter(node, `\nimport {Tooltip} from '@primer/react';`)) + } else { + if (!hasOtherImports) { + // remove the entire import statement + fixes.push(fixer.remove(node)) + } else { + const previousToken = sourceCode.getTokenBefore(tooltipSpecifier) + const nextToken = sourceCode.getTokenAfter(tooltipSpecifier) + const hasTrailingComma = nextToken && nextToken.value === ',' + const hasLeadingComma = previousToken && previousToken.value === ',' + + let rangeToRemove + + if (hasTrailingComma) { + rangeToRemove = [tooltipSpecifier.range[0], nextToken.range[1] + 1] + } else if (hasLeadingComma) { + rangeToRemove = [previousToken.range[0], tooltipSpecifier.range[1]] + } else { + rangeToRemove = [tooltipSpecifier.range[0], tooltipSpecifier.range[1]] + } + + fixes.push(fixer.removeRange(rangeToRemove)) + } + // find the last specifier in the existing @primer/react import and insert Tooltip after that + // const rootImport = sourceCode.ast.body.find(statement => { + // return statement.type === 'ImportDeclaration' && statement.source.value === '@primer/react' + // }) + const lastSpecifier = rootImport.specifiers[rootImport.specifiers.length - 1] + fixes.push(fixer.insertTextAfter(lastSpecifier, `, Tooltip`)) + } + + return fixes + }, + }) + }, + 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) + }, + }) + } + }, + } + }, +} diff --git a/src/rules/a11y-use-next-tooltip.js b/src/rules/a11y-use-next-tooltip.js deleted file mode 100644 index ab1a6e3c..00000000 --- a/src/rules/a11y-use-next-tooltip.js +++ /dev/null @@ -1,128 +0,0 @@ -'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) - }, - }) - } - }, - } - }, -} From d56bb699f19f1ccd21590ca8ca22634cd09a36f8 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 6 Nov 2024 15:30:19 +0800 Subject: [PATCH 2/7] oops --- src/rules/a11y-use-accessible-tooltip.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rules/a11y-use-accessible-tooltip.js b/src/rules/a11y-use-accessible-tooltip.js index f415e721..53b70e07 100644 --- a/src/rules/a11y-use-accessible-tooltip.js +++ b/src/rules/a11y-use-accessible-tooltip.js @@ -100,9 +100,9 @@ module.exports = { fixes.push(fixer.removeRange(rangeToRemove)) } // find the last specifier in the existing @primer/react import and insert Tooltip after that - // const rootImport = sourceCode.ast.body.find(statement => { - // return statement.type === 'ImportDeclaration' && statement.source.value === '@primer/react' - // }) + const rootImport = sourceCode.ast.body.find(statement => { + return statement.type === 'ImportDeclaration' && statement.source.value === '@primer/react' + }) const lastSpecifier = rootImport.specifiers[rootImport.specifiers.length - 1] fixes.push(fixer.insertTextAfter(lastSpecifier, `, Tooltip`)) } From 295439e173204f4e0232ec9dcdb8c084409afce3 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 6 Nov 2024 15:40:38 +0800 Subject: [PATCH 3/7] just make the code a lil better --- src/rules/a11y-use-accessible-tooltip.js | 57 ++++++++++-------------- 1 file changed, 24 insertions(+), 33 deletions(-) diff --git a/src/rules/a11y-use-accessible-tooltip.js b/src/rules/a11y-use-accessible-tooltip.js index 53b70e07..9fb7aa71 100644 --- a/src/rules/a11y-use-accessible-tooltip.js +++ b/src/rules/a11y-use-accessible-tooltip.js @@ -55,10 +55,20 @@ module.exports = { messageId: 'useAccessibleTooltip', fix(fixer) { const fixes = [] - // If Tooltip is the only import and no existing @primer/react import, replace the whole import statement - if (!hasOtherImports && !hasRootImport) { - fixes.push(fixer.replaceText(node.source, `'@primer/react'`)) - } else if (hasOtherImports && !hasRootImport) { + if (!hasOtherImports) { + // If Tooltip is the only import and no existing @primer/react import, replace the whole import statement + if (!hasRootImport) fixes.push(fixer.replaceText(node.source, `'@primer/react'`)) + if (hasRootImport) { + // remove the entire import statement + fixes.push(fixer.remove(node)) + // find the last specifier in the existing @primer/react import and insert Tooltip after that + const rootImport = sourceCode.ast.body.find(statement => { + return statement.type === 'ImportDeclaration' && statement.source.value === '@primer/react' + }) + const lastSpecifier = rootImport.specifiers[rootImport.specifiers.length - 1] + fixes.push(fixer.insertTextAfter(lastSpecifier, `, Tooltip`)) + } + } else { // There are other imports from the deprecated bundle but no existing @primer/react import, so remove the Tooltip import and add a new import statement with the correct path. const previousToken = sourceCode.getTokenBefore(tooltipSpecifier) const nextToken = sourceCode.getTokenAfter(tooltipSpecifier) @@ -74,39 +84,20 @@ module.exports = { } else { rangeToRemove = [tooltipSpecifier.range[0], tooltipSpecifier.range[1]] } - + // Remove Tooltip from the import statement fixes.push(fixer.removeRange(rangeToRemove)) - fixes.push(fixer.insertTextAfter(node, `\nimport {Tooltip} from '@primer/react';`)) - } else { - if (!hasOtherImports) { - // remove the entire import statement - fixes.push(fixer.remove(node)) - } else { - const previousToken = sourceCode.getTokenBefore(tooltipSpecifier) - const nextToken = sourceCode.getTokenAfter(tooltipSpecifier) - const hasTrailingComma = nextToken && nextToken.value === ',' - const hasLeadingComma = previousToken && previousToken.value === ',' - - let rangeToRemove - if (hasTrailingComma) { - rangeToRemove = [tooltipSpecifier.range[0], nextToken.range[1] + 1] - } else if (hasLeadingComma) { - rangeToRemove = [previousToken.range[0], tooltipSpecifier.range[1]] - } else { - rangeToRemove = [tooltipSpecifier.range[0], tooltipSpecifier.range[1]] - } - - fixes.push(fixer.removeRange(rangeToRemove)) + if (!hasRootImport) { + fixes.push(fixer.insertTextAfter(node, `\nimport {Tooltip} from '@primer/react';`)) + } else { + // find the last specifier in the existing @primer/react import and insert Tooltip after that + const rootImport = sourceCode.ast.body.find(statement => { + return statement.type === 'ImportDeclaration' && statement.source.value === '@primer/react' + }) + const lastSpecifier = rootImport.specifiers[rootImport.specifiers.length - 1] + fixes.push(fixer.insertTextAfter(lastSpecifier, `, Tooltip`)) } - // find the last specifier in the existing @primer/react import and insert Tooltip after that - const rootImport = sourceCode.ast.body.find(statement => { - return statement.type === 'ImportDeclaration' && statement.source.value === '@primer/react' - }) - const lastSpecifier = rootImport.specifiers[rootImport.specifiers.length - 1] - fixes.push(fixer.insertTextAfter(lastSpecifier, `, Tooltip`)) } - return fixes }, }) From 2f0c66b1cbdbd2677356ce936f5be05439306507 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arma=C4=9Fan?= Date: Wed, 6 Nov 2024 15:44:33 +0800 Subject: [PATCH 4/7] Create gorgeous-frogs-cover.md --- .changeset/gorgeous-frogs-cover.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/gorgeous-frogs-cover.md diff --git a/.changeset/gorgeous-frogs-cover.md b/.changeset/gorgeous-frogs-cover.md new file mode 100644 index 00000000..c216aceb --- /dev/null +++ b/.changeset/gorgeous-frogs-cover.md @@ -0,0 +1,5 @@ +--- +"eslint-plugin-primer-react": patch +--- + +Update a11y-use-next-tooltip to be a11y-use-accessible-tooltip and make the changes accordingly From b663b9d08b8474f4906b5386368da238469177c0 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Thu, 7 Nov 2024 10:43:05 +0800 Subject: [PATCH 5/7] reuse rootImport and fix method --- src/rules/a11y-use-accessible-tooltip.js | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/rules/a11y-use-accessible-tooltip.js b/src/rules/a11y-use-accessible-tooltip.js index 9fb7aa71..5e0129fc 100644 --- a/src/rules/a11y-use-accessible-tooltip.js +++ b/src/rules/a11y-use-accessible-tooltip.js @@ -40,7 +40,8 @@ module.exports = { const sourceCode = context.getSourceCode() // Checking to see if there is an existing root (@primer/react) import - const rootImport = sourceCode.ast.body.filter(statement => { + // Assuming there is one root import per file + const rootImport = sourceCode.ast.body.find(statement => { return statement.type === 'ImportDeclaration' && statement.source.value === '@primer/react' }) @@ -48,7 +49,7 @@ module.exports = { specifier => specifier.imported && specifier.imported.name === 'Tooltip', ) - const hasRootImport = rootImport.length >= 1 + const hasRootImport = rootImport !== undefined context.report({ node, @@ -62,9 +63,6 @@ module.exports = { // remove the entire import statement fixes.push(fixer.remove(node)) // find the last specifier in the existing @primer/react import and insert Tooltip after that - const rootImport = sourceCode.ast.body.find(statement => { - return statement.type === 'ImportDeclaration' && statement.source.value === '@primer/react' - }) const lastSpecifier = rootImport.specifiers[rootImport.specifiers.length - 1] fixes.push(fixer.insertTextAfter(lastSpecifier, `, Tooltip`)) } @@ -91,9 +89,6 @@ module.exports = { fixes.push(fixer.insertTextAfter(node, `\nimport {Tooltip} from '@primer/react';`)) } else { // find the last specifier in the existing @primer/react import and insert Tooltip after that - const rootImport = sourceCode.ast.body.find(statement => { - return statement.type === 'ImportDeclaration' && statement.source.value === '@primer/react' - }) const lastSpecifier = rootImport.specifiers[rootImport.specifiers.length - 1] fixes.push(fixer.insertTextAfter(lastSpecifier, `, Tooltip`)) } From c8ca93dc582f119bc921908afc89d396b7c9d733 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arma=C4=9Fan?= Date: Thu, 7 Nov 2024 10:48:32 +0800 Subject: [PATCH 6/7] Update .changeset/gorgeous-frogs-cover.md --- .changeset/gorgeous-frogs-cover.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/gorgeous-frogs-cover.md b/.changeset/gorgeous-frogs-cover.md index c216aceb..adef2f07 100644 --- a/.changeset/gorgeous-frogs-cover.md +++ b/.changeset/gorgeous-frogs-cover.md @@ -1,5 +1,5 @@ --- -"eslint-plugin-primer-react": patch +"eslint-plugin-primer-react": major --- Update a11y-use-next-tooltip to be a11y-use-accessible-tooltip and make the changes accordingly From 4cb5e9790f68a5650856c237538cfe8e833c2af1 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Fri, 15 Nov 2024 10:30:08 +0800 Subject: [PATCH 7/7] improve the rule docs so that it can be used as a migration guide --- docs/rules/a11y-use-accessible-tooltip.md | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/docs/rules/a11y-use-accessible-tooltip.md b/docs/rules/a11y-use-accessible-tooltip.md index 785711d8..55311775 100644 --- a/docs/rules/a11y-use-accessible-tooltip.md +++ b/docs/rules/a11y-use-accessible-tooltip.md @@ -1,18 +1,33 @@ -# Recommends to use the `next` tooltip instead of the current stable one. +# Recommends to use the new accessible tooltip instead of the deprecated 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. +This rule suggests switching to the new accessible tooltip from @primer/react instead of the deprecated version. Deprecated props like wrap, noDelay, and align should also be removed. + +Note that the new tooltip is intended for interactive elements only, such as buttons and links, whereas the deprecated tooltip could be applied to any element, though it lacks screen reader accessibility. As a result, the autofix for this rule will only work if the deprecated tooltip is on an interactive element. If it is applied to a non-interactive element, please consult your design team for [an alternative approach](https://primer.style/guides/accessibility/tooltip-alternatives). + 👎 Examples of **incorrect** code for this rule: -```tsx +```jsx import {Tooltip} from '@primer/react/deprecated' + +const App = () => ( + + + +) ``` 👍 Examples of **correct** code for this rule: -```tsx +```jsx import {Tooltip} from '@primer/react' + +const App = () => ( + + + +) ``` ## Icon buttons and tooltips