diff --git a/javascript/packages/linter/docs/rules/README.md b/javascript/packages/linter/docs/rules/README.md index a506bccaf..fa0ee682b 100644 --- a/javascript/packages/linter/docs/rules/README.md +++ b/javascript/packages/linter/docs/rules/README.md @@ -9,6 +9,7 @@ This page contains documentation for all Herb Linter rules. - [`a11y-no-accesskey-attribute`](./a11y-no-accesskey-attribute.md) - Prevent usage of the `accesskey` attribute - [`a11y-no-aria-unsupported-elements`](./a11y-no-aria-unsupported-elements.md) - Prevent usage of ARIA on unsupported elements - [`a11y-no-autofocus-attribute`](./a11y-no-autofocus-attribute.md) - Prevent usage of the `autofocus` attribute +- [`a11y-no-visually-hidden-interactive-elements`](./a11y-no-visually-hidden-interactive-elements.md) - Prevent visually hidden interactive elements #### Action View diff --git a/javascript/packages/linter/docs/rules/a11y-no-visually-hidden-interactive-elements.md b/javascript/packages/linter/docs/rules/a11y-no-visually-hidden-interactive-elements.md new file mode 100644 index 000000000..97ec9daf8 --- /dev/null +++ b/javascript/packages/linter/docs/rules/a11y-no-visually-hidden-interactive-elements.md @@ -0,0 +1,56 @@ +# Linter Rule: No visually hidden interactive elements + +**Rule:** `a11y-no-visually-hidden-interactive-elements` + +## Description + +Enforce that interactive elements are not visually hidden using classes like `sr-only`. + +## Rationale + +Visually hiding interactive elements can be confusing to sighted keyboard users as it appears their focus has been lost when they navigate to the hidden element. + +Note: `input` elements are not flagged at this time as some visually hidden inputs might cause false positives (e.g. file inputs). + +## Examples + +### ✅ Good + +```erb +

Welcome to GitHub

+``` + +```erb +Visually hidden text +``` + +```erb + +``` + +### 🚫 Bad + +```erb + +``` + +```erb +About +``` + +```erb +Details +``` + +```erb + +``` + +```erb + +``` + +## References + +- [erblint-github: NoVisuallyHiddenInteractiveElements](https://github.com/github/erblint-github/blob/main/lib/erblint-github/linters/github/accessibility/no_visually_hidden_interactive_elements.rb) +- [erblint-github docs](https://github.com/github/erblint-github/blob/main/docs/rules/accessibility/no-visually-hidden-interactive-elements.md) diff --git a/javascript/packages/linter/src/rules.ts b/javascript/packages/linter/src/rules.ts index 7a4f935b3..1c7459b04 100644 --- a/javascript/packages/linter/src/rules.ts +++ b/javascript/packages/linter/src/rules.ts @@ -3,6 +3,7 @@ import type { RuleClass } from "./types.js" import { A11yNoAccesskeyAttributeRule } from "./rules/a11y-no-accesskey-attribute.js" import { A11yNoAriaUnsupportedElementsRule } from "./rules/a11y-no-aria-unsupported-elements.js" import { A11yNoAutofocusAttributeRule } from "./rules/a11y-no-autofocus-attribute.js" +import { A11yNoVisuallyHiddenInteractiveElementsRule } from "./rules/a11y-no-visually-hidden-interactive-elements.js" import { ActionViewNoSilentHelperRule } from "./rules/actionview-no-silent-helper.js" import { ActionViewNoSilentRenderRule } from "./rules/actionview-no-silent-render.js" @@ -106,6 +107,7 @@ export const rules: RuleClass[] = [ A11yNoAccesskeyAttributeRule, A11yNoAriaUnsupportedElementsRule, A11yNoAutofocusAttributeRule, + A11yNoVisuallyHiddenInteractiveElementsRule, ActionViewNoSilentHelperRule, ActionViewNoSilentRenderRule, diff --git a/javascript/packages/linter/src/rules/a11y-no-visually-hidden-interactive-elements.ts b/javascript/packages/linter/src/rules/a11y-no-visually-hidden-interactive-elements.ts new file mode 100644 index 000000000..91df5ab00 --- /dev/null +++ b/javascript/packages/linter/src/rules/a11y-no-visually-hidden-interactive-elements.ts @@ -0,0 +1,59 @@ +import { type ParseResult, type ParserOptions, type HTMLElementNode, getTagLocalName, getStaticAttributeValue } from "@herb-tools/core" + +import { BaseRuleVisitor } from "./rule-utils.js" +import { ParserRule } from "../types.js" +import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js" + +const INTERACTIVE_ELEMENTS = ["a", "button", "summary", "select", "option", "textarea"] + +const VISUALLY_HIDDEN_CLASSES = ["sr-only"] +const VISUALLY_HIDDEN_UNDO_CLASSES = ["not-sr-only", "focus:not-sr-only", "focus-within:not-sr-only"] + +class NoVisuallyHiddenInteractiveElementsVisitor extends BaseRuleVisitor { + visitHTMLElementNode(node: HTMLElementNode): void { + const tagName = getTagLocalName(node) + + if (tagName && INTERACTIVE_ELEMENTS.includes(tagName)) { + const classValue = getStaticAttributeValue(node, "class") + + if (classValue) { + const classes = classValue.split(/\s+/) + + if (VISUALLY_HIDDEN_CLASSES.some((cls) => classes.includes(cls)) && !VISUALLY_HIDDEN_UNDO_CLASSES.some((cls) => classes.includes(cls))) { + this.addOffense( + "Avoid visually hiding interactive elements. Visually hiding interactive elements can be confusing to sighted keyboard users as it appears their focus has been lost when they navigate to the hidden element.", + node.tag_name!.location, + ) + } + } + } + + super.visitHTMLElementNode(node) + } +} + +export class A11yNoVisuallyHiddenInteractiveElementsRule extends ParserRule { + static ruleName = "a11y-no-visually-hidden-interactive-elements" + static introducedIn = this.version("0.9.4") + + get defaultConfig(): FullRuleConfig { + return { + enabled: false, + severity: "warning", + } + } + + get parserOptions(): Partial { + return { + action_view_helpers: true, + } + } + + check(result: ParseResult, context?: Partial): UnboundLintOffense[] { + const visitor = new NoVisuallyHiddenInteractiveElementsVisitor(this.ruleName, context) + + visitor.visit(result.value) + + return visitor.offenses + } +} diff --git a/javascript/packages/linter/src/rules/index.ts b/javascript/packages/linter/src/rules/index.ts index 815093506..9672d322a 100644 --- a/javascript/packages/linter/src/rules/index.ts +++ b/javascript/packages/linter/src/rules/index.ts @@ -1,6 +1,7 @@ export * from "./a11y-no-accesskey-attribute.js" export * from "./a11y-no-aria-unsupported-elements.js" export * from "./a11y-no-autofocus-attribute.js" +export * from "./a11y-no-visually-hidden-interactive-elements.js" export * from "./rule-utils.js" export * from "./prism-rule-utils.js" diff --git a/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap b/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap index 1b38e8234..04fb24b07 100644 --- a/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap +++ b/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap @@ -111,7 +111,7 @@ test/fixtures/ignored.html.erb:8:8 Offenses 5 errors | 2 warnings (7 offenses across 1 file) Note 3 additional offenses reported (would have been ignored) Fixable 7 offenses | 4 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > Excluded Files > skips excluded file in subdirectory with README.md project indicator 1`] = ` @@ -177,7 +177,7 @@ test/fixtures/test-file-with-errors.html.erb:2:22 Checked 1 file Offenses 2 errors | 1 warning (3 offenses across 1 file) Fixable 3 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > GitHub Actions format includes rule codes 1`] = ` @@ -202,7 +202,7 @@ test/fixtures/no-trailing-newline.html.erb:1:29 Checked 1 file Offenses 1 error | 0 warnings (1 offense across 1 file) Fixable 1 offense | 1 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > GitHub Actions format includes rule codes 2`] = ` @@ -266,7 +266,7 @@ test/fixtures/erb-no-extra-whitespace-inside-tags.html.erb:1:4 Checked 1 file Offenses 4 errors | 0 warnings (4 offenses across 1 file) Fixable 4 offenses | 3 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > Ignores disabled rules 1`] = ` @@ -307,7 +307,7 @@ test/fixtures/ignored.html.erb:6:14 Checked 1 file Offenses 2 errors | 0 warnings | 3 ignored (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > allows tag.attributes in attribute position 1`] = ` @@ -346,7 +346,7 @@ test/fixtures/tag-attributes.html.erb:5:0 Checked 1 file Offenses 2 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > diplays only parsers errors if one is present 1`] = ` @@ -372,7 +372,7 @@ test/fixtures/parser-errors.html.erb:2:16 Checked 1 file Offenses 1 error | 0 warnings (1 offense across 1 file) Fixable 0 offenses - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > displays most violated rules with multiple offenses 1`] = ` @@ -587,7 +587,7 @@ test/fixtures/multiple-rule-offenses.html.erb:4:2 Checked 1 file Offenses 8 errors | 6 warnings (14 offenses across 1 file) Fixable 14 offenses | 4 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > displays rule offenses when showing all rules 1`] = ` @@ -684,7 +684,7 @@ test/fixtures/few-rule-offenses.html.erb:6:0 Checked 1 file Offenses 4 errors | 2 warnings (6 offenses across 1 file) Fixable 6 offenses | 3 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > formats GitHub Actions output correctly for bad file 1`] = ` @@ -723,7 +723,7 @@ test/fixtures/bad-file.html.erb:1:16 Checked 1 file Offenses 2 errors | 0 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > formats GitHub Actions output correctly for clean file 1`] = ` @@ -735,7 +735,7 @@ exports[`CLI Output Formatting > formats GitHub Actions output correctly for cle Checked 1 file Offenses 0 offenses Fixable 0 offenses - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > formats GitHub Actions output correctly for file with errors 1`] = ` @@ -795,7 +795,7 @@ test/fixtures/test-file-with-errors.html.erb:2:22 Checked 1 file Offenses 2 errors | 1 warning (3 offenses across 1 file) Fixable 3 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > formats GitHub Actions output with --format=github option 1`] = ` @@ -834,7 +834,7 @@ test/fixtures/test-file-simple.html.erb:2:22 Checked 1 file Offenses 2 errors | 0 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > formats JSON output correctly for bad file 1`] = ` @@ -1038,7 +1038,7 @@ test/fixtures/test-file-with-errors.html.erb:2:22 Checked 1 file Offenses 2 errors | 1 warning (3 offenses across 1 file) Fixable 3 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > formats simple output correctly 1`] = ` @@ -1057,7 +1057,7 @@ test/fixtures/test-file-simple.html.erb: Checked 1 file Offenses 2 errors | 0 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > formats simple output for bad-file correctly 1`] = ` @@ -1076,7 +1076,7 @@ test/fixtures/bad-file.html.erb: Checked 1 file Offenses 2 errors | 0 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > formats success output correctly 1`] = ` @@ -1088,7 +1088,7 @@ exports[`CLI Output Formatting > formats success output correctly 1`] = ` Checked 1 file Offenses 0 offenses Fixable 0 offenses - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > handles boolean attributes 1`] = ` @@ -1100,7 +1100,7 @@ exports[`CLI Output Formatting > handles boolean attributes 1`] = ` Checked 1 file Offenses 0 offenses Fixable 0 offenses - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > handles multiple errors correctly 1`] = ` @@ -1135,7 +1135,7 @@ test/fixtures/bad-file.html.erb:1:16 Checked 1 file Offenses 2 errors | 0 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > herb:disable rules 1`] = ` @@ -1267,7 +1267,7 @@ test/fixtures/disabled-1.html.erb:14:19 Checked 1 file Offenses 2 errors | 6 warnings | 8 ignored (8 offenses across 1 file) Fixable 8 offenses | 1 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > herb:disable rules 2`] = ` @@ -1470,7 +1470,7 @@ test/fixtures/disabled-2.html.erb:2:44 Checked 1 file Offenses 6 errors | 7 warnings | 5 ignored (13 offenses across 1 file) Fixable 13 offenses | 6 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; exports[`CLI Output Formatting > uses GitHub Actions format by default when GITHUB_ACTIONS is true 1`] = ` @@ -1530,5 +1530,5 @@ test/fixtures/test-file-with-errors.html.erb:2:22 Checked 1 file Offenses 2 errors | 1 warning (3 offenses across 1 file) Fixable 3 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled" + Rules 83 enabled | 11 not enabled" `; diff --git a/javascript/packages/linter/test/rules/a11y-no-visually-hidden-interactive-elements.test.ts b/javascript/packages/linter/test/rules/a11y-no-visually-hidden-interactive-elements.test.ts new file mode 100644 index 000000000..c334a4b45 --- /dev/null +++ b/javascript/packages/linter/test/rules/a11y-no-visually-hidden-interactive-elements.test.ts @@ -0,0 +1,80 @@ +import { describe, test } from "vitest" +import { A11yNoVisuallyHiddenInteractiveElementsRule } from "../../src/rules/a11y-no-visually-hidden-interactive-elements.js" +import { createLinterTest } from "../helpers/linter-test-helper.js" + +const { expectNoOffenses, expectWarning, assertOffenses } = createLinterTest(A11yNoVisuallyHiddenInteractiveElementsRule) + +const MESSAGE = "Avoid visually hiding interactive elements. Visually hiding interactive elements can be confusing to sighted keyboard users as it appears their focus has been lost when they navigate to the hidden element." + +describe("a11y-no-visually-hidden-interactive-elements", () => { + test("passes for non-interactive element with sr-only", () => { + expectNoOffenses('

Welcome to GitHub

') + }) + + test("passes for non-interactive element with sr-only class among others", () => { + expectNoOffenses('Visually hidden text') + }) + + test("passes for interactive element without sr-only", () => { + expectNoOffenses('') + }) + + test("passes for interactive element without class attribute", () => { + expectNoOffenses('') + }) + + test("passes for div with sr-only", () => { + expectNoOffenses('
Hidden content
') + }) + + test("passes for p with sr-only", () => { + expectNoOffenses('

Hidden text

') + }) + + test("fails for button with sr-only", () => { + expectWarning(MESSAGE) + assertOffenses('') + }) + + test("fails for a with sr-only", () => { + expectWarning(MESSAGE) + assertOffenses('About') + }) + + test("fails for summary with sr-only", () => { + expectWarning(MESSAGE) + assertOffenses('Details') + }) + + test("fails for select with sr-only", () => { + expectWarning(MESSAGE) + assertOffenses('') + }) + + test("fails for option with sr-only", () => { + expectWarning(MESSAGE) + assertOffenses('') + }) + + test("fails for textarea with sr-only", () => { + expectWarning(MESSAGE) + assertOffenses('') + }) + + test("fails for button with sr-only among other classes", () => { + expectWarning(MESSAGE) + assertOffenses('') + }) + + test("passes for button with sr-only and focus:not-sr-only", () => { + expectNoOffenses('') + }) + + test("passes for a with sr-only and focus-within:not-sr-only", () => { + expectNoOffenses('Skip to content') + }) + + test("does not flag input elements", () => { + expectNoOffenses('') + }) +})