From 3f0eb9b6899cca088e17bb70ba270a05b39d3b1c Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Thu, 23 May 2024 14:14:04 +1000 Subject: [PATCH 1/5] add a rule to remove unsafeDisableTooltip prop --- README.md | 1 + docs/rules/a11y-remove-disable-tooltip.md | 23 +++++++++ src/configs/recommended.js | 1 + .../a11y-remove-disable-tooltip.test.js | 41 ++++++++++++++++ src/rules/a11y-remove-disable-tooltip.js | 47 +++++++++++++++++++ 5 files changed, 113 insertions(+) create mode 100644 docs/rules/a11y-remove-disable-tooltip.md create mode 100644 src/rules/__tests__/a11y-remove-disable-tooltip.test.js create mode 100644 src/rules/a11y-remove-disable-tooltip.js diff --git a/README.md b/README.md index 9163120c..b460ab95 100644 --- a/README.md +++ b/README.md @@ -37,3 +37,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) - [new-css-color-vars](https://github.com/primer/eslint-plugin-primer-react/blob/main/docs/rules/new-css-color-vars.md) - [no-deprecated-props](https://github.com/primer/eslint-plugin-primer-react/blob/main/docs/rules/no-deprecated-props.md) +- [a11y-remove-disable-tooltip](https://github.com/primer/eslint-plugin-primer-react/blob/main/docs/rules/a11y-remove-disable-tooltip.md) diff --git a/docs/rules/a11y-remove-disable-tooltip.md b/docs/rules/a11y-remove-disable-tooltip.md new file mode 100644 index 00000000..5c5ca963 --- /dev/null +++ b/docs/rules/a11y-remove-disable-tooltip.md @@ -0,0 +1,23 @@ +## 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 = () => ( + + // OR + +) +``` + +👍 Examples of **correct** code for this rule: + +```jsx +import {IconButton} from '@primer/react' + +const App = () => +``` diff --git a/src/configs/recommended.js b/src/configs/recommended.js index 25b878ca..7b4aa489 100644 --- a/src/configs/recommended.js +++ b/src/configs/recommended.js @@ -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: { diff --git a/src/rules/__tests__/a11y-remove-disable-tooltip.test.js b/src/rules/__tests__/a11y-remove-disable-tooltip.test.js new file mode 100644 index 00000000..c96e85ad --- /dev/null +++ b/src/rules/__tests__/a11y-remove-disable-tooltip.test.js @@ -0,0 +1,41 @@ +'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'; + `, + ], + invalid: [ + { + code: ``, + output: ``, + errors: [ + { + messageId: 'removeDisableTooltipProp', + }, + ], + }, + { + code: ``, + output: ``, + errors: [ + { + messageId: 'removeDisableTooltipProp', + }, + ], + }, + ], +}) diff --git a/src/rules/a11y-remove-disable-tooltip.js b/src/rules/a11y-remove-disable-tooltip.js new file mode 100644 index 00000000..071cc871 --- /dev/null +++ b/src/rules/a11y-remove-disable-tooltip.js @@ -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 making 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 + ] + }, + }) + } + }, + } + }, +} From 6c28b38867ddf0b1e858f7b634b014ceae2eb78e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arma=C4=9Fan?= Date: Thu, 23 May 2024 14:27:14 +1000 Subject: [PATCH 2/5] add changeset --- .changeset/slow-numbers-invite.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/slow-numbers-invite.md diff --git a/.changeset/slow-numbers-invite.md b/.changeset/slow-numbers-invite.md new file mode 100644 index 00000000..366499a4 --- /dev/null +++ b/.changeset/slow-numbers-invite.md @@ -0,0 +1,5 @@ +--- +"eslint-plugin-primer-react": minor +--- + +Add a rule that warns against removing `unsafeDisableTooltip` prop From bd1b0149e51c0a40e0efe879aeade8904501999f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arma=C4=9Fan?= Date: Wed, 29 May 2024 10:45:38 +1000 Subject: [PATCH 3/5] Update src/rules/a11y-remove-disable-tooltip.js Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com> --- src/rules/a11y-remove-disable-tooltip.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/a11y-remove-disable-tooltip.js b/src/rules/a11y-remove-disable-tooltip.js index 071cc871..b2896aaa 100644 --- a/src/rules/a11y-remove-disable-tooltip.js +++ b/src/rules/a11y-remove-disable-tooltip.js @@ -17,7 +17,7 @@ module.exports = { schema: [], messages: { removeDisableTooltipProp: - 'Please remove `unsafeDisableTooltip` prop from `IconButton` component to enable the tooltip and help making icon button more accessible.', + 'Please remove `unsafeDisableTooltip` prop from `IconButton` component to enable the tooltip and help make icon button more accessible.', }, }, create(context) { From 9172bd6106a08b83a5388a88454b94f188a0a379 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Fri, 31 May 2024 08:59:53 +1000 Subject: [PATCH 4/5] unsafeDisableTooltip={false} is also invalid - the prop should be removed. --- docs/rules/a11y-remove-disable-tooltip.md | 2 ++ src/rules/__tests__/a11y-remove-disable-tooltip.test.js | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/docs/rules/a11y-remove-disable-tooltip.md b/docs/rules/a11y-remove-disable-tooltip.md index 5c5ca963..8b41f449 100644 --- a/docs/rules/a11y-remove-disable-tooltip.md +++ b/docs/rules/a11y-remove-disable-tooltip.md @@ -11,6 +11,8 @@ const App = () => ( // OR + // OR + // This is incorrect because it should be removed ) ``` diff --git a/src/rules/__tests__/a11y-remove-disable-tooltip.test.js b/src/rules/__tests__/a11y-remove-disable-tooltip.test.js index c96e85ad..ebf6fb9a 100644 --- a/src/rules/__tests__/a11y-remove-disable-tooltip.test.js +++ b/src/rules/__tests__/a11y-remove-disable-tooltip.test.js @@ -37,5 +37,14 @@ ruleTester.run('a11y-remove-disable-tooltip', rule, { }, ], }, + { + code: ``, + output: ``, + errors: [ + { + messageId: 'removeDisableTooltipProp', + }, + ], + }, ], }) From 9e7a3b3add163699118596acd982cfb66a3fa554 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Fri, 31 May 2024 15:25:50 +1000 Subject: [PATCH 5/5] export the rule - duh --- src/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/index.js b/src/index.js index 2b070e60..39817402 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'), 'no-deprecated-props': require('./rules/no-deprecated-props'), + 'a11y-remove-disable-tooltip': require('./rules/a11y-remove-disable-tooltip'), }, configs: { recommended: require('./configs/recommended'),