Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: let aria-label be valid for useHeadingContent. #3767

Merged
merged 4 commits into from
Sep 4, 2024
Merged

fix: let aria-label be valid for useHeadingContent. #3767

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
27c0837
fix: let aria-label be valid for useHeadingContent.
anthonyshew Sep 3, 2024
32a6ac4
Fix changelog.
anthonyshew Sep 3, 2024
1f3f381
Fix for edge case.
anthonyshew Sep 3, 2024
d9eb234
Update crates/biome_js_analyze/src/lint/a11y/use_heading_content.rs
anthonyshew Sep 3, 2024
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
6 changes: 4 additions & 2 deletions CHANGELOG.md
View file
Open in desktop
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just taking the opportunity to remove whitespaces I left in my other fix lines. 😄

Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,14 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b

- Migrating from Prettier or ESLint no longer overwrite the `overrides` field from the configuration ([#3544](https://github.com/biomejs/biome/issues/3544)). Contributed by @Conaclos

- Fix JSX expressions for `noAriaHiddenOnFocusable` ([#3708](https://github.com/biomejs/biome/pull/3708)) . Contributed by @anthonyshew
- Fix JSX expressions for `noAriaHiddenOnFocusable` ([#3708](https://github.com/biomejs/biome/pull/3708)). Contributed by @anthonyshew

- Fix edge case for `<canvas>` elements that use `role="img"` ([#3728](https://github.com/biomejs/biome/pull/3728)) . Contributed by @anthonyshew
- Fix edge case for `<canvas>` elements that use `role="img"` ([#3728](https://github.com/biomejs/biome/pull/3728)). Contributed by @anthonyshew

- Fix [#3633](https://github.com/biomejs/biome/issues/3633), where diagnostics where incorrectly printed if the code has errors. Contributed by @ematipico

- Allow `aria-label` on heading to prevent `useHeadingContent` diagnostic ([#3767](https://github.com/biomejs/biome/pull/3767)). Contributed by @anthonyshew

### Configuration

- Add support for loading configuration from `.editorconfig` files ([#1724](https://github.com/biomejs/biome/issues/1724)).
Expand Down
14 changes: 14 additions & 0 deletions crates/biome_js_analyze/src/lint/a11y/use_heading_content.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ declare_lint_rule! {
/// ```
///
/// ```jsx,expect_diagnostic
/// <h1 aria-label="Screen reader content" aria-hidden>invisible content</h1>
/// ```
///
///
/// ```jsx,expect_diagnostic
/// <h1></h1>
/// ```
///
Expand All @@ -35,6 +40,10 @@ declare_lint_rule! {
/// ```
///
/// ```jsx
/// <h1 aria-label="Screen reader content"><div aria-hidden="true">invisible content</div></h1>
/// ```
///
/// ```jsx
/// <h1 dangerouslySetInnerHTML={{ __html: "heading" }} />
/// ```
///
Expand Down Expand Up @@ -72,6 +81,11 @@ impl Rule for UseHeadingContent {
return Some(());
}

// When node has aria-label (and doesn't have `aria-hidden`), the label will be read by screen readers
anthonyshew marked this conversation as resolved.
Show resolved Hide resolved
if node.has_truthy_attribute("aria-label") {
return None;
}

if has_valid_heading_content(node) {
return None;
}
Expand Down
1 change: 1 addition & 0 deletions crates/biome_js_analyze/tests/specs/a11y/useHeadingContent/invalid.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
</h1>
<h1 aria-hidden="true">content</h1>
<h1 aria-hidden="true" dangerouslySetInnerHTML={{ __html: "heading" }} />
<h1 aria-label="Screen reader content" aria-hidden>invisible content</h1>
<h1>{undefined}</h1>
<h1 children />
<h1 children={""} />
Expand Down
91 changes: 54 additions & 37 deletions crates/biome_js_analyze/tests/specs/a11y/useHeadingContent/invalid.jsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
assertion_line: 86
expression: invalid.jsx
---
# Input
Expand All @@ -15,6 +16,7 @@ expression: invalid.jsx
</h1>
<h1 aria-hidden="true">content</h1>
<h1 aria-hidden="true" dangerouslySetInnerHTML={{ __html: "heading" }} />
<h1 aria-label="Screen reader content" aria-hidden>invisible content</h1>
<h1>{undefined}</h1>
<h1 children />
<h1 children={""} />
Expand Down Expand Up @@ -109,7 +111,7 @@ invalid.jsx:10:2 lint/a11y/useHeadingContent ━━━━━━━━━━━
> 10 │ <h1 aria-hidden="true">content</h1>
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
11 │ <h1 aria-hidden="true" dangerouslySetInnerHTML={{ __html: "heading" }} />
12 │ <h1>{undefined}</h1>
12 │ <h1 aria-label="Screen reader content" aria-hidden>invisible content</h1>

i All headings on a page should have content that is accessible to screen readers.

Expand All @@ -125,25 +127,25 @@ invalid.jsx:11:2 lint/a11y/useHeadingContent ━━━━━━━━━━━
10 │ <h1 aria-hidden="true">content</h1>
> 11 │ <h1 aria-hidden="true" dangerouslySetInnerHTML={{ __html: "heading" }} />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
12 │ <h1>{undefined}</h1>
13 │ <h1 children />
12 │ <h1 aria-label="Screen reader content" aria-hidden>invisible content</h1>
13 │ <h1>{undefined}</h1>

i All headings on a page should have content that is accessible to screen readers.


```

```
invalid.jsx:12:2 lint/a11y/useHeadingContent ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.jsx:12:3 lint/a11y/useHeadingContent ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Provide screen reader accessible content when using heading elements.

10 │ <h1 aria-hidden="true">content</h1>
11 │ <h1 aria-hidden="true" dangerouslySetInnerHTML={{ __html: "heading" }} />
> 12 │ <h1>{undefined}</h1>
│ ^^^^^^^^^^^^^^^^^^^^
13 │ <h1 children />
14 │ <h1 children={""} />
> 12 │ <h1 aria-label="Screen reader content" aria-hidden>invisible content</h1>
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
13 │ <h1>{undefined}</h1>
14 │ <h1 children />

i All headings on a page should have content that is accessible to screen readers.

Expand All @@ -156,11 +158,11 @@ invalid.jsx:13:2 lint/a11y/useHeadingContent ━━━━━━━━━━━
! Provide screen reader accessible content when using heading elements.

11 │ <h1 aria-hidden="true" dangerouslySetInnerHTML={{ __html: "heading" }} />
12 │ <h1>{undefined}</h1>
> 13 │ <h1 children />
│ ^^^^^^^^^^^^^^^
14 │ <h1 children={""} />
15 │ <h1 children={null} />
12 │ <h1 aria-label="Screen reader content" aria-hidden>invisible content</h1>
> 13 │ <h1>{undefined}</h1>
│ ^^^^^^^^^^^^^^^^^^^^
14 │ <h1 children />
15 │ <h1 children={""} />

i All headings on a page should have content that is accessible to screen readers.

Expand All @@ -172,12 +174,12 @@ invalid.jsx:14:2 lint/a11y/useHeadingContent ━━━━━━━━━━━

! Provide screen reader accessible content when using heading elements.

12 │ <h1>{undefined}</h1>
13 │ <h1 children />
> 14 │ <h1 children={""} />
│ ^^^^^^^^^^^^^^^^^^^^
15 │ <h1 children={null} />
16 │ <h1 children={undefined} />
12 │ <h1 aria-label="Screen reader content" aria-hidden>invisible content</h1>
13 │ <h1>{undefined}</h1>
> 14 │ <h1 children />
│ ^^^^^^^^^^^^^^^
15 │ <h1 children={""} />
16 │ <h1 children={null} />

i All headings on a page should have content that is accessible to screen readers.

Expand All @@ -189,12 +191,12 @@ invalid.jsx:15:2 lint/a11y/useHeadingContent ━━━━━━━━━━━

! Provide screen reader accessible content when using heading elements.

13 │ <h1 children />
14 │ <h1 children={""} />
> 15 │ <h1 children={null} />
│ ^^^^^^^^^^^^^^^^^^^^^^
16 │ <h1 children={undefined} />
17 │ <h1 {...props} aria-hidden="true" />
13 │ <h1>{undefined}</h1>
14 │ <h1 children />
> 15 │ <h1 children={""} />
│ ^^^^^^^^^^^^^^^^^^^^
16 │ <h1 children={null} />
17 │ <h1 children={undefined} />

i All headings on a page should have content that is accessible to screen readers.

Expand All @@ -206,12 +208,12 @@ invalid.jsx:16:2 lint/a11y/useHeadingContent ━━━━━━━━━━━

! Provide screen reader accessible content when using heading elements.

14 │ <h1 children={""} />
15 │ <h1 children={null} />
> 16 │ <h1 children={undefined} />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
17 │ <h1 {...props} aria-hidden="true" />
18 │ </>;
14 │ <h1 children />
15 │ <h1 children={""} />
> 16 │ <h1 children={null} />
│ ^^^^^^^^^^^^^^^^^^^^^^
17 │ <h1 children={undefined} />
18 │ <h1 {...props} aria-hidden="true" />

i All headings on a page should have content that is accessible to screen readers.

Expand All @@ -223,16 +225,31 @@ invalid.jsx:17:2 lint/a11y/useHeadingContent ━━━━━━━━━━━

! Provide screen reader accessible content when using heading elements.

15 │ <h1 children={null} />
16 │ <h1 children={undefined} />
> 17 │ <h1 {...props} aria-hidden="true" />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
18 │ </>;
19 │
15 │ <h1 children={""} />
16 │ <h1 children={null} />
> 17 │ <h1 children={undefined} />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
18 │ <h1 {...props} aria-hidden="true" />
19 │ </>;

i All headings on a page should have content that is accessible to screen readers.


```

```
invalid.jsx:18:2 lint/a11y/useHeadingContent ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Provide screen reader accessible content when using heading elements.

16 │ <h1 children={null} />
17 │ <h1 children={undefined} />
> 18 │ <h1 {...props} aria-hidden="true" />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
19 │ </>;
20 │

i All headings on a page should have content that is accessible to screen readers.


```
1 change: 1 addition & 0 deletions crates/biome_js_analyze/tests/specs/a11y/useHeadingContent/valid.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
<div aria-hidden />
visible content
</h1>
<h1 aria-label="Screen reader content"><div aria-hidden="true">invisible content</div></h1>
<h1 dangerouslySetInnerHTML={{ __html: "heading" }}></h1>
<h1 children={children} />
<h1 children={"heading"} />
Expand Down
4 changes: 2 additions & 2 deletions crates/biome_js_analyze/tests/specs/a11y/useHeadingContent/valid.jsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
assertion_line: 86
expression: valid.jsx
---
# Input
Expand Down Expand Up @@ -40,6 +41,7 @@ expression: valid.jsx
<div aria-hidden />
visible content
</h1>
<h1 aria-label="Screen reader content"><div aria-hidden="true">invisible content</div></h1>
<h1 dangerouslySetInnerHTML={{ __html: "heading" }}></h1>
<h1 children={children} />
<h1 children={"heading"} />
Expand All @@ -55,5 +57,3 @@ expression: valid.jsx
</>;

```


Loading