From fd8d75f09150c7aca154d012cc9f9ac613d0bd9a Mon Sep 17 00:00:00 2001 From: Sandi Barr Date: Sat, 10 Sep 2022 16:32:01 -0500 Subject: [PATCH 1/7] feat(eslint-plugin-template): [interactive-supports-focus] add rule --- .../docs/rules/interactive-supports-focus.md | 798 ++++++++++++++++++ .../src/configs/all.json | 1 + packages/eslint-plugin-template/src/index.ts | 4 + .../src/rules/interactive-supports-focus.ts | 86 ++ .../src/utils/is-disabled-element.ts | 23 + ...et-non-interactive-element-role-schemas.ts | 42 +- .../src/utils/is-interactive-element/index.ts | 18 +- .../rules/interactive-supports-focus/cases.ts | 254 ++++++ .../rules/interactive-supports-focus/spec.ts | 12 + 9 files changed, 1219 insertions(+), 19 deletions(-) create mode 100644 packages/eslint-plugin-template/docs/rules/interactive-supports-focus.md create mode 100644 packages/eslint-plugin-template/src/rules/interactive-supports-focus.ts create mode 100644 packages/eslint-plugin-template/src/utils/is-disabled-element.ts create mode 100644 packages/eslint-plugin-template/tests/rules/interactive-supports-focus/cases.ts create mode 100644 packages/eslint-plugin-template/tests/rules/interactive-supports-focus/spec.ts diff --git a/packages/eslint-plugin-template/docs/rules/interactive-supports-focus.md b/packages/eslint-plugin-template/docs/rules/interactive-supports-focus.md new file mode 100644 index 000000000..46c73eefb --- /dev/null +++ b/packages/eslint-plugin-template/docs/rules/interactive-supports-focus.md @@ -0,0 +1,798 @@ + + +
+ +# `@angular-eslint/template/interactive-supports-focus` + +Ensures that elements with interactive handlers like `(click)` are focusable. + +- Type: suggestion + +
+ +## Rule Options + +The rule does not have any configuration options. + +
+ +## Usage Examples + +> The following examples are generated automatically from the actual unit tests within the plugin, so you can be assured that their behavior is accurate based on the current commit. + +
+ +
+❌ - Toggle examples of incorrect code for this rule + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```html +
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```html +
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```html +
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```html +
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```html +
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```html +
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```html +Submit +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```html +
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```html +
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```html + +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```html +Click me +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +``` + +
+ +
+ +--- + +
+ +
+✅ - Toggle examples of correct code for this rule + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```html +
+``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```html +
+ +
+``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```html +
+
+``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```html +
+
+``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```html +
+
+``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```html + + + +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```html + + + + + + +Foo + +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```html + + +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```html +Click me +Click me', +Click me', +Click me', +Click me', +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```html +hash +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```html +x.y.z +x.y.z +Click ALL the things! +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```html +x.y.z +x.y.z +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```html + + +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```html +
+
+Submit +Submit +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```html +Click me! +Click me! +Click me! +Click me! +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```html +
+
+
+
+
+
+
+
+
+
+
+
+
+``` + +
+ +
diff --git a/packages/eslint-plugin-template/src/configs/all.json b/packages/eslint-plugin-template/src/configs/all.json index 10af6f97f..6169d44c4 100644 --- a/packages/eslint-plugin-template/src/configs/all.json +++ b/packages/eslint-plugin-template/src/configs/all.json @@ -13,6 +13,7 @@ "@angular-eslint/template/cyclomatic-complexity": "error", "@angular-eslint/template/eqeqeq": "error", "@angular-eslint/template/i18n": "error", + "@angular-eslint/template/interactive-supports-focus": "error", "@angular-eslint/template/mouse-events-have-key-events": "error", "@angular-eslint/template/no-any": "error", "@angular-eslint/template/no-autofocus": "error", diff --git a/packages/eslint-plugin-template/src/index.ts b/packages/eslint-plugin-template/src/index.ts index 4f2deb265..f712f6ce5 100644 --- a/packages/eslint-plugin-template/src/index.ts +++ b/packages/eslint-plugin-template/src/index.ts @@ -35,6 +35,9 @@ import cyclomaticComplexity, { } from './rules/cyclomatic-complexity'; import eqeqeq, { RULE_NAME as eqeqeqRuleName } from './rules/eqeqeq'; import i18n, { RULE_NAME as i18nRuleName } from './rules/i18n'; +import interactiveSupportsFocus, { + RULE_NAME as interactiveSupportsFocusRuleName, +} from './rules/interactive-supports-focus'; import mouseEventsHaveKeyEvents, { RULE_NAME as mouseEventsHaveKeyEventsRuleName, } from './rules/mouse-events-have-key-events'; @@ -86,6 +89,7 @@ export default { [cyclomaticComplexityRuleName]: cyclomaticComplexity, [eqeqeqRuleName]: eqeqeq, [i18nRuleName]: i18n, + [interactiveSupportsFocusRuleName]: interactiveSupportsFocus, [mouseEventsHaveKeyEventsRuleName]: mouseEventsHaveKeyEvents, [noAnyRuleName]: noAny, [noAutofocusRuleName]: noAutofocus, diff --git a/packages/eslint-plugin-template/src/rules/interactive-supports-focus.ts b/packages/eslint-plugin-template/src/rules/interactive-supports-focus.ts new file mode 100644 index 000000000..2d78b4732 --- /dev/null +++ b/packages/eslint-plugin-template/src/rules/interactive-supports-focus.ts @@ -0,0 +1,86 @@ +import type { TmplAstElement } from '@angular-eslint/bundled-angular-compiler'; +import { + createESLintRule, + getTemplateParserServices, +} from '../utils/create-eslint-rule'; +import { getDomElements } from '../utils/get-dom-elements'; +import { isHiddenFromScreenReader } from '../utils/is-hidden-from-screen-reader'; +import { + isInteractiveElement, + isNonInteractiveRole, +} from '../utils/is-interactive-element'; +import { isDisabledElement } from '../utils/is-disabled-element'; +import { isPresentationRole } from '../utils/is-presentation-role'; + +type Options = []; +export type MessageIds = 'interactiveSupportsFocus'; +export const RULE_NAME = 'interactive-supports-focus'; + +export default createESLintRule({ + name: RULE_NAME, + meta: { + type: 'suggestion', + docs: { + description: + 'Ensures that elements with interactive handlers like `(click)` are focusable.', + recommended: false, + }, + schema: [], + messages: { + interactiveSupportsFocus: + 'Elements with interaction handlers must be focusable.', + }, + }, + defaultOptions: [], + create(context) { + return { + Element$1(node: TmplAstElement) { + const elementType = node.name; + if (!getDomElements().has(elementType)) { + return; + } + + const tabIndex = [...node.attributes, ...node.inputs].find( + (attr) => attr.name === 'tabindex', + ); + + const interactiveOutput = node.outputs.find( + (output: { name: string }) => + output.name === 'click' || + output.name.startsWith('keyup') || + output.name.startsWith('keydown') || + output.name.startsWith('keypress'), + ); + + if ( + !interactiveOutput || + isDisabledElement(node) || + isHiddenFromScreenReader(node) || + isPresentationRole(node) + ) { + // Presentation is an intentional signal from the author + // that this element is not meant to be perceivable. + // For example, a click screen overlay to close a dialog. + return; + } + + if ( + interactiveOutput && + !tabIndex && + !isInteractiveElement(node) && + !isNonInteractiveRole(node) + ) { + const parserServices = getTemplateParserServices(context); + const loc = parserServices.convertNodeSourceSpanToLoc( + node.sourceSpan, + ); + const messageId: MessageIds = 'interactiveSupportsFocus'; + context.report({ + loc, + messageId, + }); + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin-template/src/utils/is-disabled-element.ts b/packages/eslint-plugin-template/src/utils/is-disabled-element.ts new file mode 100644 index 000000000..f014dd205 --- /dev/null +++ b/packages/eslint-plugin-template/src/utils/is-disabled-element.ts @@ -0,0 +1,23 @@ +import type { TmplAstElement } from '@angular-eslint/bundled-angular-compiler'; +import { getOriginalAttributeName } from './get-original-attribute-name'; +import { getAttributeValue } from './get-attribute-value'; + +export function isDisabledElement(node: TmplAstElement): boolean { + const attributesInputs = [...node.attributes, ...node.inputs]; + const disabledAttr = attributesInputs.find( + (attr) => getOriginalAttributeName(attr) === 'disabled', + ); + const disabledValue = getAttributeValue(node, 'disabled'); + const isHTML5Disabled = disabledAttr && disabledValue !== undefined; + if (isHTML5Disabled) { + return true; + } + + const isAriaDisabled = + String(getAttributeValue(node, 'aria-disabled')).toLowerCase() === 'true'; + if (isAriaDisabled) { + return true; + } + + return false; +} diff --git a/packages/eslint-plugin-template/src/utils/is-interactive-element/get-non-interactive-element-role-schemas.ts b/packages/eslint-plugin-template/src/utils/is-interactive-element/get-non-interactive-element-role-schemas.ts index 03dfab062..29634e2b3 100644 --- a/packages/eslint-plugin-template/src/utils/is-interactive-element/get-non-interactive-element-role-schemas.ts +++ b/packages/eslint-plugin-template/src/utils/is-interactive-element/get-non-interactive-element-role-schemas.ts @@ -2,17 +2,35 @@ import type { ARIARoleDefintionKey, ARIARoleRelationConcept } from 'aria-query'; import { elementRoles, roles } from 'aria-query'; let nonInteractiveElementRoleSchemas: ARIARoleRelationConcept[] | null = null; +let nonInteractiveRoles: Set | null = null; -// This function follows the lazy initialization pattern. -// Since this is a top-level module (it will be included via `require`), we do not need to -// initialize the `nonInteractiveElementRoleSchemas` until the function is called -// for the first time, so we will not take up the memory. +// These functions follow the lazy initialization pattern. +// Since this is a top-level module (it will be included via `require`), +// we do not need to initialize the `nonInteractiveElementRoleSchemas` or +// `nonInteractiveRoles` until the functions are called for the first time, +// so we will not take up the memory. export function getNonInteractiveElementRoleSchemas(): ARIARoleRelationConcept[] { if (nonInteractiveElementRoleSchemas === null) { - const roleKeys = [...roles.keys()]; const elementRoleEntries = [...elementRoles.entries()]; - const nonInteractiveRoles = new Set( + nonInteractiveElementRoleSchemas = elementRoleEntries.reduce< + ARIARoleRelationConcept[] + >((accumulator, [elementSchema, roleSet]) => { + return accumulator.concat( + [...roleSet].every((role) => getNonInteractiveRoles().has(role)) + ? elementSchema + : [], + ); + }, []); + } + + return nonInteractiveElementRoleSchemas; +} + +export function getNonInteractiveRoles(): Set { + if (nonInteractiveRoles === null) { + const roleKeys = [...roles.keys()]; + nonInteractiveRoles = new Set( roleKeys .filter((name) => { const role = roles.get(name); @@ -31,17 +49,7 @@ export function getNonInteractiveElementRoleSchemas(): ARIARoleRelationConcept[] 'progressbar', ), ); - - nonInteractiveElementRoleSchemas = elementRoleEntries.reduce< - ARIARoleRelationConcept[] - >((accumulator, [elementSchema, roleSet]) => { - return accumulator.concat( - [...roleSet].every((role) => nonInteractiveRoles.has(role)) - ? elementSchema - : [], - ); - }, []); } - return nonInteractiveElementRoleSchemas; + return nonInteractiveRoles; } diff --git a/packages/eslint-plugin-template/src/utils/is-interactive-element/index.ts b/packages/eslint-plugin-template/src/utils/is-interactive-element/index.ts index f91ad69f6..22614d89c 100644 --- a/packages/eslint-plugin-template/src/utils/is-interactive-element/index.ts +++ b/packages/eslint-plugin-template/src/utils/is-interactive-element/index.ts @@ -1,10 +1,14 @@ import type { TmplAstElement } from '@angular-eslint/bundled-angular-compiler'; -import type { ARIARoleRelationConcept } from 'aria-query'; +import type { ARIARole, ARIARoleRelationConcept } from 'aria-query'; import { attributesComparator } from '../attributes-comparator'; +import { getAttributeValue } from '../get-attribute-value'; import { getDomElements } from '../get-dom-elements'; import { getInteractiveElementAXObjectSchemas } from './get-interactive-element-ax-object-schemas'; import { getInteractiveElementRoleSchemas } from './get-interactive-element-role-schemas'; -import { getNonInteractiveElementRoleSchemas } from './get-non-interactive-element-role-schemas'; +import { + getNonInteractiveElementRoleSchemas, + getNonInteractiveRoles, +} from './get-non-interactive-element-role-schemas'; function checkIsInteractiveElement(node: TmplAstElement): boolean { function elementSchemaMatcher({ attributes, name }: ARIARoleRelationConcept) { @@ -28,6 +32,12 @@ function checkIsInteractiveElement(node: TmplAstElement): boolean { return getInteractiveElementAXObjectSchemas().some(elementSchemaMatcher); } +function checkIsNonInteractiveRole(node: TmplAstElement): boolean { + return getNonInteractiveRoles().has( + getAttributeValue(node, 'role') as ARIARole, + ); +} + /** * Returns boolean indicating whether the given element is * interactive on the DOM or not. Usually used when an element @@ -37,3 +47,7 @@ function checkIsInteractiveElement(node: TmplAstElement): boolean { export function isInteractiveElement(node: TmplAstElement): boolean { return getDomElements().has(node.name) && checkIsInteractiveElement(node); } + +export function isNonInteractiveRole(node: TmplAstElement): boolean { + return checkIsNonInteractiveRole(node); +} diff --git a/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/cases.ts b/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/cases.ts new file mode 100644 index 000000000..ff40c81b2 --- /dev/null +++ b/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/cases.ts @@ -0,0 +1,254 @@ +import { convertAnnotatedSourceToFailureCase } from '@angular-eslint/utils'; +import type { MessageIds } from '../../../src/rules/interactive-supports-focus'; + +const messageId: MessageIds = 'interactiveSupportsFocus'; + +export const valid = [ + // no interactive outputs + { code: '
' }, + + // aria-hidden + { + code: ` +
+ +
+ `, + }, + + // aria-disabled + { + code: ` +
+
+ `, + }, + + // presentation role + { + code: ` +
+
+ `, + }, + + // explicitly assigned not interactive role + { + code: ` +
+
+ `, + }, + + // hidden input is not interactive, tabindex is not required + { + code: ` + + + + `, + }, + + // interactive elements + { + code: ` + + + + + + + Foo + + `, + }, + + // area without href needs tabindex for focus + { + code: ` + + + `, + }, + + // a without href needs tabindex for focus + { + code: ` + Click me + Click me', + Click me', + Click me', + Click me', + `, + }, + + // interactive role with href and click + { code: 'hash' }, + // href and click + { + code: ` + x.y.z + x.y.z + Click ALL the things! + `, + }, + // click and tabindex (focusable but generally not recommended) + { + code: ` + x.y.z + x.y.z + `, + }, + // routerLink + { + code: ` + + + `, + }, + + // invalid tabindex + { + code: ` +
+
+ Submit + Submit + `, + }, + + // valid tabindex + { + code: ` + Click me! + Click me! + Click me! + Click me! + `, + }, + + // interactive role with tabindex + { + code: ` +
+
+
+
+
+
+
+
+
+
+
+
+
+ `, + }, +]; + +export const invalid = [ + // aria-hidden="false" + convertAnnotatedSourceToFailureCase({ + description: 'should fail not hidden from screen reader', + annotatedSource: ` +
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + `, + messageId, + }), + convertAnnotatedSourceToFailureCase({ + description: 'should fail not hidden from screen reader', + annotatedSource: ` +
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + `, + messageId, + }), + // aria-disabled="false" + convertAnnotatedSourceToFailureCase({ + description: 'should fail not hidden from screen reader', + annotatedSource: ` +
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + `, + messageId, + }), + convertAnnotatedSourceToFailureCase({ + description: 'should fail not hidden from screen reader', + annotatedSource: ` +
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + `, + messageId, + }), + + // interactive role, non interactive element + convertAnnotatedSourceToFailureCase({ + description: + 'should fail interactive role but element does not support focus', + annotatedSource: ` +
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + `, + messageId, + }), + + // no role, non interactive element + convertAnnotatedSourceToFailureCase({ + description: 'should fail non-interactive element does not support focus', + annotatedSource: ` +
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + `, + messageId, + }), + convertAnnotatedSourceToFailureCase({ + description: 'should fail non-interactive element does not support focus', + annotatedSource: ` + Submit + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + `, + messageId, + }), + convertAnnotatedSourceToFailureCase({ + description: 'should fail non-interactive element does not support focus', + annotatedSource: ` +
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + `, + messageId, + }), + + // invalid role, non interactive element + convertAnnotatedSourceToFailureCase({ + description: + 'should fail non-interactive element with invalid role does not support focus', + annotatedSource: ` +
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + `, + messageId, + data: { + role: 'invalid', + }, + }), + + // area and a are not interactive without href + convertAnnotatedSourceToFailureCase({ + description: 'should fail non-interactive element does not support focus', + annotatedSource: ` + + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + `, + messageId, + }), + convertAnnotatedSourceToFailureCase({ + description: 'should fail non-interactive element does not support focus', + annotatedSource: ` + Click me + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + `, + messageId, + }), +]; diff --git a/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/spec.ts b/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/spec.ts new file mode 100644 index 000000000..32e486eb7 --- /dev/null +++ b/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/spec.ts @@ -0,0 +1,12 @@ +import { RuleTester } from '@angular-eslint/utils'; +import rule, { RULE_NAME } from '../../../src/rules/interactive-supports-focus'; +import { invalid, valid } from './cases'; + +const ruleTester = new RuleTester({ + parser: '@angular-eslint/template-parser', +}); + +ruleTester.run(RULE_NAME, rule, { + valid, + invalid, +}); From ef148928b782c123bcd817f266428e8c9ecca5bf Mon Sep 17 00:00:00 2001 From: Sandi Barr Date: Fri, 16 Sep 2022 15:47:19 -0500 Subject: [PATCH 2/7] feat(eslint-plugin-template): [interactive-supports-focus] move tabindex to avoid when not needed --- .../src/rules/interactive-supports-focus.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin-template/src/rules/interactive-supports-focus.ts b/packages/eslint-plugin-template/src/rules/interactive-supports-focus.ts index 2d78b4732..e737af3fe 100644 --- a/packages/eslint-plugin-template/src/rules/interactive-supports-focus.ts +++ b/packages/eslint-plugin-template/src/rules/interactive-supports-focus.ts @@ -40,10 +40,6 @@ export default createESLintRule({ return; } - const tabIndex = [...node.attributes, ...node.inputs].find( - (attr) => attr.name === 'tabindex', - ); - const interactiveOutput = node.outputs.find( (output: { name: string }) => output.name === 'click' || @@ -64,6 +60,10 @@ export default createESLintRule({ return; } + const tabIndex = [...node.attributes, ...node.inputs].find( + (attr) => attr.name === 'tabindex', + ); + if ( interactiveOutput && !tabIndex && From 355bcc46310bfba38618f7179877c7dd9ac7983e Mon Sep 17 00:00:00 2001 From: Sandi Barr Date: Fri, 16 Sep 2022 16:29:47 -0500 Subject: [PATCH 3/7] feat(eslint-plugin-template): [interactive-supports-focus] add disabled test cases --- .../docs/rules/interactive-supports-focus.md | 28 +++++++++++++++++++ .../rules/interactive-supports-focus/cases.ts | 9 ++++++ 2 files changed, 37 insertions(+) diff --git a/packages/eslint-plugin-template/docs/rules/interactive-supports-focus.md b/packages/eslint-plugin-template/docs/rules/interactive-supports-focus.md index 46c73eefb..8280f038a 100644 --- a/packages/eslint-plugin-template/docs/rules/interactive-supports-focus.md +++ b/packages/eslint-plugin-template/docs/rules/interactive-supports-focus.md @@ -554,6 +554,34 @@ The rule does not have any configuration options. #### ✅ Valid Code +```html + + + +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + ```html diff --git a/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/cases.ts b/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/cases.ts index ff40c81b2..fbb299c84 100644 --- a/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/cases.ts +++ b/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/cases.ts @@ -63,6 +63,15 @@ export const valid = [ `, }, + // disabled + { + code: ` + + + + `, + }, + // area without href needs tabindex for focus { code: ` From 8e455ccd2d4248b7cb617cf947526bd99f881215 Mon Sep 17 00:00:00 2001 From: Sandi Barr Date: Fri, 16 Sep 2022 16:30:36 -0500 Subject: [PATCH 4/7] feat(eslint-plugin-template): [interactive-supports-focus] unique invalid test descriptions --- .../docs/rules/interactive-supports-focus.md | 27 ------------------- .../rules/interactive-supports-focus/cases.ts | 24 +++++++---------- 2 files changed, 10 insertions(+), 41 deletions(-) diff --git a/packages/eslint-plugin-template/docs/rules/interactive-supports-focus.md b/packages/eslint-plugin-template/docs/rules/interactive-supports-focus.md index 8280f038a..8fc10c838 100644 --- a/packages/eslint-plugin-template/docs/rules/interactive-supports-focus.md +++ b/packages/eslint-plugin-template/docs/rules/interactive-supports-focus.md @@ -189,33 +189,6 @@ The rule does not have any configuration options. #### ❌ Invalid Code -```html -
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -``` - -
- ---- - -
- -#### Default Config - -```json -{ - "rules": { - "@angular-eslint/template/interactive-supports-focus": [ - "error" - ] - } -} -``` - -
- -#### ❌ Invalid Code - ```html Submit ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/cases.ts b/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/cases.ts index fbb299c84..7285e257c 100644 --- a/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/cases.ts +++ b/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/cases.ts @@ -167,7 +167,8 @@ export const invalid = [ messageId, }), convertAnnotatedSourceToFailureCase({ - description: 'should fail not hidden from screen reader', + description: + 'should fail not hidden from screen reader with bound aria-hidden attribute', annotatedSource: `
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -176,7 +177,7 @@ export const invalid = [ }), // aria-disabled="false" convertAnnotatedSourceToFailureCase({ - description: 'should fail not hidden from screen reader', + description: 'should fail aria-disabled is false', annotatedSource: `
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -184,7 +185,7 @@ export const invalid = [ messageId, }), convertAnnotatedSourceToFailureCase({ - description: 'should fail not hidden from screen reader', + description: 'should fail aria-disabled is false with bound attribute', annotatedSource: `
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -204,14 +205,6 @@ export const invalid = [ }), // no role, non interactive element - convertAnnotatedSourceToFailureCase({ - description: 'should fail non-interactive element does not support focus', - annotatedSource: ` -
- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - `, - messageId, - }), convertAnnotatedSourceToFailureCase({ description: 'should fail non-interactive element does not support focus', annotatedSource: ` @@ -221,7 +214,8 @@ export const invalid = [ messageId, }), convertAnnotatedSourceToFailureCase({ - description: 'should fail non-interactive element does not support focus', + description: + 'should fail non-interactive element with aria-label does not support focus', annotatedSource: `
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -245,7 +239,8 @@ export const invalid = [ // area and a are not interactive without href convertAnnotatedSourceToFailureCase({ - description: 'should fail non-interactive element does not support focus', + description: + 'should fail non-interactive element does not support focus, area should have href', annotatedSource: ` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -253,7 +248,8 @@ export const invalid = [ messageId, }), convertAnnotatedSourceToFailureCase({ - description: 'should fail non-interactive element does not support focus', + description: + 'should fail non-interactive element does not support focus, anchor should have href', annotatedSource: ` Click me ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From 8a263ac14c5aa729227ebd1868695754502072dd Mon Sep 17 00:00:00 2001 From: Sandi Barr Date: Sun, 18 Sep 2022 12:39:17 -0500 Subject: [PATCH 5/7] feat(eslint-plugin-template): [interactive-supports-focus] tests with key events --- .../docs/rules/interactive-supports-focus.md | 33 +++++++++++++++++-- .../rules/interactive-supports-focus/cases.ts | 17 ++++++++-- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/packages/eslint-plugin-template/docs/rules/interactive-supports-focus.md b/packages/eslint-plugin-template/docs/rules/interactive-supports-focus.md index 8fc10c838..53dbc0c57 100644 --- a/packages/eslint-plugin-template/docs/rules/interactive-supports-focus.md +++ b/packages/eslint-plugin-template/docs/rules/interactive-supports-focus.md @@ -302,6 +302,33 @@ The rule does not have any configuration options. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```html +
Cannot be focused
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +``` +
@@ -495,14 +522,14 @@ The rule does not have any configuration options. #### ✅ Valid Code ```html - - + + Foo - + ```
diff --git a/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/cases.ts b/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/cases.ts index 7285e257c..60ca8e018 100644 --- a/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/cases.ts +++ b/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/cases.ts @@ -52,14 +52,14 @@ export const valid = [ // interactive elements { code: ` - - + + Foo - + `, }, @@ -256,4 +256,15 @@ export const invalid = [ `, messageId, }), + + // non-interactive element with keyup, keydown, keypress interaction handlers + convertAnnotatedSourceToFailureCase({ + description: + 'should fail non-interactive element with key event does not support focus', + annotatedSource: ` +
Cannot be focused
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + `, + messageId, + }), ]; From b2d6a47642dfc43dc025d4e9a1d448d426460186 Mon Sep 17 00:00:00 2001 From: Sandi Barr Date: Sun, 18 Sep 2022 17:29:17 -0500 Subject: [PATCH 6/7] feat(eslint-plugin-template): [accessibility-interactive-supports-focus] prefix rule name --- ...cessibility-interactive-supports-focus.md} | 117 +++++++++++++----- .../src/configs/all.json | 2 +- packages/eslint-plugin-template/src/index.ts | 9 +- ...cessibility-interactive-supports-focus.ts} | 2 +- .../cases.ts | 2 +- .../spec.ts | 4 +- 6 files changed, 97 insertions(+), 39 deletions(-) rename packages/eslint-plugin-template/docs/rules/{interactive-supports-focus.md => accessibility-interactive-supports-focus.md} (74%) rename packages/eslint-plugin-template/src/rules/{interactive-supports-focus.ts => accessibility-interactive-supports-focus.ts} (97%) rename packages/eslint-plugin-template/tests/rules/{interactive-supports-focus => accessibility-interactive-supports-focus}/cases.ts (98%) rename packages/eslint-plugin-template/tests/rules/{interactive-supports-focus => accessibility-interactive-supports-focus}/spec.ts (70%) diff --git a/packages/eslint-plugin-template/docs/rules/interactive-supports-focus.md b/packages/eslint-plugin-template/docs/rules/accessibility-interactive-supports-focus.md similarity index 74% rename from packages/eslint-plugin-template/docs/rules/interactive-supports-focus.md rename to packages/eslint-plugin-template/docs/rules/accessibility-interactive-supports-focus.md index 53dbc0c57..fb7bae82c 100644 --- a/packages/eslint-plugin-template/docs/rules/interactive-supports-focus.md +++ b/packages/eslint-plugin-template/docs/rules/accessibility-interactive-supports-focus.md @@ -3,8 +3,8 @@ DO NOT EDIT. This markdown file was autogenerated using a mixture of the following files as the source of truth for its data: - - ../../src/rules/interactive-supports-focus.ts - - ../../tests/rules/interactive-supports-focus/cases.ts + - ../../src/rules/accessibility-interactive-supports-focus.ts + - ../../tests/rules/accessibility-interactive-supports-focus/cases.ts In order to update this file, it is therefore those files which need to be updated, as well as potentially the generator script: - ../../../../tools/scripts/generate-rule-docs.ts @@ -13,7 +13,7 @@
-# `@angular-eslint/template/interactive-supports-focus` +# `@angular-eslint/template/accessibility-interactive-supports-focus` Ensures that elements with interactive handlers like `(click)` are focusable. @@ -43,7 +43,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -70,7 +70,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -97,7 +97,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -124,7 +124,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -151,7 +151,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -178,7 +178,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -205,7 +205,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -232,7 +232,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -259,7 +259,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -286,7 +286,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -313,7 +313,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -329,6 +329,33 @@ The rule does not have any configuration options. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/accessibility-interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```html +
Cannot be focused
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +``` +
@@ -347,7 +374,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -373,7 +400,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -401,7 +428,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -428,7 +455,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -455,7 +482,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -482,7 +509,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -510,7 +537,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -543,7 +570,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -571,7 +598,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -598,7 +625,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -628,7 +655,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -654,7 +681,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -682,7 +709,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -709,7 +736,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -736,7 +763,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -765,7 +792,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -794,7 +821,7 @@ The rule does not have any configuration options. ```json { "rules": { - "@angular-eslint/template/interactive-supports-focus": [ + "@angular-eslint/template/accessibility-interactive-supports-focus": [ "error" ] } @@ -821,6 +848,34 @@ The rule does not have any configuration options.
``` +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/template/accessibility-interactive-supports-focus": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```html +
Edit this text
+
Edit this text
+
Edit this too!
+``` +
diff --git a/packages/eslint-plugin-template/src/configs/all.json b/packages/eslint-plugin-template/src/configs/all.json index 6169d44c4..71aaf1e4e 100644 --- a/packages/eslint-plugin-template/src/configs/all.json +++ b/packages/eslint-plugin-template/src/configs/all.json @@ -3,6 +3,7 @@ "rules": { "@angular-eslint/template/accessibility-alt-text": "error", "@angular-eslint/template/accessibility-elements-content": "error", + "@angular-eslint/template/accessibility-interactive-supports-focus": "error", "@angular-eslint/template/accessibility-label-for": "error", "@angular-eslint/template/accessibility-label-has-associated-control": "error", "@angular-eslint/template/accessibility-table-scope": "error", @@ -13,7 +14,6 @@ "@angular-eslint/template/cyclomatic-complexity": "error", "@angular-eslint/template/eqeqeq": "error", "@angular-eslint/template/i18n": "error", - "@angular-eslint/template/interactive-supports-focus": "error", "@angular-eslint/template/mouse-events-have-key-events": "error", "@angular-eslint/template/no-any": "error", "@angular-eslint/template/no-autofocus": "error", diff --git a/packages/eslint-plugin-template/src/index.ts b/packages/eslint-plugin-template/src/index.ts index f712f6ce5..756384f88 100644 --- a/packages/eslint-plugin-template/src/index.ts +++ b/packages/eslint-plugin-template/src/index.ts @@ -9,6 +9,9 @@ import accessibilityAltText, { import accessibilityElementsContent, { RULE_NAME as accessibilityElementsContentRuleName, } from './rules/accessibility-elements-content'; +import accessibilityInteractiveSupportsFocus, { + RULE_NAME as accessibilityInteractiveSupportsFocusRuleName, +} from './rules/accessibility-interactive-supports-focus'; import accessibilityLabelFor, { RULE_NAME as accessibilityLabelForRuleName, } from './rules/accessibility-label-for'; @@ -35,9 +38,6 @@ import cyclomaticComplexity, { } from './rules/cyclomatic-complexity'; import eqeqeq, { RULE_NAME as eqeqeqRuleName } from './rules/eqeqeq'; import i18n, { RULE_NAME as i18nRuleName } from './rules/i18n'; -import interactiveSupportsFocus, { - RULE_NAME as interactiveSupportsFocusRuleName, -} from './rules/interactive-supports-focus'; import mouseEventsHaveKeyEvents, { RULE_NAME as mouseEventsHaveKeyEventsRuleName, } from './rules/mouse-events-have-key-events'; @@ -78,6 +78,8 @@ export default { rules: { [accessibilityAltTextRuleName]: accessibilityAltText, [accessibilityElementsContentRuleName]: accessibilityElementsContent, + [accessibilityInteractiveSupportsFocusRuleName]: + accessibilityInteractiveSupportsFocus, [accessibilityLabelForRuleName]: accessibilityLabelFor, [accessibilityLabelHasAssociatedControlRuleName]: accessibilityLabelHasAssociatedControl, @@ -89,7 +91,6 @@ export default { [cyclomaticComplexityRuleName]: cyclomaticComplexity, [eqeqeqRuleName]: eqeqeq, [i18nRuleName]: i18n, - [interactiveSupportsFocusRuleName]: interactiveSupportsFocus, [mouseEventsHaveKeyEventsRuleName]: mouseEventsHaveKeyEvents, [noAnyRuleName]: noAny, [noAutofocusRuleName]: noAutofocus, diff --git a/packages/eslint-plugin-template/src/rules/interactive-supports-focus.ts b/packages/eslint-plugin-template/src/rules/accessibility-interactive-supports-focus.ts similarity index 97% rename from packages/eslint-plugin-template/src/rules/interactive-supports-focus.ts rename to packages/eslint-plugin-template/src/rules/accessibility-interactive-supports-focus.ts index e737af3fe..230366e00 100644 --- a/packages/eslint-plugin-template/src/rules/interactive-supports-focus.ts +++ b/packages/eslint-plugin-template/src/rules/accessibility-interactive-supports-focus.ts @@ -14,7 +14,7 @@ import { isPresentationRole } from '../utils/is-presentation-role'; type Options = []; export type MessageIds = 'interactiveSupportsFocus'; -export const RULE_NAME = 'interactive-supports-focus'; +export const RULE_NAME = 'accessibility-interactive-supports-focus'; export default createESLintRule({ name: RULE_NAME, diff --git a/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/cases.ts b/packages/eslint-plugin-template/tests/rules/accessibility-interactive-supports-focus/cases.ts similarity index 98% rename from packages/eslint-plugin-template/tests/rules/interactive-supports-focus/cases.ts rename to packages/eslint-plugin-template/tests/rules/accessibility-interactive-supports-focus/cases.ts index 60ca8e018..e8cb05b78 100644 --- a/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/cases.ts +++ b/packages/eslint-plugin-template/tests/rules/accessibility-interactive-supports-focus/cases.ts @@ -1,5 +1,5 @@ import { convertAnnotatedSourceToFailureCase } from '@angular-eslint/utils'; -import type { MessageIds } from '../../../src/rules/interactive-supports-focus'; +import type { MessageIds } from '../../../src/rules/accessibility-interactive-supports-focus'; const messageId: MessageIds = 'interactiveSupportsFocus'; diff --git a/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/spec.ts b/packages/eslint-plugin-template/tests/rules/accessibility-interactive-supports-focus/spec.ts similarity index 70% rename from packages/eslint-plugin-template/tests/rules/interactive-supports-focus/spec.ts rename to packages/eslint-plugin-template/tests/rules/accessibility-interactive-supports-focus/spec.ts index 32e486eb7..d5d5f2abe 100644 --- a/packages/eslint-plugin-template/tests/rules/interactive-supports-focus/spec.ts +++ b/packages/eslint-plugin-template/tests/rules/accessibility-interactive-supports-focus/spec.ts @@ -1,5 +1,7 @@ import { RuleTester } from '@angular-eslint/utils'; -import rule, { RULE_NAME } from '../../../src/rules/interactive-supports-focus'; +import rule, { + RULE_NAME, +} from '../../../src/rules/accessibility-interactive-supports-focus'; import { invalid, valid } from './cases'; const ruleTester = new RuleTester({ From 486a6f7adffc2c49307fc8d4a56cdff920b7b40b Mon Sep 17 00:00:00 2001 From: Sandi Barr Date: Sun, 18 Sep 2022 17:34:05 -0500 Subject: [PATCH 7/7] feat(eslint-plugin-template): [accessibility-interactive-supports-focus] supports contenteditable --- ...ccessibility-interactive-supports-focus.ts | 4 +++- .../src/utils/is-content-editable.ts | 16 +++++++++++++++ .../cases.ts | 20 +++++++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 packages/eslint-plugin-template/src/utils/is-content-editable.ts diff --git a/packages/eslint-plugin-template/src/rules/accessibility-interactive-supports-focus.ts b/packages/eslint-plugin-template/src/rules/accessibility-interactive-supports-focus.ts index 230366e00..58733c572 100644 --- a/packages/eslint-plugin-template/src/rules/accessibility-interactive-supports-focus.ts +++ b/packages/eslint-plugin-template/src/rules/accessibility-interactive-supports-focus.ts @@ -9,6 +9,7 @@ import { isInteractiveElement, isNonInteractiveRole, } from '../utils/is-interactive-element'; +import { isContentEditable } from '../utils/is-content-editable'; import { isDisabledElement } from '../utils/is-disabled-element'; import { isPresentationRole } from '../utils/is-presentation-role'; @@ -68,7 +69,8 @@ export default createESLintRule({ interactiveOutput && !tabIndex && !isInteractiveElement(node) && - !isNonInteractiveRole(node) + !isNonInteractiveRole(node) && + !isContentEditable(node) ) { const parserServices = getTemplateParserServices(context); const loc = parserServices.convertNodeSourceSpanToLoc( diff --git a/packages/eslint-plugin-template/src/utils/is-content-editable.ts b/packages/eslint-plugin-template/src/utils/is-content-editable.ts new file mode 100644 index 000000000..98295f954 --- /dev/null +++ b/packages/eslint-plugin-template/src/utils/is-content-editable.ts @@ -0,0 +1,16 @@ +import type { TmplAstElement } from '@angular-eslint/bundled-angular-compiler'; +import { getOriginalAttributeName } from './get-original-attribute-name'; +import { getAttributeValue } from './get-attribute-value'; + +export function isContentEditable(node: TmplAstElement): boolean { + const attributesInputs = [...node.attributes, ...node.inputs]; + const contentEditableAttr = attributesInputs.find( + (attr) => getOriginalAttributeName(attr) === 'contenteditable', + ); + const contentEditableValue = getAttributeValue(node, 'contenteditable'); + return ( + !!contentEditableAttr && + (contentEditableValue === '' || + String(contentEditableValue).toLowerCase() === 'true') + ); +} diff --git a/packages/eslint-plugin-template/tests/rules/accessibility-interactive-supports-focus/cases.ts b/packages/eslint-plugin-template/tests/rules/accessibility-interactive-supports-focus/cases.ts index e8cb05b78..c991c9fc1 100644 --- a/packages/eslint-plugin-template/tests/rules/accessibility-interactive-supports-focus/cases.ts +++ b/packages/eslint-plugin-template/tests/rules/accessibility-interactive-supports-focus/cases.ts @@ -154,6 +154,15 @@ export const valid = [
`, }, + + // elements with contenteditable enabled are interactive by default + { + code: ` +
Edit this text
+
Edit this text
+
Edit this too!
+ `, + }, ]; export const invalid = [ @@ -267,4 +276,15 @@ export const invalid = [ `, messageId, }), + + // contenteditable="false" + convertAnnotatedSourceToFailureCase({ + description: + 'should fail non-interactive element with contenteditable disabled does not support focus', + annotatedSource: ` +
Cannot be focused
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + `, + messageId, + }), ];