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

fix(lint): fix false-positives for aria-label and report cases where the role is implicit in noSvgWithoutTitle #788

Merged
merged 6 commits into from
Nov 19, 2023
Merged

fix(lint): fix false-positives for aria-label and report cases where the role is implicit in noSvgWithoutTitle #788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a2e098d
fix(lint): false-positives for aria-label in no-svg-without-title and…
unvalley Nov 19, 2023
c4b37ef
docs: update lint example
unvalley Nov 19, 2023
07b4072
chore: update the CHANGELOG.md
unvalley Nov 19, 2023
06b8160
chore: fix waste space and codegen for changelog
unvalley Nov 19, 2023
3c23440
fix: do not report to specific role other than img
unvalley Nov 19, 2023
c2c94ed
chore: codegen CHANGELOG.md
unvalley Nov 19, 2023
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom

- Fix [#727](https://github.com/biomejs/biome/issues/727). [noInferrableTypes](https://biomejs.dev/linter/rules/no-inferrable-types) now correctly keeps type annotations when the initialization expression is `null`. Contributed by @Conaclos

- Fix [#784](https://github.com/biomejs/biome/issues/784), [noSvgWithoutTitle](https://biomejs.dev/linter/rules/no-svg-without-title) fixes false-positives to `aria-label` and reports svg's role attribute is implicit. Contributed by @unvalley

### Parser

### VSCode
Expand Down
38 changes: 23 additions & 15 deletions crates/biome_js_analyze/src/analyzers/a11y/no_svg_without_title.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ declare_rule! {
/// ```
///
/// ```js
/// <svg role="img" aria-labelledby="title">
/// <span id="title">Pass</span>
/// </svg>
/// ```
///
/// ```js
/// <svg role="img" aria-label="title">
/// <span id="title">Pass</span>
/// </svg>
Expand Down Expand Up @@ -108,29 +114,31 @@ impl Rule for NoSvgWithoutTitle {
};

let role_attribute_value = role_attribute.initializer()?.value().ok()?;
let Some(text) = role_attribute_value
let Some(role_attribute_text) = role_attribute_value
.as_jsx_string()?
.inner_string_text()
.ok()
else {
return Some(());
};

if text.to_lowercase() == "img" {
let [aria_label, aria_labelledby] = node
.attributes()
.find_by_names(["aria-label", "aria-labelledby"]);

let jsx_child_list = jsx_element.children();
let is_valid = is_valid_attribute_value(aria_label, &jsx_child_list).unwrap_or(false)
|| is_valid_attribute_value(aria_labelledby, &jsx_child_list).unwrap_or(false);

if !is_valid {
return Some(());
match role_attribute_text.to_lowercase().as_str() {
"img" => {
let [aria_label, aria_labelledby] = node
.attributes()
.find_by_names(["aria-label", "aria-labelledby"]);
let is_valid_a11y_attribute = aria_label.is_some()
|| is_valid_attribute_value(aria_labelledby, &jsx_element.children())
.unwrap_or(false);
if is_valid_a11y_attribute {
return None;
}
Some(())
}
};

None
// if role attribute is empty, the svg element should have title element
"" => Some(()),
_ => None,
}
}

fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {
Expand Down
6 changes: 3 additions & 3 deletions crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/invalid.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@
<title></title>
<circle />
</svg>
<svg role="img" aria-label="title">
<span id="">foo</span>
</svg>
<svg role="img" title="title">
<span id="">foo</span>
</svg>
<svg role="img" aria-labelledby="title">
<span id="">foo</span>
</svg>
<svg role="">
<span>implicit role</span>
</svg>
</>;
20 changes: 10 additions & 10 deletions crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/invalid.jsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ expression: invalid.jsx
<title></title>
<circle />
</svg>
<svg role="img" aria-label="title">
<span id="">foo</span>
</svg>
<svg role="img" title="title">
<span id="">foo</span>
</svg>
<svg role="img" aria-labelledby="title">
<span id="">foo</span>
</svg>
<svg role="">
<span>implicit role</span>
</svg>
</>;

```
Expand Down Expand Up @@ -64,8 +64,8 @@ invalid.jsx:7:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━━

5 │ <circle />
6 │ </svg>
> 7 │ <svg role="img" aria-label="title">
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 7 │ <svg role="img" title="title">
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
8 │ <span id="">foo</span>
9 │ </svg>

Expand All @@ -81,8 +81,8 @@ invalid.jsx:10:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━

8 │ <span id="">foo</span>
9 │ </svg>
> 10 │ <svg role="img" title="title">
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 10 │ <svg role="img" aria-labelledby="title">
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
11 │ <span id="">foo</span>
12 │ </svg>

Expand All @@ -98,9 +98,9 @@ invalid.jsx:13:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━

11 │ <span id="">foo</span>
12 │ </svg>
> 13 │ <svg role="img" aria-labelledby="title">
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
14 │ <span id="">foo</span>
> 13 │ <svg role="">
│ ^^^^^^^^^^^^^
14 │ <span>implicit role</span>
15 │ </svg>

i For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
Expand Down
9 changes: 8 additions & 1 deletion crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/valid.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,19 @@
<title id="title">Pass</title>
</svg>
<svg role="img" aria-label="title">
<span id="title">Pass</span>
<span>Pass</span>
</svg>
<svg role="img" aria-label="title">
<span id="sample">Pass</span>
</svg>
<svg role="img" aria-labelledby="title">
<title id="title">Pass</title>
</svg>
<svg role="img" aria-labelledby="title">
<span id="title">Pass</span>
</svg>
<svg role="">
<title>implicit role</title>
<span>Pass</span>
</svg>
</>;
9 changes: 8 additions & 1 deletion crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/valid.jsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,21 @@ expression: valid.jsx
<title id="title">Pass</title>
</svg>
<svg role="img" aria-label="title">
<span id="title">Pass</span>
<span>Pass</span>
</svg>
<svg role="img" aria-label="title">
<span id="sample">Pass</span>
</svg>
<svg role="img" aria-labelledby="title">
<title id="title">Pass</title>
</svg>
<svg role="img" aria-labelledby="title">
<span id="title">Pass</span>
</svg>
<svg role="">
<title>implicit role</title>
<span>Pass</span>
</svg>
</>;

```
Expand Down
2 changes: 2 additions & 0 deletions website/src/content/docs/internals/changelog.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom

- Fix [#727](https://github.com/biomejs/biome/issues/727). [noInferrableTypes](https://biomejs.dev/linter/rules/no-inferrable-types) now correctly keeps type annotations when the initialization expression is `null`. Contributed by @Conaclos

- Fix [#784](https://github.com/biomejs/biome/issues/784), [noSvgWithoutTitle](https://biomejs.dev/linter/rules/no-svg-without-title) fixes false-positives to `aria-label` and reports svg's role attribute is implicit. Contributed by @unvalley

### Parser

### VSCode
Expand Down
6 changes: 6 additions & 0 deletions website/src/content/docs/linter/rules/no-svg-without-title.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,12 @@ To make svg accessible, the following methods are available:
</svg>
```

```jsx
<svg role="img" aria-labelledby="title">
<span id="title">Pass</span>
</svg>
```

```jsx
<svg role="img" aria-label="title">
<span id="title">Pass</span>
Expand Down