From 77d6cb6acd82bbe6cfdf317c2a6a2921431a07ba Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 22 Apr 2026 09:37:15 -0600 Subject: [PATCH 1/3] Implement a11y-no-visually-hidden-interactive-elements linter rule Implements the `a11y-no-visually-hidden-interactive-elements` rule from erblint-github's `NoVisuallyHiddenInteractiveElements`. This rule flags interactive elements (a, button, summary, select, option, textarea) that have the `sr-only` CSS class, which visually hides them. Sighted keyboard users navigating to a visually hidden interactive element may become confused, thinking keyboard focus has been lost. Note: `input` elements are intentionally not flagged to avoid false positives (e.g. file inputs). Closes #1225 --- ...no-visually-hidden-interactive-elements.md | 56 +++++++++++++++ javascript/packages/linter/src/rules.ts | 2 + ...no-visually-hidden-interactive-elements.ts | 58 +++++++++++++++ javascript/packages/linter/src/rules/index.ts | 1 + .../test/__snapshots__/cli.test.ts.snap | 44 ++++++------ ...sually-hidden-interactive-elements.test.ts | 72 +++++++++++++++++++ 6 files changed, 211 insertions(+), 22 deletions(-) create mode 100644 javascript/packages/linter/docs/rules/a11y-no-visually-hidden-interactive-elements.md create mode 100644 javascript/packages/linter/src/rules/a11y-no-visually-hidden-interactive-elements.ts create mode 100644 javascript/packages/linter/test/rules/a11y-no-visually-hidden-interactive-elements.test.ts 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..f3df8f5f2 --- /dev/null +++ b/javascript/packages/linter/src/rules/a11y-no-visually-hidden-interactive-elements.ts @@ -0,0 +1,58 @@ +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"] + +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))) { + 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..548efeaca --- /dev/null +++ b/javascript/packages/linter/test/rules/a11y-no-visually-hidden-interactive-elements.test.ts @@ -0,0 +1,72 @@ +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("does not flag input elements", () => { + expectNoOffenses('') + }) +}) From 418fc1b69df0720c962301406a8b930f3cf75199 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 23 Apr 2026 11:30:53 -0600 Subject: [PATCH 2/3] Fix false positive for sr-only with focus undo classes Avoid flagging interactive elements that use sr-only alongside focus:not-sr-only or focus-within:not-sr-only, since these elements become visible on focus (e.g. skip-to-content links). Co-authored-by: Bruno Prieto --- .../rules/a11y-no-visually-hidden-interactive-elements.ts | 3 ++- .../a11y-no-visually-hidden-interactive-elements.test.ts | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) 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 index f3df8f5f2..91df5ab00 100644 --- 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 @@ -7,6 +7,7 @@ import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.j 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 { @@ -18,7 +19,7 @@ class NoVisuallyHiddenInteractiveElementsVisitor extends BaseRuleVisitor { if (classValue) { const classes = classValue.split(/\s+/) - if (VISUALLY_HIDDEN_CLASSES.some((cls) => classes.includes(cls))) { + 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, 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 index 548efeaca..c334a4b45 100644 --- 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 @@ -66,6 +66,14 @@ describe("a11y-no-visually-hidden-interactive-elements", () => { 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('') }) From 01e9958837372c0d80b963d1e6ed6c134371f324 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 8 May 2026 14:46:17 -0600 Subject: [PATCH 3/3] Add a11y-no-visually-hidden-interactive-elements to rules README --- javascript/packages/linter/docs/rules/README.md | 1 + 1 file changed, 1 insertion(+) 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