From e55163db2e85c86ea880344f7897f00e9aac89e1 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Thu, 13 Feb 2025 08:45:03 -0500 Subject: [PATCH 1/7] Add new rule `a11y-no-title-usage` --- docs/rules/a11y-no-title-usage.md | 21 +++++++++++ .../__tests__/a11y-no-title-usage.test.js | 32 ++++++++++++++++ src/rules/a11y-no-title-usage.js | 37 +++++++++++++++++++ 3 files changed, 90 insertions(+) create mode 100644 docs/rules/a11y-no-title-usage.md create mode 100644 src/rules/__tests__/a11y-no-title-usage.test.js create mode 100644 src/rules/a11y-no-title-usage.js diff --git a/docs/rules/a11y-no-title-usage.md b/docs/rules/a11y-no-title-usage.md new file mode 100644 index 00000000..0c94c3bc --- /dev/null +++ b/docs/rules/a11y-no-title-usage.md @@ -0,0 +1,21 @@ +## Rule Details + +This rule aims to prevent the use of the `title` attribute with some components from `@primer/react`. The `title` attribute is not keyboard accessible, which results in accessibility issues. Instead, we should utilize alternatives that are accessible. + +👎 Examples of **incorrect** code for this rule + +```jsx +import {RelativeTime} from '@primer/react' + + +``` + +👍 Examples of **correct** code for this rule: + +```jsx +import {RelativeTime} from '@primer/react' + + +``` + +The `noTitle` attribute can be omitted because its default value is `true` internally. \ No newline at end of file diff --git a/src/rules/__tests__/a11y-no-title-usage.test.js b/src/rules/__tests__/a11y-no-title-usage.test.js new file mode 100644 index 00000000..c097daeb --- /dev/null +++ b/src/rules/__tests__/a11y-no-title-usage.test.js @@ -0,0 +1,32 @@ +const rule = require('../a11y-no-title-usage') +const {RuleTester} = require('eslint') + +const ruleTester = new RuleTester({ + languageOptions: { + parserOptions: { + ecmaVersion: 'latest', + sourceType: 'module', + ecmaFeatures: { + jsx: true, + }, + }, + }, +}) + +ruleTester.run('a11y-no-title-usage', rule, { + valid: [ + `import {RelativeTime} from '@primer/react'; + `, + `import {RelativeTime} from '@primer/react'; + `, + `import {RelativeTime} from '@primer/react'; + `, + ], + invalid: [ + { + code: `import {RelativeTime} from '@primer/react'; `, + output: `import {RelativeTime} from '@primer/react'; `, + errors: [{messageId: 'noTitleOnRelativeTime'}], + }, + ], +}) diff --git a/src/rules/a11y-no-title-usage.js b/src/rules/a11y-no-title-usage.js new file mode 100644 index 00000000..e2993a5e --- /dev/null +++ b/src/rules/a11y-no-title-usage.js @@ -0,0 +1,37 @@ +const {getJSXOpeningElementAttribute} = require('../utils/get-jsx-opening-element-attribute') + +module.exports = { + meta: { + type: 'error', // TODO: Confirm the type + docs: { + description: 'Disallow usage of title attribute on some components', + recommended: true, + url: null, // TODO: Add URL + }, + messages: { + noTitleOnRelativeTime: 'Avoid using the title attribute on RelativeTime.', + }, + fixable: 'code', + schema: [], + }, + + create(context) { + return { + JSXOpeningElement(jsxNode) { + const title = getJSXOpeningElementAttribute(jsxNode, 'noTitle') + + if (title && title.value && title.value.expression && title.value.expression.value !== true) { + context.report({ + node: title, + messageId: 'noTitleOnRelativeTime', + fix(fixer) { + const start = title.range[0] - 1 // Accounts for whitespace + const end = title.range[1] + return fixer.removeRange([start, end]) + }, + }) + } + }, + } + }, +} From 199637a539170adc7c83381eb43247640a742a1b Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Thu, 13 Feb 2025 09:04:57 -0500 Subject: [PATCH 2/7] Small clean up --- src/rules/a11y-no-title-usage.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/rules/a11y-no-title-usage.js b/src/rules/a11y-no-title-usage.js index e2993a5e..53ef25f8 100644 --- a/src/rules/a11y-no-title-usage.js +++ b/src/rules/a11y-no-title-usage.js @@ -1,18 +1,18 @@ +const url = require('../url') const {getJSXOpeningElementAttribute} = require('../utils/get-jsx-opening-element-attribute') module.exports = { meta: { - type: 'error', // TODO: Confirm the type + type: 'error', docs: { description: 'Disallow usage of title attribute on some components', recommended: true, - url: null, // TODO: Add URL + url: url(module), }, messages: { noTitleOnRelativeTime: 'Avoid using the title attribute on RelativeTime.', }, fixable: 'code', - schema: [], }, create(context) { @@ -25,7 +25,7 @@ module.exports = { node: title, messageId: 'noTitleOnRelativeTime', fix(fixer) { - const start = title.range[0] - 1 // Accounts for whitespace + const start = title.range[0] - 1 const end = title.range[1] return fixer.removeRange([start, end]) }, From 8c05f27bfe1b8b32ebb18a9f279abc799983aa0a Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Thu, 13 Feb 2025 09:14:17 -0500 Subject: [PATCH 3/7] Fix test, format --- docs/rules/a11y-no-title-usage.md | 6 +++--- src/rules/__tests__/a11y-no-title-usage.test.js | 12 +++++------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/docs/rules/a11y-no-title-usage.md b/docs/rules/a11y-no-title-usage.md index 0c94c3bc..d12beb63 100644 --- a/docs/rules/a11y-no-title-usage.md +++ b/docs/rules/a11y-no-title-usage.md @@ -7,7 +7,7 @@ This rule aims to prevent the use of the `title` attribute with some components ```jsx import {RelativeTime} from '@primer/react' - +const App = () => ``` 👍 Examples of **correct** code for this rule: @@ -15,7 +15,7 @@ import {RelativeTime} from '@primer/react' ```jsx import {RelativeTime} from '@primer/react' - +const App = () => ``` -The `noTitle` attribute can be omitted because its default value is `true` internally. \ No newline at end of file +The `noTitle` attribute can be omitted because its default value is `true` internally. diff --git a/src/rules/__tests__/a11y-no-title-usage.test.js b/src/rules/__tests__/a11y-no-title-usage.test.js index c097daeb..b06c2029 100644 --- a/src/rules/__tests__/a11y-no-title-usage.test.js +++ b/src/rules/__tests__/a11y-no-title-usage.test.js @@ -2,13 +2,11 @@ const rule = require('../a11y-no-title-usage') const {RuleTester} = require('eslint') const ruleTester = new RuleTester({ - languageOptions: { - parserOptions: { - ecmaVersion: 'latest', - sourceType: 'module', - ecmaFeatures: { - jsx: true, - }, + parserOptions: { + ecmaVersion: 'latest', + sourceType: 'module', + ecmaFeatures: { + jsx: true, }, }, }) From 8c38359915c1f616c43fdf5c86abaf36b4646c6b Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Thu, 13 Feb 2025 09:51:47 -0500 Subject: [PATCH 4/7] Add to index, recommended --- src/configs/recommended.js | 1 + src/index.js | 1 + src/rules/__tests__/a11y-no-title-usage.test.js | 13 +++++-------- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/configs/recommended.js b/src/configs/recommended.js index 2b247df4..e29c3c00 100644 --- a/src/configs/recommended.js +++ b/src/configs/recommended.js @@ -15,6 +15,7 @@ module.exports = { 'primer-react/a11y-tooltip-interactive-trigger': 'error', 'primer-react/new-color-css-vars': 'error', 'primer-react/a11y-explicit-heading': 'error', + 'primer-react/a11y-no-title-usage': 'error', 'primer-react/no-deprecated-props': 'warn', 'primer-react/a11y-remove-disable-tooltip': 'error', 'primer-react/a11y-use-accessible-tooltip': 'error', diff --git a/src/index.js b/src/index.js index 2acf7c88..9ba22767 100644 --- a/src/index.js +++ b/src/index.js @@ -10,6 +10,7 @@ module.exports = { 'a11y-link-in-text-block': require('./rules/a11y-link-in-text-block'), 'a11y-remove-disable-tooltip': require('./rules/a11y-remove-disable-tooltip'), 'a11y-use-accessible-tooltip': require('./rules/a11y-use-accessible-tooltip'), + 'a11y-no-title-usage': require('./rules/a11y-no-title-usage'), '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-no-title-usage.test.js b/src/rules/__tests__/a11y-no-title-usage.test.js index b06c2029..04b418bc 100644 --- a/src/rules/__tests__/a11y-no-title-usage.test.js +++ b/src/rules/__tests__/a11y-no-title-usage.test.js @@ -13,17 +13,14 @@ const ruleTester = new RuleTester({ ruleTester.run('a11y-no-title-usage', rule, { valid: [ - `import {RelativeTime} from '@primer/react'; - `, - `import {RelativeTime} from '@primer/react'; - `, - `import {RelativeTime} from '@primer/react'; - `, + ``, + ``, + ``, ], invalid: [ { - code: `import {RelativeTime} from '@primer/react'; `, - output: `import {RelativeTime} from '@primer/react'; `, + code: ``, + output: ``, errors: [{messageId: 'noTitleOnRelativeTime'}], }, ], From c982ad64e41bde38d9166914cf681c26cb14d75b Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Thu, 13 Feb 2025 10:56:21 -0500 Subject: [PATCH 5/7] Add alternative to docs --- docs/rules/a11y-no-title-usage.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/rules/a11y-no-title-usage.md b/docs/rules/a11y-no-title-usage.md index d12beb63..70602e51 100644 --- a/docs/rules/a11y-no-title-usage.md +++ b/docs/rules/a11y-no-title-usage.md @@ -19,3 +19,22 @@ const App = () => ``` The `noTitle` attribute can be omitted because its default value is `true` internally. + +## With alternative tooltip + +If you want to still utilize a tooltip in a similar way to how the `title` attribute works, you can use the [Primer `Tooltip`](https://primer.style/components/tooltip/react/beta). If you use the `Tooltip` component, you must use it with an interactive element, such as with a button or a link. + +```jsx +import {RelativeTime, Tooltip} from '@primer/react' + +const App = () => { + const date = new Date('2020-01-01T00:00:00Z') + return ( + + + + + + ) +} +``` From 295fa79b0056744edfdd8ba798c51b96a1d266aa Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Thu, 13 Feb 2025 11:00:30 -0500 Subject: [PATCH 6/7] Add changeset --- .changeset/nine-mirrors-float.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/nine-mirrors-float.md diff --git a/.changeset/nine-mirrors-float.md b/.changeset/nine-mirrors-float.md new file mode 100644 index 00000000..952d2057 --- /dev/null +++ b/.changeset/nine-mirrors-float.md @@ -0,0 +1,5 @@ +--- +'eslint-plugin-primer-react': major +--- + +Add `a11y-no-title-usage` that warns against using `title` in some components From 0c278b7f2464465650c7c1d2d73d4971f4655308 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Thu, 13 Feb 2025 12:06:06 -0500 Subject: [PATCH 7/7] Update docs/rules/a11y-no-title-usage.md --- docs/rules/a11y-no-title-usage.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/a11y-no-title-usage.md b/docs/rules/a11y-no-title-usage.md index 70602e51..0339b07c 100644 --- a/docs/rules/a11y-no-title-usage.md +++ b/docs/rules/a11y-no-title-usage.md @@ -18,7 +18,7 @@ import {RelativeTime} from '@primer/react' const App = () => ``` -The `noTitle` attribute can be omitted because its default value is `true` internally. +The noTitle attribute is true by default, so it can be omitted. ## With alternative tooltip