-
-
Notifications
You must be signed in to change notification settings - Fork 474
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.
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
feat(linter): add noNoninteractiveElementInteractions
rule
#4358
base: main
Are you sure you want to change the base?
feat(linter): add noNoninteractiveElementInteractions
rule
#4358
Conversation
CodSpeed Performance ReportMerging #4358 will not alter performanceComparing Summary
|
02a8329
to
5d53042
Compare
|
||
/// List of `widget` role | ||
/// https://www.w3.org/TR/wai-aria-1.2/#widget | ||
const INTERACTIVE_ROLES: &[&str] = &[ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be coming from our biome_aria
crate. Preferably generated from the spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your review!
I’ve updated the code to use biome_aria
as suggested (df9cbe0).
However, I noticed a difference in the test results after this change.
For example, the radiogroup
role, which inherits from select
and should be interactive, is now being treated as non-interactive.
54 │+# Diagnostics
55 │+```
56 │+valid.jsx:49:5 lint/nursery/noNoninteractiveElementInteractions ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
57 │+
58 │+ ! Non-interactive element should not have event handler.
59 │+
60 │+ > 49 │ <div role="radiogroup" onClick={() => { }} />
61 │+ │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
62 │+ 50 │ </>
63 │+
64 │+ i Consider replace semantically interactive element like <button/> or <a href/>.
65 │+
66 │+
According to WAI-ARIA 1.1 and 1.2, radiogroup inherits from select
, but role.rs indicates inheritance from range
.
biome/crates/biome_aria/src/roles.rs
Lines 487 to 493 in 19abdcf
define_role! { | |
/// https://www.w3.org/TR/wai-aria-1.1/#radiogroup | |
RadiogroupRole { | |
PROPS: [("aria-readonly", false), ("aria-required", false)], | |
ROLES: ["range"], | |
} | |
} |
https://www.w3.org/TR/wai-aria-1.1/#radiogroup
https://www.w3.org/TR/wai-aria-1.2/#radiogroup
I am not familiar with accessibility, so I apologize if there is any misunderstanding.
Could you please advise me on how to proceed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's probably a bug, right? You could try to add the missing roles and see if that fixes the issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I manually added the widget
role and the radiogroup
is no longer reported.
--- a/crates/biome_aria/src/roles.rs
+++ b/crates/biome_aria/src/roles.rs
@@ -488,7 +488,7 @@ define_role! {
/// https://www.w3.org/TR/wai-aria-1.1/#radiogroup
RadiogroupRole {
PROPS: [("aria-readonly", false), ("aria-required", false)],
- ROLES: ["range"],
+ ROLES: ["range","select","composite","widget","group"],
}
}
Expression: valid.jsx
───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
-old snapshot
+new results
────────────┬──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
44 44 │ <div role="textbox" onClick={() => { }} />
45 45 │
46 46 │ {/* Presentation role */}
47 47 │ <div role="presentation" onClick={() => { }} />
48 │+
49 │+
50 │+ <div role="radiogroup" onClick={() => { }} />
48 51 │ </>
49 52 │ ```
If it's not my implementation error, I think it's a bug.
Summary
Implement jsx-a11y/no-noninteractive-element-interactions
Close #3936
Test Plan