Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions javascript/packages/linter/docs/rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ This page contains documentation for all Herb Linter rules.
- [`html-attribute-values-require-quotes`](./html-attribute-values-require-quotes.md) - Requires quotes around attribute values
- [`html-avoid-both-disabled-and-aria-disabled`](./html-avoid-both-disabled-and-aria-disabled.md) - Avoid using both `disabled` and `aria-disabled` attributes
- [`html-body-only-elements`](./html-body-only-elements.md) - Require content elements inside `<body>`.
- [`html-details-has-summary`](./html-details-has-summary.md) - Require `<summary>` in `<details>` elements
- [`html-boolean-attributes-no-value`](./html-boolean-attributes-no-value.md) - Prevents values on boolean attributes
- [`html-head-only-elements`](./html-head-only-elements.md) - Require head-scoped elements inside `<head>`.
- [`html-iframe-has-title`](./html-iframe-has-title.md) - `iframe` elements must have a `title` attribute
Expand Down
46 changes: 46 additions & 0 deletions javascript/packages/linter/docs/rules/html-details-has-summary.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Linter Rule: `<details>` elements must have a `<summary>` child

**Rule:** `html-details-has-summary`

## Description

Ensure that all `<details>` elements have a direct `<summary>` child element that describes what the disclosure widget will expand.

## Rationale

The `<summary>` element provides a visible label for the `<details>` disclosure widget, hinting to the user what they'll be expanding. Screen reader users rely on `<summary>` elements to understand the purpose of the expandable content. If a developer omits the `<summary>`, the user agent adds a default one with no meaningful context. The `<summary>` must be a direct child of `<details>` to function correctly — nesting it inside another element will not work as intended.

## Examples

### ✅ Good

```erb
<details>
<summary>Expand me!</summary>
I do have a summary tag!
</details>

<details>
I do have a summary tag!
<summary>Expand me!</summary>
</details>
```

### 🚫 Bad

```erb
<details>
I don't have a summary tag!
</details>

<details>
<div><summary>Expand me!</summary></div>
The summary tag needs to be a direct child of the details tag.
</details>
```

## References

- [HTML: `details` element](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/details)
- [HTML: `summary` element](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/summary)
- [erblint-github: GitHub::Accessibility::DetailsHasSummary](https://github.com/github/erblint-github/pull/23)
2 changes: 2 additions & 0 deletions javascript/packages/linter/src/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import { HTMLAttributeEqualsSpacingRule } from "./rules/html-attribute-equals-sp
import { HTMLAttributeValuesRequireQuotesRule } from "./rules/html-attribute-values-require-quotes.js"
import { HTMLAvoidBothDisabledAndAriaDisabledRule } from "./rules/html-avoid-both-disabled-and-aria-disabled.js"
import { HTMLBodyOnlyElementsRule } from "./rules/html-body-only-elements.js"
import { HTMLDetailsHasSummaryRule } from "./rules/html-details-has-summary.js"
import { HTMLBooleanAttributesNoValueRule } from "./rules/html-boolean-attributes-no-value.js"
import { HTMLHeadOnlyElementsRule } from "./rules/html-head-only-elements.js"
import { HTMLIframeHasTitleRule } from "./rules/html-iframe-has-title.js"
Expand Down Expand Up @@ -133,6 +134,7 @@ export const rules: RuleClass[] = [
HTMLAttributeValuesRequireQuotesRule,
HTMLAvoidBothDisabledAndAriaDisabledRule,
HTMLBodyOnlyElementsRule,
HTMLDetailsHasSummaryRule,
HTMLBooleanAttributesNoValueRule,
HTMLHeadOnlyElementsRule,
HTMLIframeHasTitleRule,
Expand Down
69 changes: 69 additions & 0 deletions javascript/packages/linter/src/rules/html-details-has-summary.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { BaseRuleVisitor } from "./rule-utils.js"
import { getTagLocalName, isHTMLElementNode } from "@herb-tools/core"

import { ParserRule } from "../types.js"
import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js"
import type { HTMLElementNode, ParseResult, ParserOptions } from "@herb-tools/core"

class DetailsHasSummaryVisitor extends BaseRuleVisitor {
visitHTMLElementNode(node: HTMLElementNode): void {
this.checkDetailsElement(node)
super.visitHTMLElementNode(node)
}

private checkDetailsElement(node: HTMLElementNode): void {
const tagName = getTagLocalName(node)

if (tagName !== "details") {
return
}

if (!this.hasDirectSummaryChild(node)) {
this.addOffense(
"`<details>` element must have a direct `<summary>` child element.",
node.location,
)
}
}

private hasDirectSummaryChild(node: HTMLElementNode): boolean {
if (!node.body || node.body.length === 0) {
return false
}

for (const child of node.body) {
if (isHTMLElementNode(child)) {
const childTagName = getTagLocalName(child)

if (childTagName === "summary") {
return true
}
}
}

return false
}
}

export class HTMLDetailsHasSummaryRule extends ParserRule {
static ruleName = "html-details-has-summary"

get defaultConfig(): FullRuleConfig {
return {
enabled: true,
severity: "warning"
}
}

get parserOptions(): Partial<ParserOptions> {
return {
action_view_helpers: true,
}
}

check(result: ParseResult, context?: Partial<LintContext>): UnboundLintOffense[] {
const visitor = new DetailsHasSummaryVisitor(this.ruleName, context)
visitor.visit(result.value)
return visitor.offenses
}
}
1 change: 1 addition & 0 deletions javascript/packages/linter/src/rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export * from "./html-attribute-equals-spacing.js"
export * from "./html-attribute-values-require-quotes.js"
export * from "./html-avoid-both-disabled-and-aria-disabled.js"
export * from "./html-body-only-elements.js"
export * from "./html-details-has-summary.js"
export * from "./html-boolean-attributes-no-value.js"
export * from "./html-head-only-elements.js"
export * from "./html-iframe-has-title.js"
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

142 changes: 142 additions & 0 deletions javascript/packages/linter/test/rules/html-details-has-summary.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
import { describe, test } from "vitest"
import { HTMLDetailsHasSummaryRule } from "../../src/rules/html-details-has-summary.js"
import { createLinterTest } from "../helpers/linter-test-helper.js"

const { expectNoOffenses, expectWarning, assertOffenses } = createLinterTest(HTMLDetailsHasSummaryRule)

describe("html-details-has-summary", () => {
test("passes when details has a direct summary child", () => {
expectNoOffenses('<details><summary>Expand me!</summary><p>Content</p></details>')
})

test("passes when summary is not the first child", () => {
expectNoOffenses('<details><p>Surprise text!</p><summary>Expand me!</summary></details>')
})

test("fails when details has no summary", () => {
expectWarning("`<details>` element must have a direct `<summary>` child element.")

assertOffenses("<details>I don't have a summary element</details>")
})

test("fails when summary is not a direct child of details", () => {
expectWarning("`<details>` element must have a direct `<summary>` child element.")

assertOffenses("<details><div><summary>Expand me!</summary></div></details>")
})

test("ignores non-details elements", () => {
expectNoOffenses('<div><p>No summary needed here</p></div>')
})

test("handles multiple details elements independently", () => {
expectWarning("`<details>` element must have a direct `<summary>` child element.")

assertOffenses('<details><summary>OK</summary></details><details><p>Missing summary</p></details>')
})

test("passes for details with summary and other content", () => {
expectNoOffenses('<details><summary>Expand me!</summary><button>Surprise button!</button></details>')
})

test("fails for empty details element", () => {
expectWarning("`<details>` element must have a direct `<summary>` child element.")

assertOffenses('<details></details>')
})

test("fails when details is rendered with tag.details and no summary", () => {
expectWarning("`<details>` element must have a direct `<summary>` child element.")

assertOffenses(`
<details>
<%= tag.div "Not a summary" %>
</details>
`)
})

test("fails when details has content_tag but not a summary", () => {
expectWarning("`<details>` element must have a direct `<summary>` child element.")

assertOffenses(`
<details>
<%= content_tag(:div) { "Not a summary" } %>
</details>
`)
})

test("fails when details only contains ERB output", () => {
expectWarning("`<details>` element must have a direct `<summary>` child element.")

assertOffenses(`
<details>
<%= tag.p "Some content" %>
</details>
`)
})

test("fails when details only contains content_tag with do block", () => {
expectWarning("`<details>` element must have a direct `<summary>` child element.")

assertOffenses(`
<details>
<%= content_tag(:div) do %>
Not a summary
<% end %>
</details>
`)
})

test("fails when tag.details has no summary", () => {
expectWarning("`<details>` element must have a direct `<summary>` child element.")

assertOffenses(`
<%= tag.details do %>
Some content without summary
<% end %>
`)
})

test("fails when content_tag(:details) has no summary", () => {
expectWarning("`<details>` element must have a direct `<summary>` child element.")

assertOffenses(`<%= content_tag(:details) { "Some content" } %>`)
})

test("fails when content_tag(:details) with do block has no summary", () => {
expectWarning("`<details>` element must have a direct `<summary>` child element.")

assertOffenses(`
<%= content_tag(:details) do %>
Some content without summary
<% end %>
`)
})

test("passes when content_tag(:details) has content_tag(:summary)", () => {
expectNoOffenses(`
<%= content_tag(:details) do %>
<%= content_tag(:summary) { "Expand me!" } %>
Some content
<% end %>
`)
})

test("passes when tag.details has a summary child", () => {
expectNoOffenses(`
<%= tag.details do %>
<summary>Expand me!</summary>
Some content
<% end %>
`)
})

test("passes when tag.details has tag.summary child", () => {
expectNoOffenses(`
<%= tag.details do %>
<%= tag.summary "Expand me!" %>
Some content
<% end %>
`)
})
})
Loading