diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 5108ff9038e..6293685d60d 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -13,8 +13,14 @@ This package contains an eslint plugin that enforces some default rules for usin `` should either be a button or a link, for a11y purposes. When given an `href` the button behaves as a link, otherwise an `onClick` handler is expected and it will behave as a button. +### `@elastic/eui/href-or-on-click` + +`` should either be a button or a link, for a11y purposes. When given an `href` the button behaves as a link, otherwise an `onClick` handler is expected and it will behave as a button. + In some cases it makes sense to disable this rule locally, such as when cmd + click should open the link in a new tab, but a standard click should use the `history.pushState()` API to change the URL without triggering a full page load. +**Exception**: `EuiLink` has to be provided with both `onClick` and `href` so that it renders as an anchor and support Ctrl/Cmd+Click to open in a new tab, and other standard link interactions. See `@elastic/eui/require-href-for-link`. + ### `@elastic/eui/no-restricted-eui-imports` At times, we deprecate features that may need more highlighting and/or that are not possible to annotate with JSDoc `@deprecated`, e.g. JSON token imports: `@elastic/eui/dist/eui_theme_*.json` (for context: https://github.com/elastic/kibana/issues/199715#json-tokens). @@ -175,6 +181,10 @@ Ensure the `EuiBadge` includes appropriate accessibility attributes. - `iconOnClickAriaLabel` is only valid when `iconOnClick` is present. The rule autofixes by removing `iconOnClickAriaLabel`. - `onClickAriaLabel` is only valid when `onClick` is present. The rule autofixes by removing `onClickAriaLabel`. +### `@elastic/eui/require-href-for-link` + +Ensure `EuiLink` components that have an `onClick` handler also include an `href` prop. Without `href`, the component does not render as a true link, which means users cannot Ctrl/Cmd+Click to open in a new tab or use other standard link interactions. The rule bails out when spread attributes are present, since `href` may be provided via the spread. + ### `@elastic/eui/icon-accessibility-rules` Ensure the `EuiIcon` includes appropriate accessibility attributes. diff --git a/packages/eslint-plugin/changelogs/upcoming/9615.md b/packages/eslint-plugin/changelogs/upcoming/9615.md new file mode 100644 index 00000000000..14e97bf6e3b --- /dev/null +++ b/packages/eslint-plugin/changelogs/upcoming/9615.md @@ -0,0 +1 @@ +- Added new `require-href-for-link` rule diff --git a/packages/eslint-plugin/src/index.ts b/packages/eslint-plugin/src/index.ts index 0e20ad94e40..4bc91ee2670 100644 --- a/packages/eslint-plugin/src/index.ts +++ b/packages/eslint-plugin/src/index.ts @@ -10,6 +10,7 @@ import { AccessibleInteractiveElements } from './rules/a11y/accessible_interacti import { CallOutAnnounceOnMount } from './rules/a11y/callout_announce_on_mount'; import { ConsistentIsInvalidProps } from './rules/a11y/consistent_is_invalid_props'; import { HrefOnClick } from './rules/href_or_on_click'; +import { RequireHrefForLink } from './rules/require_href_for_link'; import { NoCssColor } from './rules/no_css_color'; import { NoRestrictedEuiImports } from './rules/no_restricted_eui_imports'; import { NoStaticZIndex } from './rules/no_static_z_index'; @@ -44,6 +45,7 @@ const config = { PreferTooltipTriggerFocusTestUtility, 'badge-accessibility-rules': EuiBadgeAccessibilityRules, 'icon-accessibility-rules': EuiIconAccessibilityRules, + 'require-href-for-link': RequireHrefForLink, }, configs: { recommended: { @@ -66,6 +68,7 @@ const config = { '@elastic/eui/prefer-tooltip-trigger-focus-test-utility': 'warn', '@elastic/eui/badge-accessibility-rules': 'warn', '@elastic/eui/icon-accessibility-rules': 'warn', + '@elastic/eui/require-href-for-link': 'warn', }, }, }, diff --git a/packages/eslint-plugin/src/rules/href_or_on_click.test.ts b/packages/eslint-plugin/src/rules/href_or_on_click.test.ts index a24700b091c..34c348ac27b 100644 --- a/packages/eslint-plugin/src/rules/href_or_on_click.test.ts +++ b/packages/eslint-plugin/src/rules/href_or_on_click.test.ts @@ -96,18 +96,5 @@ ruleTester.run('href-or-on-click', HrefOnClick, { }, ], }, - { - code: dedent` - module.export = () => ( - - ) - `, - languageOptions, - errors: [ - { - messageId: 'hrefOrOnClick', - }, - ], - }, ], }); diff --git a/packages/eslint-plugin/src/rules/href_or_on_click.ts b/packages/eslint-plugin/src/rules/href_or_on_click.ts index 0fb76c1ae74..a39ca14f02b 100644 --- a/packages/eslint-plugin/src/rules/href_or_on_click.ts +++ b/packages/eslint-plugin/src/rules/href_or_on_click.ts @@ -8,7 +8,7 @@ import { TSESTree, ESLintUtils } from '@typescript-eslint/utils'; -const componentNames = ['EuiButton', 'EuiButtonEmpty', 'EuiLink', 'EuiBadge']; +const componentNames = ['EuiButton', 'EuiButtonEmpty', 'EuiBadge']; export const HrefOnClick = ESLintUtils.RuleCreator.withoutDocs({ create(context) { diff --git a/packages/eslint-plugin/src/rules/require_href_for_link.test.ts b/packages/eslint-plugin/src/rules/require_href_for_link.test.ts new file mode 100644 index 00000000000..2fb6b8ad342 --- /dev/null +++ b/packages/eslint-plugin/src/rules/require_href_for_link.test.ts @@ -0,0 +1,83 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import dedent from 'dedent'; +import { RuleTester } from '@typescript-eslint/rule-tester'; + +import { RequireHrefForLink } from './require_href_for_link'; + +const languageOptions = { + parserOptions: { + ecmaFeatures: { + jsx: true, + }, + }, +}; + +const ruleTester = new RuleTester(); + +ruleTester.run('require-href-for-link', RequireHrefForLink, { + valid: [ + { + code: dedent` + module.export = () => ( + + ) + `, + languageOptions, + }, + { + code: dedent` + module.export = () => ( + + ) + `, + languageOptions, + }, + { + code: dedent` + module.export = () => ( + + ) + `, + languageOptions, + }, + { + code: dedent` + module.export = () => ( + + ) + `, + languageOptions, + }, + { + code: dedent` + module.export = () => ( + + ) + `, + languageOptions, + }, + ], + + invalid: [ + { + code: dedent` + module.export = () => ( + + ) + `, + languageOptions, + errors: [ + { + messageId: 'requireHrefForLink', + }, + ], + }, + ], +}); diff --git a/packages/eslint-plugin/src/rules/require_href_for_link.ts b/packages/eslint-plugin/src/rules/require_href_for_link.ts new file mode 100644 index 00000000000..abaa5570a9b --- /dev/null +++ b/packages/eslint-plugin/src/rules/require_href_for_link.ts @@ -0,0 +1,64 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { TSESTree, ESLintUtils } from '@typescript-eslint/utils'; + +import { hasSpread } from '../utils/has_spread'; + +export const RequireHrefForLink = ESLintUtils.RuleCreator.withoutDocs({ + create(context) { + return { + JSXOpeningElement(node: TSESTree.JSXOpeningElement): void { + if ( + node.name.type !== 'JSXIdentifier' || + node.name.name !== 'EuiLink' + ) { + return; + } + + // Bail out if props are spread — we can't statically determine + // whether `href` is provided via the spread + if (hasSpread(node.attributes)) { + return; + } + + // Check if the node has `href` and `onClick` attributes + const hasHref = node.attributes.some( + (attr) => attr.type === 'JSXAttribute' && attr.name.name === 'href' + ); + const hasOnClick = node.attributes.some( + (attr) => attr.type === 'JSXAttribute' && attr.name.name === 'onClick' + ); + + // Report an issue if `onClick` is present without `href` + if (hasOnClick && !hasHref) { + context.report({ + node, + messageId: 'requireHrefForLink', + data: { + name: node.name.name, + }, + }); + } + }, + }; + }, + meta: { + type: 'suggestion', + docs: { + description: + 'Recommend including `href` alongside `onClick` on EuiLink so that Ctrl/Cmd+Click (open link in new tab) works.', + }, + schema: [], + messages: { + requireHrefForLink: + '<{{name}}> has `onClick` but no `href`. Consider adding `href` so that the component renders as a link and supports Ctrl/Cmd+Click / "Open link in new tab".', + }, + }, + defaultOptions: [], +});