Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_js_analyze): update noNoninteractiveElementToInteractiveRole and noNoninteractiveTabindex #4439

Merged
merged 5 commits into from
May 6, 2023
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 CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ when there are breaking changes.
- Fix false positive diagnostics that [`useCamelCase`](https://docs.rome.tools/lint/rules/usecamelcase/) caused to private class members
- Fix false positive diagnostics that [`useHookAtTopLevel`](https://docs.rome.tools/lint/rules/usehookattoplevel/) caused to arrow functions, export default functions and function expressions.
- Fix false positive diagnostics that [`noHeadeScope`](https://docs.rome.tools/lint/rules/noheaderscope/) caused to custom components
- Fix false negative diagnostics that [`noNoninteractiveElementToInteractiveRole`](https://docs.rome.tools/lint/rules/nononinteractiveelementtointeractiverole/) and [`noNoninteractiveTabindex`](https://docs.rome.tools/lint/rules/nononinteractivetabindex/) caused to non-interactive elements.

### Configuration
### Editors
Expand Down
151 changes: 129 additions & 22 deletions crates/rome_aria/src/roles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ define_role! {
/// https://www.w3.org/TR/wai-aria-1.1/#link
LinkRole {
PROPS: [("aria-expanded", false)],
ROLES: ["command"],
ROLES: ["command", "widget"],
Copy link
Contributor Author

@unvalley unvalley May 5, 2023

Choose a reason for hiding this comment

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

The changes to ROLES in this diff is based on the following link.
https://w3c.github.io/aria/#widget_roles
In order to pass the jsx-a11y test case, it was necessary to refer to this Draft stage document.

CONCEPTS: &[("a", &[]), ("link", &[])],
}
}
Expand All @@ -342,7 +342,7 @@ define_role! {
/// https://www.w3.org/TR/wai-aria-1.1/#listbox
ListBoxRole {
PROPS: [],
ROLES: ["select"],
ROLES: ["select", "widget"],
CONCEPTS: &[("select", &[])],
}
}
Expand All @@ -365,10 +365,11 @@ define_role! {
}

define_role! {
/// https://www.w3.org/TR/wai-aria-1.1/#main
/// https://w3c.github.io/aria/#main
MainRole {
PROPS: [],
ROLES: ["landmark"],
CONCEPTS: &[("main", &[])],
}
}

Expand All @@ -382,25 +383,33 @@ define_role! {

define_role! {
/// https://www.w3.org/TR/wai-aria-1.1/#menu
MenuItem {
MenuRole {
PROPS: [("aria-posinset", false), ("aria-setsize", false)],
ROLES: ["command"],
ROLES: ["select"],
}
}

define_role! {
/// https://www.w3.org/TR/wai-aria-1.1/#menuitem
MenuItemRole {
PROPS: [("aria-posinset", false), ("aria-setsize", false)],
ROLES: ["command", "widget"],
}
}

define_role! {
/// https://www.w3.org/TR/wai-aria-1.1/#menuitemcheckbox
MenuItemCheckboxRole {
PROPS: [("aria-checked", true)],
ROLES: ["checkbox", "menuitem"],
ROLES: ["checkbox", "menuitem", "widget"],
}
}

define_role! {
/// https://www.w3.org/TR/wai-aria-1.1/#menuitemradio
MenuItemRadioRole {
PROPS: [("aria-checked", true)],
ROLES: ["radio", "menuitemcheckbox"],
ROLES: ["radio", "menuitemcheckbox", "widget"],
}
}

Expand All @@ -417,7 +426,7 @@ define_role! {
/// https://www.w3.org/TR/wai-aria-1.1/#progressbar
ProgressBarRole {
PROPS: [("aria-valuenow", true), ("aria-valuemin", true), ("aria-valuemax", true)],
ROLES: ["range"],
ROLES: ["range", "widget"],
}
}
define_role! {
Expand Down Expand Up @@ -466,7 +475,7 @@ define_role! {
("aria-readonly", false),
("aria-required", false),
],
ROLES: ["textbox"],
ROLES: ["textbox", "widget"],
CONCEPTS: &[("input", &[("type", "search")])],
}
}
Expand Down Expand Up @@ -516,7 +525,7 @@ define_role! {
("aria-readonly", false),
("aria-required", false),
],
ROLES: ["input"],
ROLES: ["input", "widget"],
CONCEPTS: &[("textarea", &[]), ("input", &[("type", "search")])],
}
}
Expand Down Expand Up @@ -547,10 +556,11 @@ define_role! {
}

define_role! {
/// https://www.w3.org/TR/wai-aria-1.2/#complementary
/// https://w3c.github.io/aria/#complementary
ComplementaryRole {
PROPS: [],
ROLES: ["landmark"],
CONCEPTS: &[("aside", &[])],
}
}

Expand All @@ -564,11 +574,11 @@ define_role! {
}

define_role! {
/// https://www.w3.org/TR/wai-aria-1.2/#caption
/// https://w3c.github.io/aria/#caption
CaptionRole {
PROPS: [],
ROLES: ["section"],
CONCEPTS: &[("caption", &[]), ("figcaption", &[])],
CONCEPTS: &[("caption", &[]), ("figcaption", &[]), ("legend", &[])],
}
}

Expand Down Expand Up @@ -683,6 +693,7 @@ define_role! {
StatusRole {
PROPS: [],
ROLES: ["section"],
CONCEPTS: &[("output", &[])],
}
}

Expand Down Expand Up @@ -712,6 +723,42 @@ define_role! {
}
}

define_role! {
/// https://w3c.github.io/aria/#mark
MarkRole {
PROPS: [],
ROLES: ["section"],
CONCEPTS: &[("mark", &[])],
}
}

define_role! {
/// https://w3c.github.io/aria/#marquee
MarqueeRole {
PROPS: [],
ROLES: ["section"],
CONCEPTS: &[("marquee", &[])],
}
}

define_role! {
/// https://w3c.github.io/aria/#associationlist
AssociationListRole {
PROPS: [],
ROLES: ["section"],
CONCEPTS: &[("dl", &[])],
}
}

define_role! {
/// https://w3c.github.io/aria/#contentinfo
ContentInfoRole {
PROPS: [],
ROLES: ["landmark"],
CONCEPTS: &[("footer", &[])],
}
}

impl<'a> AriaRoles {
/// These are roles that will contain "concepts".
pub(crate) const ROLE_WITH_CONCEPTS: &'a [&'a str] = &[
Expand Down Expand Up @@ -748,6 +795,16 @@ impl<'a> AriaRoles {
"term",
"textbox",
"generic",
"caption",
"main",
"time",
"p",
"aside",
"blockquote",
"associationlist",
"status",
"contentinfo",
"region",
];

/// It returns the metadata of a role, if it exits.
Expand Down Expand Up @@ -801,7 +858,8 @@ impl<'a> AriaRoles {
"log" => &LogRole as &dyn AriaRoleDefinition,
"main" => &MainRole as &dyn AriaRoleDefinition,
"menubar" => &MenubarRole as &dyn AriaRoleDefinition,
"menu" => &MenuItem as &dyn AriaRoleDefinition,
"menu" => &MenuRole as &dyn AriaRoleDefinition,
"menuitem" => &MenuItemRole as &dyn AriaRoleDefinition,
"menuitemcheckbox" => &MenuItemCheckboxRole as &dyn AriaRoleDefinition,
"menuitemradio" => &MenuItemRadioRole as &dyn AriaRoleDefinition,
"navigation" => &NavigationRole as &dyn AriaRoleDefinition,
Expand Down Expand Up @@ -962,7 +1020,11 @@ impl<'a> AriaRoles {
}

/// Given the name of element, the function tells whether it's interactive
pub fn is_not_interactive_element(&self, element_name: &str) -> bool {
pub fn is_not_interactive_element(
&self,
element_name: &str,
attributes: Option<HashMap<String, Vec<String>>>,
) -> bool {
// <header> elements do not technically have semantics, unless the
// element is a direct descendant of <body>, and this crate cannot
// reliably test that.
Expand All @@ -971,6 +1033,26 @@ impl<'a> AriaRoles {
if element_name == "header" {
return false;
}

let elements_no_concept_info = [
"body", "br", "details", "dir", "frame", "iframe", "label", "mark", "marquee", "menu",
"meter", "optgroup", "pre", "progress", "ruby",
];
if elements_no_concept_info.contains(&element_name) {
return true;
}

// <input type="hidden"> is not interactive.
// `type=hidden` is not represented as concept information.
if element_name == "input"
&& attributes
.as_ref()
.and_then(|attributes| attributes.get("type"))
.map_or(false, |values| values.iter().any(|x| x == "hidden"))
{
return true;
}

for element in Self::ROLE_WITH_CONCEPTS {
let role = match *element {
"checkbox" => &CheckboxRole as &dyn AriaRoleDefinitionWithConcepts,
Expand Down Expand Up @@ -1006,6 +1088,16 @@ impl<'a> AriaRoles {
"term" => &TermRole as &dyn AriaRoleDefinitionWithConcepts,
"textbox" => &TextboxRole as &dyn AriaRoleDefinitionWithConcepts,
"generic" => &GenericRole as &dyn AriaRoleDefinitionWithConcepts,
"caption" => &CaptionRole as &dyn AriaRoleDefinitionWithConcepts,
"main" => &MainRole as &dyn AriaRoleDefinitionWithConcepts,
"time" => &TimeRole as &dyn AriaRoleDefinitionWithConcepts,
"p" => &ParagraphRole as &dyn AriaRoleDefinitionWithConcepts,
"aside" => &ComplementaryRole as &dyn AriaRoleDefinitionWithConcepts,
"blockquote" => &BlockQuoteRole as &dyn AriaRoleDefinitionWithConcepts,
"associationlist" => &AssociationListRole as &dyn AriaRoleDefinitionWithConcepts,
"status" => &StatusRole as &dyn AriaRoleDefinitionWithConcepts,
"contentinfo" => &ContentInfoRole as &dyn AriaRoleDefinitionWithConcepts,
"region" => &RegionRole as &dyn AriaRoleDefinitionWithConcepts,
_ => return false,
};
if let Some(mut concepts) = role.concepts_by_element_name(element_name) {
Expand Down Expand Up @@ -1040,14 +1132,29 @@ mod test {
#[test]
fn should_be_interactive() {
let aria_roles = AriaRoles {};
assert!(!aria_roles.is_not_interactive_element("header", None));
assert!(!aria_roles.is_not_interactive_element("input", {
let mut attributes = HashMap::new();
attributes.insert("type".to_string(), vec!["search".to_string()]);
Some(attributes)
}));
}

assert!(!aria_roles.is_not_interactive_element("header"));
assert!(aria_roles.is_not_interactive_element("h1"));
assert!(aria_roles.is_not_interactive_element("h2"));
assert!(aria_roles.is_not_interactive_element("h3"));
assert!(aria_roles.is_not_interactive_element("h4"));
assert!(aria_roles.is_not_interactive_element("h5"));
assert!(aria_roles.is_not_interactive_element("h6"));
#[test]
fn should_not_be_interactive() {
let aria_roles = AriaRoles {};
assert!(aria_roles.is_not_interactive_element("h1", None));
assert!(aria_roles.is_not_interactive_element("h2", None));
assert!(aria_roles.is_not_interactive_element("h3", None));
assert!(aria_roles.is_not_interactive_element("h4", None));
assert!(aria_roles.is_not_interactive_element("h5", None));
assert!(aria_roles.is_not_interactive_element("h6", None));
assert!(aria_roles.is_not_interactive_element("body", None));
assert!(aria_roles.is_not_interactive_element("input", {
let mut attributes = HashMap::new();
attributes.insert("type".to_string(), vec!["hidden".to_string()]);
Some(attributes)
}));
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,9 @@ impl Rule for NoNoninteractiveElementToInteractiveRole {
let role_attribute_static_value = role_attribute.as_static_value()?;
let role_attribute_value = role_attribute_static_value.text();
let element_name = node.name().ok()?.as_jsx_name()?.value_token().ok()?;
if aria_roles.is_not_interactive_element(element_name.text_trimmed())

let attributes = ctx.extract_attributes(&node.attributes());
if aria_roles.is_not_interactive_element(element_name.text_trimmed(), attributes)
&& aria_roles.is_role_interactive(role_attribute_value)
{
// <div> and <span> are considered neither interactive nor non-interactive, depending on the presence or absence of the role attribute.
Expand All @@ -85,7 +87,6 @@ impl Rule for NoNoninteractiveElementToInteractiveRole {
});
}
}

None
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,9 @@ impl Rule for NoNoninteractiveTabindex {

let element_name = node.name().ok()?.as_jsx_name()?.value_token().ok()?;
let aria_roles = ctx.aria_roles();
let attributes = ctx.extract_attributes(&node.attributes());

if aria_roles.is_not_interactive_element(element_name.text_trimmed()) {
if aria_roles.is_not_interactive_element(element_name.text_trimmed(), attributes) {
let tabindex_attribute = node.find_attribute_by_name("tabIndex")?;
let tabindex_attribute_value = tabindex_attribute.initializer()?.value().ok()?;
if attribute_has_negative_tabindex(&tabindex_attribute_value)? {
Expand Down
Loading