From ded3726ef2d26652ba51553142106bce8fa5422e Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Fri, 20 Oct 2023 11:24:03 +1000 Subject: [PATCH 01/14] Add use-next-tooltip rule --- docs/rules/use-next-tooltip.md | 16 +++++++++ src/rules/__tests__/use-next-tooltip.test.js | 32 +++++++++++++++++ src/rules/use-next-tooltip.js | 36 ++++++++++++++++++++ 3 files changed, 84 insertions(+) create mode 100644 docs/rules/use-next-tooltip.md create mode 100644 src/rules/__tests__/use-next-tooltip.test.js create mode 100644 src/rules/use-next-tooltip.js diff --git a/docs/rules/use-next-tooltip.md b/docs/rules/use-next-tooltip.md new file mode 100644 index 00000000..5e4907bf --- /dev/null +++ b/docs/rules/use-next-tooltip.md @@ -0,0 +1,16 @@ +# 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' +``` diff --git a/src/rules/__tests__/use-next-tooltip.test.js b/src/rules/__tests__/use-next-tooltip.test.js new file mode 100644 index 00000000..09edc5c6 --- /dev/null +++ b/src/rules/__tests__/use-next-tooltip.test.js @@ -0,0 +1,32 @@ +const rule = require('../use-next-tooltip') +const {RuleTester} = require('eslint') + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 'latest', + sourceType: 'module', + ecmaFeatures: { + jsx: true + } + } +}) + +ruleTester.run('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'}] + }, + { + code: `import {Tooltip, Button} from '@primer/react'`, + errors: [{messageId: 'useNextTooltip'}] + } + ] +}) diff --git a/src/rules/use-next-tooltip.js b/src/rules/use-next-tooltip.js new file mode 100644 index 00000000..02daf9c0 --- /dev/null +++ b/src/rules/use-next-tooltip.js @@ -0,0 +1,36 @@ +'use strict' + +module.exports = { + meta: { + type: 'suggestion', + docs: { + description: 'recommends the use of @primer/react/next Tooltip component', + category: 'Best Practices', + recommended: true + }, + fixable: null, + schema: [], + messages: { + useNextTooltip: 'Please use @primer/react/next Tooltip component that has accessibility improvements' + } + }, + create(context) { + return { + ImportDeclaration(node) { + if (node.source.value !== '@primer/react') { + return + } + const hasTooltip = node.specifiers.some( + specifier => specifier.imported && specifier.imported.name === 'Tooltip' + ) + if (!hasTooltip) { + return + } + context.report({ + node, + messageId: 'useNextTooltip' + }) + } + } + } +} From 7d7c94b08212f8ba989585409998f2bb549e8407 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Fri, 20 Oct 2023 11:33:42 +1000 Subject: [PATCH 02/14] format --- src/rules/__tests__/use-next-tooltip.test.js | 16 ++++++++-------- src/rules/use-next-tooltip.js | 14 +++++++------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/rules/__tests__/use-next-tooltip.test.js b/src/rules/__tests__/use-next-tooltip.test.js index 09edc5c6..c4027f0c 100644 --- a/src/rules/__tests__/use-next-tooltip.test.js +++ b/src/rules/__tests__/use-next-tooltip.test.js @@ -6,9 +6,9 @@ const ruleTester = new RuleTester({ ecmaVersion: 'latest', sourceType: 'module', ecmaFeatures: { - jsx: true - } - } + jsx: true, + }, + }, }) ruleTester.run('use-next-tooltip', rule, { @@ -17,16 +17,16 @@ ruleTester.run('use-next-tooltip', rule, { `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';` + import {Tooltip, SelectPanel} from '@primer/react/next';`, ], invalid: [ { code: `import {Tooltip} from '@primer/react'`, - errors: [{messageId: 'useNextTooltip'}] + errors: [{messageId: 'useNextTooltip'}], }, { code: `import {Tooltip, Button} from '@primer/react'`, - errors: [{messageId: 'useNextTooltip'}] - } - ] + errors: [{messageId: 'useNextTooltip'}], + }, + ], }) diff --git a/src/rules/use-next-tooltip.js b/src/rules/use-next-tooltip.js index 02daf9c0..5b0cbd27 100644 --- a/src/rules/use-next-tooltip.js +++ b/src/rules/use-next-tooltip.js @@ -6,13 +6,13 @@ module.exports = { docs: { description: 'recommends the use of @primer/react/next Tooltip component', category: 'Best Practices', - recommended: true + recommended: true, }, fixable: null, schema: [], messages: { - useNextTooltip: 'Please use @primer/react/next Tooltip component that has accessibility improvements' - } + useNextTooltip: 'Please use @primer/react/next Tooltip component that has accessibility improvements', + }, }, create(context) { return { @@ -21,16 +21,16 @@ module.exports = { return } const hasTooltip = node.specifiers.some( - specifier => specifier.imported && specifier.imported.name === 'Tooltip' + specifier => specifier.imported && specifier.imported.name === 'Tooltip', ) if (!hasTooltip) { return } context.report({ node, - messageId: 'useNextTooltip' + messageId: 'useNextTooltip', }) - } + }, } - } + }, } From 27c7e24fdcdefbdb4570b2cc6078200b849c15e2 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Thu, 9 Nov 2023 17:35:58 +1000 Subject: [PATCH 03/14] add a fix to replace the path --- src/rules/__tests__/use-next-tooltip.test.js | 13 +++++++--- src/rules/use-next-tooltip.js | 26 +++++++++++++++++++- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/rules/__tests__/use-next-tooltip.test.js b/src/rules/__tests__/use-next-tooltip.test.js index c4027f0c..019d6b0d 100644 --- a/src/rules/__tests__/use-next-tooltip.test.js +++ b/src/rules/__tests__/use-next-tooltip.test.js @@ -13,7 +13,7 @@ const ruleTester = new RuleTester({ ruleTester.run('use-next-tooltip', rule, { valid: [ - `import {Tooltip} from '@primer/react/next'`, + `import {Tooltip} from '@primer/react/next';`, `import {UnderlineNav, Button} from '@primer/react'; import {Tooltip} from '@primer/react/next';`, `import {UnderlineNav, Button} from '@primer/react'; @@ -21,12 +21,19 @@ ruleTester.run('use-next-tooltip', rule, { ], invalid: [ { - code: `import {Tooltip} from '@primer/react'`, + code: `import {Tooltip} from '@primer/react';`, errors: [{messageId: 'useNextTooltip'}], + output: `import {Tooltip} from '@primer/react/next';`, }, { - code: `import {Tooltip, Button} from '@primer/react'`, + code: `import {Tooltip, Button} from '@primer/react';`, errors: [{messageId: 'useNextTooltip'}], + output: `import {Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/next';`, + }, + { + code: `import {ActionList, ActionMenu, Tooltip, Button} from '@primer/react';`, + errors: [{messageId: 'useNextTooltip'}], + output: `import {ActionList, ActionMenu, Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/next';`, }, ], }) diff --git a/src/rules/use-next-tooltip.js b/src/rules/use-next-tooltip.js index 5b0cbd27..b9defbdf 100644 --- a/src/rules/use-next-tooltip.js +++ b/src/rules/use-next-tooltip.js @@ -8,7 +8,7 @@ module.exports = { category: 'Best Practices', recommended: true, }, - fixable: null, + fixable: true, schema: [], messages: { useNextTooltip: 'Please use @primer/react/next Tooltip component that has accessibility improvements', @@ -23,12 +23,36 @@ module.exports = { const hasTooltip = node.specifiers.some( specifier => specifier.imported && specifier.imported.name === 'Tooltip', ) + + const hasOtherImports = node.specifiers.some( + specifier => specifier.imported && specifier.imported.name !== 'Tooltip', + ) 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', + ) + return [ + // remove tooltip specifier and the space and comma after it + fixer.removeRange([tooltipSpecifier.range[0], tooltipSpecifier.range[1] + 2]), + // fixer.remove(tooltipSpecifier), + fixer.insertTextAfterRange( + [node.range[1], node.range[1]], + `\nimport {Tooltip} from '@primer/react/next';`, + ), + ] + } + }, }) }, } From 613795b63acf8a6b5f8ff23e200fab39bb342598 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 6 Dec 2023 13:32:45 +1000 Subject: [PATCH 04/14] add a fix to replace aria-label with text --- src/rules/__tests__/use-next-tooltip.test.js | 15 +++++++++------ src/rules/use-next-tooltip.js | 13 +++++++++++++ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/rules/__tests__/use-next-tooltip.test.js b/src/rules/__tests__/use-next-tooltip.test.js index 019d6b0d..7a77c6a6 100644 --- a/src/rules/__tests__/use-next-tooltip.test.js +++ b/src/rules/__tests__/use-next-tooltip.test.js @@ -26,14 +26,17 @@ ruleTester.run('use-next-tooltip', rule, { output: `import {Tooltip} from '@primer/react/next';`, }, { - code: `import {Tooltip, Button} from '@primer/react';`, - errors: [{messageId: 'useNextTooltip'}], - output: `import {Button} from '@primer/react';\nimport {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';`, - errors: [{messageId: 'useNextTooltip'}], - output: `import {ActionList, ActionMenu, Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/next';`, + 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`, }, ], }) diff --git a/src/rules/use-next-tooltip.js b/src/rules/use-next-tooltip.js index b9defbdf..2b7534eb 100644 --- a/src/rules/use-next-tooltip.js +++ b/src/rules/use-next-tooltip.js @@ -1,4 +1,5 @@ 'use strict' +const {getJSXOpeningElementAttribute} = require('../utils/get-jsx-opening-element-attribute') module.exports = { meta: { @@ -12,6 +13,7 @@ module.exports = { schema: [], messages: { useNextTooltip: 'Please use @primer/react/next Tooltip component that has accessibility improvements', + useTextProp: 'Please use the text prop instead of aria-label', }, }, create(context) { @@ -55,6 +57,17 @@ module.exports = { }, }) }, + JSXAttribute(node) { + if (node.name.name === 'aria-label') { + context.report({ + node, + messageId: 'useTextProp', + fix(fixer) { + return fixer.replaceText(node.name, 'text') + }, + }) + } + }, } }, } From 924cdc4f56c7b400210e3a2a6666bbe61bcfc1d9 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 6 Dec 2023 13:34:44 +1000 Subject: [PATCH 05/14] fix linting --- src/rules/use-next-tooltip.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/rules/use-next-tooltip.js b/src/rules/use-next-tooltip.js index 2b7534eb..2258d120 100644 --- a/src/rules/use-next-tooltip.js +++ b/src/rules/use-next-tooltip.js @@ -1,5 +1,4 @@ 'use strict' -const {getJSXOpeningElementAttribute} = require('../utils/get-jsx-opening-element-attribute') module.exports = { meta: { From b584b8a65b5041ef964e958c0a38358c7c189bc0 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Thu, 7 Dec 2023 07:15:59 +1000 Subject: [PATCH 06/14] update the entry point and add additional fixes --- src/rules/__tests__/use-next-tooltip.test.js | 23 ++++++++---- src/rules/use-next-tooltip.js | 38 +++++++++++++++++--- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/src/rules/__tests__/use-next-tooltip.test.js b/src/rules/__tests__/use-next-tooltip.test.js index 7a77c6a6..aad14312 100644 --- a/src/rules/__tests__/use-next-tooltip.test.js +++ b/src/rules/__tests__/use-next-tooltip.test.js @@ -13,22 +13,22 @@ const ruleTester = new RuleTester({ ruleTester.run('use-next-tooltip', rule, { valid: [ - `import {Tooltip} from '@primer/react/next';`, + `import {Tooltip} from '@primer/react/experimental';`, `import {UnderlineNav, Button} from '@primer/react'; - import {Tooltip} from '@primer/react/next';`, + import {Tooltip} from '@primer/react/experimental';`, `import {UnderlineNav, Button} from '@primer/react'; - import {Tooltip, SelectPanel} from '@primer/react/next';`, + import {Tooltip, SelectPanel} from '@primer/react/experimental';`, ], invalid: [ { code: `import {Tooltip} from '@primer/react';`, errors: [{messageId: 'useNextTooltip'}], - output: `import {Tooltip} from '@primer/react/next';`, + output: `import {Tooltip} from '@primer/react/experimental';`, }, { 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`, + output: `import {Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/experimental';\n`, }, { code: `import {ActionList, ActionMenu, Tooltip, Button} from '@primer/react';\n`, @@ -36,7 +36,18 @@ ruleTester.run('use-next-tooltip', rule, { {messageId: 'useNextTooltip', line: 1}, {messageId: 'useTextProp', line: 2}, ], - output: `import {ActionList, ActionMenu, Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/next';\n`, + output: `import {ActionList, ActionMenu, Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/experimental';\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/experimental';\n`, }, ], }) diff --git a/src/rules/use-next-tooltip.js b/src/rules/use-next-tooltip.js index 2258d120..bb332fe0 100644 --- a/src/rules/use-next-tooltip.js +++ b/src/rules/use-next-tooltip.js @@ -4,15 +4,18 @@ module.exports = { meta: { type: 'suggestion', docs: { - description: 'recommends the use of @primer/react/next Tooltip component', + description: 'recommends the use of @primer/react/experimental Tooltip component', category: 'Best Practices', recommended: true, }, fixable: true, schema: [], messages: { - useNextTooltip: 'Please use @primer/react/next Tooltip component that has accessibility improvements', + useNextTooltip: 'Please use @primer/react/experimental 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) { @@ -37,7 +40,7 @@ module.exports = { fix(fixer) { // If Tooltip is the only import, replace the whole import statement if (!hasOtherImports) { - return fixer.replaceText(node.source, `'@primer/react/next'`) + return fixer.replaceText(node.source, `'@primer/react/experimental'`) } else { // Otherwise, remove Tooltip from the import statement and add a new import statement with the correct path const tooltipSpecifier = node.specifiers.find( @@ -49,7 +52,7 @@ module.exports = { // fixer.remove(tooltipSpecifier), fixer.insertTextAfterRange( [node.range[1], node.range[1]], - `\nimport {Tooltip} from '@primer/react/next';`, + `\nimport {Tooltip} from '@primer/react/experimental';`, ), ] } @@ -66,6 +69,33 @@ module.exports = { }, }) } + if (node.name.name === 'noDelay') { + context.report({ + node, + messageId: 'noDelayRemoved', + fix(fixer) { + return fixer.remove(node) + }, + }) + } + if (node.name.name === 'wrap') { + context.report({ + node, + messageId: 'wrapRemoved', + fix(fixer) { + return fixer.remove(node) + }, + }) + } + if (node.name.name === 'align') { + context.report({ + node, + messageId: 'alignRemoved', + fix(fixer) { + return fixer.remove(node) + }, + }) + } }, } }, From ec3ca306bbd8c923f5b83468e97a45dc4fa15458 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Thu, 7 Dec 2023 13:03:38 +1000 Subject: [PATCH 07/14] fix attribute checker to only look for in Tooltip --- src/configs/recommended.js | 1 + src/index.js | 1 + src/rules/__tests__/use-next-tooltip.test.js | 10 +++- src/rules/use-next-tooltip.js | 52 ++++++++++++++------ 4 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/configs/recommended.js b/src/configs/recommended.js index 793932b1..e2e30b88 100644 --- a/src/configs/recommended.js +++ b/src/configs/recommended.js @@ -17,6 +17,7 @@ module.exports = { 'primer-react/new-color-css-vars': 'error', 'primer-react/a11y-explicit-heading': 'error', 'primer-react/new-color-css-vars-have-fallback': 'error', + 'primer-react/use-next-tooltip': 'error', }, settings: { github: { diff --git a/src/index.js b/src/index.js index 9df33fd5..0a82552b 100644 --- a/src/index.js +++ b/src/index.js @@ -7,6 +7,7 @@ module.exports = { 'new-color-css-vars': require('./rules/new-color-css-vars'), 'a11y-explicit-heading': require('./rules/a11y-explicit-heading'), 'new-color-css-vars-have-fallback': require('./rules/new-color-css-vars-have-fallback'), + 'use-next-tooltip': require('./rules/use-next-tooltip'), }, configs: { recommended: require('./configs/recommended'), diff --git a/src/rules/__tests__/use-next-tooltip.test.js b/src/rules/__tests__/use-next-tooltip.test.js index aad14312..c09c8dc7 100644 --- a/src/rules/__tests__/use-next-tooltip.test.js +++ b/src/rules/__tests__/use-next-tooltip.test.js @@ -38,6 +38,14 @@ ruleTester.run('use-next-tooltip', rule, { ], output: `import {ActionList, ActionMenu, Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/experimental';\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/experimental';\n`, + }, { code: `import {ActionList, ActionMenu, Tooltip, Button} from '@primer/react';\n`, errors: [ @@ -47,7 +55,7 @@ ruleTester.run('use-next-tooltip', rule, { {messageId: 'wrapRemoved', line: 2}, {messageId: 'alignRemoved', line: 2}, ], - output: `import {ActionList, ActionMenu, Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/experimental';\n`, + output: `import {ActionList, ActionMenu, Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/experimental';\n`, }, ], }) diff --git a/src/rules/use-next-tooltip.js b/src/rules/use-next-tooltip.js index bb332fe0..e279a5ec 100644 --- a/src/rules/use-next-tooltip.js +++ b/src/rules/use-next-tooltip.js @@ -1,4 +1,6 @@ 'use strict' +const {getJSXOpeningElementAttribute} = require('../utils/get-jsx-opening-element-attribute') +const {getJSXOpeningElementName} = require('../utils/get-jsx-opening-element-name') module.exports = { meta: { @@ -28,9 +30,7 @@ module.exports = { specifier => specifier.imported && specifier.imported.name === 'Tooltip', ) - const hasOtherImports = node.specifiers.some( - specifier => specifier.imported && specifier.imported.name !== 'Tooltip', - ) + const hasOtherImports = node.specifiers.length > 1 if (!hasTooltip) { return } @@ -46,10 +46,26 @@ module.exports = { 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([tooltipSpecifier.range[0], tooltipSpecifier.range[1] + 2]), - // fixer.remove(tooltipSpecifier), + fixer.removeRange([start, end]), fixer.insertTextAfterRange( [node.range[1], node.range[1]], `\nimport {Tooltip} from '@primer/react/experimental';`, @@ -59,40 +75,48 @@ module.exports = { }, }) }, - JSXAttribute(node) { - if (node.name.name === 'aria-label') { + 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(node.name, 'text') + return fixer.replaceText(ariaLabel.name, 'text') }, }) } - if (node.name.name === 'noDelay') { + const noDelay = getJSXOpeningElementAttribute(node, 'noDelay') + if (noDelay !== undefined) { context.report({ node, messageId: 'noDelayRemoved', fix(fixer) { - return fixer.remove(node) + return fixer.remove(noDelay) }, }) } - if (node.name.name === 'wrap') { + const wrap = getJSXOpeningElementAttribute(node, 'wrap') + if (wrap !== undefined) { context.report({ node, messageId: 'wrapRemoved', fix(fixer) { - return fixer.remove(node) + return fixer.remove(wrap) }, }) } - if (node.name.name === 'align') { + const align = getJSXOpeningElementAttribute(node, 'align') + if (align !== undefined) { context.report({ node, messageId: 'alignRemoved', fix(fixer) { - return fixer.remove(node) + return fixer.remove(align) }, }) } From aaf53d27dfbb7e1359375089250b1df45f71ccd0 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Fri, 5 Jul 2024 11:05:51 +1000 Subject: [PATCH 08/14] rename the rule to a11y-use-next-tooltip --- docs/rules/{use-next-tooltip.md => a11y-use-next-tooltip.md} | 0 src/configs/recommended.js | 2 +- src/index.js | 2 +- ...use-next-tooltip.test.js => a11y-use-next-tooltip.test.js} | 4 ++-- src/rules/{use-next-tooltip.js => a11y-use-next-tooltip.js} | 0 5 files changed, 4 insertions(+), 4 deletions(-) rename docs/rules/{use-next-tooltip.md => a11y-use-next-tooltip.md} (100%) rename src/rules/__tests__/{use-next-tooltip.test.js => a11y-use-next-tooltip.test.js} (96%) rename src/rules/{use-next-tooltip.js => a11y-use-next-tooltip.js} (100%) diff --git a/docs/rules/use-next-tooltip.md b/docs/rules/a11y-use-next-tooltip.md similarity index 100% rename from docs/rules/use-next-tooltip.md rename to docs/rules/a11y-use-next-tooltip.md diff --git a/src/configs/recommended.js b/src/configs/recommended.js index b303e11a..916be1fa 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/use-next-tooltip': 'error', + 'primer-react/a11y-use-next-tooltip': 'error', }, settings: { github: { diff --git a/src/index.js b/src/index.js index 82289221..20e1fb01 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'), - 'use-next-tooltip': require('./rules/use-next-tooltip'), + 'a11y-use-next-tooltip': require('./rules/a11y-use-next-tooltip'), }, configs: { recommended: require('./configs/recommended'), diff --git a/src/rules/__tests__/use-next-tooltip.test.js b/src/rules/__tests__/a11y-use-next-tooltip.test.js similarity index 96% rename from src/rules/__tests__/use-next-tooltip.test.js rename to src/rules/__tests__/a11y-use-next-tooltip.test.js index c09c8dc7..cf5d96ac 100644 --- a/src/rules/__tests__/use-next-tooltip.test.js +++ b/src/rules/__tests__/a11y-use-next-tooltip.test.js @@ -1,4 +1,4 @@ -const rule = require('../use-next-tooltip') +const rule = require('../a11y-use-next-tooltip') const {RuleTester} = require('eslint') const ruleTester = new RuleTester({ @@ -11,7 +11,7 @@ const ruleTester = new RuleTester({ }, }) -ruleTester.run('use-next-tooltip', rule, { +ruleTester.run('a11y-use-next-tooltip', rule, { valid: [ `import {Tooltip} from '@primer/react/experimental';`, `import {UnderlineNav, Button} from '@primer/react'; diff --git a/src/rules/use-next-tooltip.js b/src/rules/a11y-use-next-tooltip.js similarity index 100% rename from src/rules/use-next-tooltip.js rename to src/rules/a11y-use-next-tooltip.js From 773698b10075e9ad8465440ed8ea68a893c481ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arma=C4=9Fan?= Date: Fri, 5 Jul 2024 11:09:08 +1000 Subject: [PATCH 09/14] Create silly-phones-jog.md --- .changeset/silly-phones-jog.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/silly-phones-jog.md 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) From d651470c42ffd142dded79321451c49e1c36a867 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 17 Jul 2024 15:43:28 +1000 Subject: [PATCH 10/14] experimental->next --- .../__tests__/a11y-use-next-tooltip.test.js | 16 ++++++++-------- src/rules/a11y-use-next-tooltip.js | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/rules/__tests__/a11y-use-next-tooltip.test.js b/src/rules/__tests__/a11y-use-next-tooltip.test.js index cf5d96ac..64115c7c 100644 --- a/src/rules/__tests__/a11y-use-next-tooltip.test.js +++ b/src/rules/__tests__/a11y-use-next-tooltip.test.js @@ -13,22 +13,22 @@ const ruleTester = new RuleTester({ ruleTester.run('a11y-use-next-tooltip', rule, { valid: [ - `import {Tooltip} from '@primer/react/experimental';`, + `import {Tooltip} from '@primer/react/next';`, `import {UnderlineNav, Button} from '@primer/react'; - import {Tooltip} from '@primer/react/experimental';`, + import {Tooltip} from '@primer/react/next';`, `import {UnderlineNav, Button} from '@primer/react'; - import {Tooltip, SelectPanel} from '@primer/react/experimental';`, + import {Tooltip, SelectPanel} from '@primer/react/next';`, ], invalid: [ { code: `import {Tooltip} from '@primer/react';`, errors: [{messageId: 'useNextTooltip'}], - output: `import {Tooltip} from '@primer/react/experimental';`, + 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/experimental';\n`, + output: `import {Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/next';\n`, }, { code: `import {ActionList, ActionMenu, Tooltip, Button} from '@primer/react';\n`, @@ -36,7 +36,7 @@ ruleTester.run('a11y-use-next-tooltip', rule, { {messageId: 'useNextTooltip', line: 1}, {messageId: 'useTextProp', line: 2}, ], - output: `import {ActionList, ActionMenu, Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/experimental';\n`, + 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`, @@ -44,7 +44,7 @@ ruleTester.run('a11y-use-next-tooltip', rule, { {messageId: 'useNextTooltip', line: 1}, {messageId: 'useTextProp', line: 2}, ], - output: `import {ActionList, ActionMenu, Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/experimental';\n`, + 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`, @@ -55,7 +55,7 @@ ruleTester.run('a11y-use-next-tooltip', rule, { {messageId: 'wrapRemoved', line: 2}, {messageId: 'alignRemoved', line: 2}, ], - output: `import {ActionList, ActionMenu, Button} from '@primer/react';\nimport {Tooltip} from '@primer/react/experimental';\n`, + 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 index e279a5ec..cba8ab80 100644 --- a/src/rules/a11y-use-next-tooltip.js +++ b/src/rules/a11y-use-next-tooltip.js @@ -6,14 +6,14 @@ module.exports = { meta: { type: 'suggestion', docs: { - description: 'recommends the use of @primer/react/experimental Tooltip component', + description: 'recommends the use of @primer/react/next Tooltip component', category: 'Best Practices', recommended: true, }, fixable: true, schema: [], messages: { - useNextTooltip: 'Please use @primer/react/experimental Tooltip component that has accessibility improvements', + 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', @@ -40,7 +40,7 @@ module.exports = { fix(fixer) { // If Tooltip is the only import, replace the whole import statement if (!hasOtherImports) { - return fixer.replaceText(node.source, `'@primer/react/experimental'`) + 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( @@ -68,7 +68,7 @@ module.exports = { fixer.removeRange([start, end]), fixer.insertTextAfterRange( [node.range[1], node.range[1]], - `\nimport {Tooltip} from '@primer/react/experimental';`, + `\nimport {Tooltip} from '@primer/react/next';`, ), ] } From 0b3f2f7dfd032b02cd712757260e82cb9e52fcac Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Thu, 18 Jul 2024 14:05:44 +1000 Subject: [PATCH 11/14] Add docs link --- src/rules/a11y-use-next-tooltip.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/rules/a11y-use-next-tooltip.js b/src/rules/a11y-use-next-tooltip.js index cba8ab80..ab1a6e3c 100644 --- a/src/rules/a11y-use-next-tooltip.js +++ b/src/rules/a11y-use-next-tooltip.js @@ -1,4 +1,5 @@ 'use strict' +const url = require('../url') const {getJSXOpeningElementAttribute} = require('../utils/get-jsx-opening-element-attribute') const {getJSXOpeningElementName} = require('../utils/get-jsx-opening-element-name') @@ -9,6 +10,7 @@ module.exports = { description: 'recommends the use of @primer/react/next Tooltip component', category: 'Best Practices', recommended: true, + url: url(module), }, fixable: true, schema: [], From 1d187b91ea94489d177ad468ed3bd393d9934301 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Thu, 18 Jul 2024 14:14:58 +1000 Subject: [PATCH 12/14] docs --- docs/rules/a11y-use-next-tooltip.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/rules/a11y-use-next-tooltip.md b/docs/rules/a11y-use-next-tooltip.md index 5e4907bf..bfbbcf5a 100644 --- a/docs/rules/a11y-use-next-tooltip.md +++ b/docs/rules/a11y-use-next-tooltip.md @@ -14,3 +14,15 @@ import {Tooltip} from '@primer/react' ```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' +; + + +``` From 5002f332b6f15aee0c36e6705a02c0a149745b58 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Thu, 18 Jul 2024 14:26:18 +1000 Subject: [PATCH 13/14] docs --- README.md | 1 + docs/rules/a11y-use-next-tooltip.md | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) 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 index bfbbcf5a..d364d5d2 100644 --- a/docs/rules/a11y-use-next-tooltip.md +++ b/docs/rules/a11y-use-next-tooltip.md @@ -19,10 +19,10 @@ 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 +```tsx import {IconButton} from '@primer/react' import {Tooltip} from '@primer/react/next' -; - + + ``` From 1bfbc8ab8168b65a6182b53bb06c5d3e76495ced Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Thu, 18 Jul 2024 14:28:44 +1000 Subject: [PATCH 14/14] linting --- docs/rules/a11y-use-next-tooltip.md | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/rules/a11y-use-next-tooltip.md b/docs/rules/a11y-use-next-tooltip.md index d364d5d2..18659934 100644 --- a/docs/rules/a11y-use-next-tooltip.md +++ b/docs/rules/a11y-use-next-tooltip.md @@ -19,10 +19,15 @@ 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. -```tsx +```jsx import {IconButton} from '@primer/react' import {Tooltip} from '@primer/react/next' - - - + +function ExampleComponent() { + return ( + + + + ) +} ```