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.

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): eslint-plugin-jsx-a11y click-events-have-key-events #1976

Merged
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
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ mod jsx_a11y {
pub mod aria_role;
pub mod aria_unsupported_elements;
pub mod autocomplete_valid;
pub mod click_events_have_key_events;
pub mod heading_has_content;
pub mod html_has_lang;
pub mod iframe_has_title;
Expand Down Expand Up @@ -508,6 +509,7 @@ oxc_macros::declare_all_lint_rules! {
jsx_a11y::anchor_is_valid,
jsx_a11y::aria_props,
jsx_a11y::aria_unsupported_elements,
jsx_a11y::click_events_have_key_events,
jsx_a11y::heading_has_content,
jsx_a11y::html_has_lang,
jsx_a11y::lang,
Expand Down
150 changes: 150 additions & 0 deletions crates/oxc_linter/src/rules/jsx_a11y/click_events_have_key_events.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
use oxc_ast::AstKind;
use oxc_diagnostics::{
miette::{self, Diagnostic},
thiserror::Error,
};
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;

use crate::{
context::LintContext,
globals::HTML_TAG,
rule::Rule,
utils::{
get_element_type, has_jsx_prop, is_hidden_from_screen_reader, is_interactive_element,
is_presentation_role,
},
AstNode,
};

#[derive(Debug, Error, Diagnostic)]
#[error("eslint-plugin-jsx-a11y(click-events-have-key-events): Enforce a clickable non-interactive element has at least one keyboard event listener.")]
#[diagnostic(severity(warning), help("Visible, non-interactive elements with click handlers must have one of keyup, keydown, or keypress listener."))]
struct ClickEventsHaveKeyEventsDiagnostic(#[label] pub Span);

#[derive(Debug, Default, Clone)]
pub struct ClickEventsHaveKeyEvents;

declare_oxc_lint!(
/// ### What it does
///
/// Enforce onClick is accompanied by at least one of the following: onKeyUp, onKeyDown, onKeyPress.
///
/// ### Why is this bad?
///
/// Coding for the keyboard is important for users with physical disabilities who cannot use a mouse, AT compatibility, and screenreader users.
/// This does not apply for interactive or hidden elements.
///
/// ### Example
/// ```jsx
/// // Good
/// <div onClick={() => void 0} onKeyDown={() => void 0} />
///
/// // Bad
/// <div onClick={() => void 0} />
/// ```
ClickEventsHaveKeyEvents,
correctness
);

impl Rule for ClickEventsHaveKeyEvents {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
let AstKind::JSXOpeningElement(jsx_opening_el) = node.kind() else {
return;
};

if has_jsx_prop(jsx_opening_el, "onClick").is_none() {
return;
};

// Check only native DOM elements or custom component via settings
let Some(element_type) = get_element_type(ctx, jsx_opening_el) else {
return;
};
if !HTML_TAG.contains(&element_type) {
return;
};

if is_hidden_from_screen_reader(jsx_opening_el) || is_presentation_role(jsx_opening_el) {
return;
}

if is_interactive_element(&element_type, jsx_opening_el) {
return;
}

if ["onKeyUp", "onKeyDown", "onKeyPress"]
.iter()
.find_map(|prop| has_jsx_prop(jsx_opening_el, prop))
.is_some()
{
return;
}

ctx.diagnostic(ClickEventsHaveKeyEventsDiagnostic(jsx_opening_el.span));
}
}

#[test]
fn test() {
use crate::tester::Tester;

let pass = vec![
(r"<div onClick={() => void 0} onKeyDown={foo}/>;", None, None, None),
(r"<div onClick={() => void 0} onKeyUp={foo} />;", None, None, None),
(r"<div onClick={() => void 0} onKeyPress={foo}/>;", None, None, None),
(r"<div onClick={() => void 0} onKeyDown={foo} onKeyUp={bar} />;", None, None, None),
(r"<div onClick={() => void 0} onKeyDown={foo} {...props} />;", None, None, None),
(r#"<div className="foo" />;"#, None, None, None),
(r"<div onClick={() => void 0} aria-hidden />;", None, None, None),
(r"<div onClick={() => void 0} aria-hidden={true} />;", None, None, None),
(r"<div onClick={() => void 0} aria-hidden={false} onKeyDown={foo} />;", None, None, None),
(
r"<div onClick={() => void 0} onKeyDown={foo} aria-hidden={undefined} />;",
None,
None,
None,
),
(r#"<input type="text" onClick={() => void 0} />"#, None, None, None),
(r"<input onClick={() => void 0} />", None, None, None),
(r#"<button onClick={() => void 0} className="foo" />"#, None, None, None),
(r#"<select onClick={() => void 0} className="foo" />"#, None, None, None),
(r#"<textarea onClick={() => void 0} className="foo" />"#, None, None, None),
(r#"<a onClick={() => void 0} href="http://x.y.z" />"#, None, None, None),
(r#"<a onClick={() => void 0} href="http://x.y.z" tabIndex="0" />"#, None, None, None),
(r#"<input onClick={() => void 0} type="hidden" />;"#, None, None, None),
(r#"<div onClick={() => void 0} role="presentation" />;"#, None, None, None),
(r#"<div onClick={() => void 0} role="none" />;"#, None, None, None),
(r"<TestComponent onClick={doFoo} />", None, None, None),
(r"<Button onClick={doFoo} />", None, None, None),
(r"<Footer onClick={doFoo} />", None, None, None),
];

let fail = vec![
(r"<div onClick={() => void 0} />;", None, None, None),
(r"<div onClick={() => void 0} role={undefined} />;", None, None, None),
(r"<div onClick={() => void 0} {...props} />;", None, None, None),
(r"<section onClick={() => void 0} />;", None, None, None),
(r"<main onClick={() => void 0} />;", None, None, None),
(r"<article onClick={() => void 0} />;", None, None, None),
(r"<header onClick={() => void 0} />;", None, None, None),
(r"<footer onClick={() => void 0} />;", None, None, None),
(r"<div onClick={() => void 0} aria-hidden={false} />;", None, None, None),
(r"<a onClick={() => void 0} />", None, None, None),
(r#"<a tabIndex="0" onClick={() => void 0} />"#, None, None, None),
(
r"<Footer onClick={doFoo} />",
None,
Some(serde_json::json!({
"jsx-a11y": {
"components": {
"Footer": "footer",
}
}
})),
None,
),
];

Tester::new(ClickEventsHaveKeyEvents::NAME, pass, fail).test_and_snapshot();
}
89 changes: 89 additions & 0 deletions crates/oxc_linter/src/snapshots/click_events_have_key_events.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
---
source: crates/oxc_linter/src/tester.rs
expression: click_events_have_key_events
---
⚠ eslint-plugin-jsx-a11y(click-events-have-key-events): Enforce a clickable non-interactive element has at least one keyboard event listener.
╭─[click_events_have_key_events.tsx:1:1]
1 │ <div onClick={() => void 0} />;
· ──────────────────────────────
╰────
help: Visible, non-interactive elements with click handlers must have one of keyup, keydown, or keypress listener.

⚠ eslint-plugin-jsx-a11y(click-events-have-key-events): Enforce a clickable non-interactive element has at least one keyboard event listener.
╭─[click_events_have_key_events.tsx:1:1]
1 │ <div onClick={() => void 0} role={undefined} />;
· ───────────────────────────────────────────────
╰────
help: Visible, non-interactive elements with click handlers must have one of keyup, keydown, or keypress listener.

⚠ eslint-plugin-jsx-a11y(click-events-have-key-events): Enforce a clickable non-interactive element has at least one keyboard event listener.
╭─[click_events_have_key_events.tsx:1:1]
1 │ <div onClick={() => void 0} {...props} />;
· ─────────────────────────────────────────
╰────
help: Visible, non-interactive elements with click handlers must have one of keyup, keydown, or keypress listener.

⚠ eslint-plugin-jsx-a11y(click-events-have-key-events): Enforce a clickable non-interactive element has at least one keyboard event listener.
╭─[click_events_have_key_events.tsx:1:1]
1 │ <section onClick={() => void 0} />;
· ──────────────────────────────────
╰────
help: Visible, non-interactive elements with click handlers must have one of keyup, keydown, or keypress listener.

⚠ eslint-plugin-jsx-a11y(click-events-have-key-events): Enforce a clickable non-interactive element has at least one keyboard event listener.
╭─[click_events_have_key_events.tsx:1:1]
1 │ <main onClick={() => void 0} />;
· ───────────────────────────────
╰────
help: Visible, non-interactive elements with click handlers must have one of keyup, keydown, or keypress listener.

⚠ eslint-plugin-jsx-a11y(click-events-have-key-events): Enforce a clickable non-interactive element has at least one keyboard event listener.
╭─[click_events_have_key_events.tsx:1:1]
1 │ <article onClick={() => void 0} />;
· ──────────────────────────────────
╰────
help: Visible, non-interactive elements with click handlers must have one of keyup, keydown, or keypress listener.

⚠ eslint-plugin-jsx-a11y(click-events-have-key-events): Enforce a clickable non-interactive element has at least one keyboard event listener.
╭─[click_events_have_key_events.tsx:1:1]
1 │ <header onClick={() => void 0} />;
· ─────────────────────────────────
╰────
help: Visible, non-interactive elements with click handlers must have one of keyup, keydown, or keypress listener.

⚠ eslint-plugin-jsx-a11y(click-events-have-key-events): Enforce a clickable non-interactive element has at least one keyboard event listener.
╭─[click_events_have_key_events.tsx:1:1]
1 │ <footer onClick={() => void 0} />;
· ─────────────────────────────────
╰────
help: Visible, non-interactive elements with click handlers must have one of keyup, keydown, or keypress listener.

⚠ eslint-plugin-jsx-a11y(click-events-have-key-events): Enforce a clickable non-interactive element has at least one keyboard event listener.
╭─[click_events_have_key_events.tsx:1:1]
1 │ <div onClick={() => void 0} aria-hidden={false} />;
· ──────────────────────────────────────────────────
╰────
help: Visible, non-interactive elements with click handlers must have one of keyup, keydown, or keypress listener.

⚠ eslint-plugin-jsx-a11y(click-events-have-key-events): Enforce a clickable non-interactive element has at least one keyboard event listener.
╭─[click_events_have_key_events.tsx:1:1]
1 │ <a onClick={() => void 0} />
· ────────────────────────────
╰────
help: Visible, non-interactive elements with click handlers must have one of keyup, keydown, or keypress listener.

⚠ eslint-plugin-jsx-a11y(click-events-have-key-events): Enforce a clickable non-interactive element has at least one keyboard event listener.
╭─[click_events_have_key_events.tsx:1:1]
1 │ <a tabIndex="0" onClick={() => void 0} />
· ─────────────────────────────────────────
╰────
help: Visible, non-interactive elements with click handlers must have one of keyup, keydown, or keypress listener.

⚠ eslint-plugin-jsx-a11y(click-events-have-key-events): Enforce a clickable non-interactive element has at least one keyboard event listener.
╭─[click_events_have_key_events.tsx:1:1]
1 │ <Footer onClick={doFoo} />
· ──────────────────────────
╰────
help: Visible, non-interactive elements with click handlers must have one of keyup, keydown, or keypress listener.


42 changes: 42 additions & 0 deletions crates/oxc_linter/src/utils/react.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,48 @@ pub fn object_has_accessible_child(node: &JSXElement<'_>) -> bool {
|| has_jsx_prop_lowercase(&node.opening_element, "children").is_some()
}

pub fn is_presentation_role(jsx_opening_el: &JSXOpeningElement) -> bool {
let Some(role) = has_jsx_prop(jsx_opening_el, "role") else {
return false;
};
let Some("presentation" | "none") = get_string_literal_prop_value(role) else {
return false;
};

true
}

// TODO: Should re-implement
// https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/4c7e7815c12a797587bb8e3cdced7f3003848964/src/util/isInteractiveElement.js
// with `oxc-project/aria-query` which is currently W.I.P.
//
// Until then, use simplified version by https://html.spec.whatwg.org/multipage/dom.html#interactive-content
pub fn is_interactive_element(element_type: &str, jsx_opening_el: &JSXOpeningElement) -> bool {
leaysgur marked this conversation as resolved.
Show resolved Hide resolved
// Interactive contents are...
// - button, details, embed, iframe, label, select, textarea
// - input (if the `type` attribute is not in the Hidden state)
// - a (if the `href` attribute is present)
// - audio, video (if the `controls` attribute is present)
// - img (if the `usemap` attribute is present)
match element_type {
"button" | "details" | "embed" | "iframe" | "label" | "select" | "textarea" => true,
"input" => {
if let Some(input_type) = has_jsx_prop(jsx_opening_el, "type") {
if get_string_literal_prop_value(input_type)
.is_some_and(|val| val.to_uppercase() == "HIDDEN")
{
return false;
}
}
true
}
"a" => has_jsx_prop(jsx_opening_el, "href").is_some(),
"audio" | "video" => has_jsx_prop(jsx_opening_el, "controls").is_some(),
"img" => has_jsx_prop(jsx_opening_el, "usemap").is_some(),
_ => false,
}
}

const PRAGMA: &str = "React";
const CREATE_CLASS: &str = "createReactClass";

Expand Down