feat: Only focus focusable nodes (#884)

* feat: Only focus focusable nodes * chore: Update tests * chore: Update tests * chore: Bring back a11y_focusable
marc2332 · Sep 19, 2024 · 3ae218d · 3ae218d
1 parent ebb22ee
commit 3ae218d
Show file tree

Hide file tree

Showing 13 changed files with 86 additions and 21 deletions.
diff --git a/crates/components/src/button.rs b/crates/components/src/button.rs
@@ -180,7 +180,6 @@ pub fn Button(
             height: "{height}",
             padding: "{padding}",
             margin: "{margin}",
-            a11y_focusable: "true",
             overflow: "clip",
             a11y_role:"button",
             color: "{font_theme.color}",

diff --git a/crates/components/src/input.rs b/crates/components/src/input.rs
@@ -217,7 +217,6 @@ pub fn Input(
             main_align: "center",
             cursor_reference,
             a11y_id,
-            a11y_focusable: "true",
             a11y_role: "textInput",
             a11y_auto_focus: "{auto_focus}",
             onkeydown,

diff --git a/crates/components/src/menu.rs b/crates/components/src/menu.rs
@@ -201,7 +201,6 @@ pub fn MenuItem(
             width: "fill-min",
             padding: "6",
             margin: "2",
-            a11y_focusable: "true",
             a11y_role:"button",
             color: "{font_theme.color}",
             corner_radius: "{corner_radius}",

diff --git a/crates/components/src/radio.rs b/crates/components/src/radio.rs
@@ -83,7 +83,6 @@ pub fn Radio(
             corner_radius: "99",
             rect {
                 a11y_id: focus.attribute(),
-                a11y_focusable: "true",
                 width: "18",
                 height: "18",
                 border: "2 solid {fill}",

diff --git a/crates/components/src/tabs.rs b/crates/components/src/tabs.rs
@@ -142,7 +142,6 @@ pub fn Tab(
             a11y_id,
             width: "{width}",
             height: "{height}",
-            a11y_focusable: "true",
             overflow: "clip",
             a11y_role:"tab",
             color: "{font_theme.color}",
@@ -254,7 +253,6 @@ pub fn BottomTab(children: Element, theme: Option<BottomTabThemeWith>) -> Elemen
             a11y_id,
             width: "{width}",
             height: "{height}",
-            a11y_focusable: "true",
             overflow: "clip",
             a11y_role:"tab",
             color: "{font_theme.color}",

diff --git a/crates/core/src/accessibility/tree.rs b/crates/core/src/accessibility/tree.rs
@@ -257,7 +257,10 @@ impl AccessibilityTree {
             let accessibility_id = node_ref.get_accessibility_id();
 
             if let Some(accessibility_id) = accessibility_id {
-                nodes.push((accessibility_id, node_ref.id()))
+                let accessibility_state = node_ref.get::<AccessibilityNodeState>().unwrap();
+                if accessibility_state.a11y_focusable.is_enabled() {
+                    nodes.push((accessibility_id, node_ref.id()))
+                }
             }
 
             if let Some(tag) = node_ref.node_type().tag() {
@@ -347,7 +350,7 @@ impl AccessibilityTree {
         // Set focusable action
         // This will cause assistive technology to offer the user an option
         // to focus the current element if it supports it.
-        if node_accessibility.a11y_focusable {
+        if node_accessibility.a11y_focusable.is_enabled() {
             builder.add_action(Action::Focus);
         }
 

diff --git a/crates/elements/src/definitions.rs b/crates/elements/src/definitions.rs
@@ -235,10 +235,10 @@ builder_constructors! {
 
         a11y_auto_focus: String,
         a11y_name: String,
-        a11y_focusable: String,
         a11y_role:String,
         a11y_id: AccessibilityId,
         a11y_alt: String,
+        a11y_focusable: String,
         canvas_reference: String,
         layer: String,
         offset_y: String,
@@ -308,10 +308,10 @@ builder_constructors! {
         layer: String,
         a11y_auto_focus: String,
         a11y_name: String,
-        a11y_focusable: String,
         a11y_role:String,
         a11y_id: AccessibilityId,
         a11y_alt: String,
+        a11y_focusable: String,
     };
     /// `paragraph` element let's you build texts with different styles.
     ///
@@ -387,10 +387,10 @@ builder_constructors! {
         cursor_id: String,
         a11y_auto_focus: String,
         a11y_name: String,
-        a11y_focusable: String,
         a11y_role:String,
         a11y_id: AccessibilityId,
         a11y_alt: String,
+        a11y_focusable: String,
         highlights: String,
         highlight_color: String,
         highlight_mode: String,
@@ -459,7 +459,6 @@ builder_constructors! {
         image_reference: String,
         a11y_auto_focus: String,
         a11y_name: String,
-        a11y_focusable: String,
         a11y_role:String,
         a11y_id: AccessibilityId,
         a11y_alt: String,
@@ -499,10 +498,10 @@ builder_constructors! {
         svg_content: String,
         a11y_auto_focus: String,
         a11y_name: String,
-        a11y_focusable: String,
         a11y_role:String,
         a11y_id: AccessibilityId,
         a11y_alt: String,
+        a11y_focusable: String,
     };
 }
 

diff --git a/crates/hooks/src/use_init_native_platform.rs b/crates/hooks/src/use_init_native_platform.rs
@@ -152,7 +152,9 @@ mod test {
     pub async fn uncontrolled_focus_accessibility() {
         #[allow(non_snake_case)]
         fn OtherChild() -> Element {
+            let mut focus = use_focus();
             rsx!(rect {
+                a11y_id: focus.attribute(),
                 a11y_role: "genericContainer",
                 width: "100%",
                 height: "50%",
@@ -216,12 +218,16 @@ mod test {
     #[tokio::test]
     pub async fn auto_focus_accessibility() {
         fn use_focus_app() -> Element {
+            let mut focus_1 = use_focus();
+            let mut focus_2 = use_focus();
             rsx!(
                 rect {
+                    a11y_id: focus_1.attribute(),
                     a11y_role: "genericContainer",
                     a11y_auto_focus: "true",
                 }
                 rect {
+                    a11y_id: focus_2.attribute(),
                     a11y_role: "genericContainer",
                     a11y_auto_focus: "true",
                 }

diff --git a/crates/native-core/src/attributes.rs b/crates/native-core/src/attributes.rs
@@ -119,10 +119,10 @@ impl FromStr for AttributeName {
             "content" => Ok(AttributeName::Content),
             "a11y_auto_focus" => Ok(AttributeName::A11YAutoFocus),
             "a11y_name" => Ok(AttributeName::A11YName),
-            "a11y_focusable" => Ok(AttributeName::A11YFocusable),
             "a11y_role" => Ok(AttributeName::A11YRole),
             "a11y_id" => Ok(AttributeName::A11YId),
             "a11y_alt" => Ok(AttributeName::A11YAlt),
+            "a11y_focusable" => Ok(AttributeName::A11YFocusable),
             "canvas_reference" => Ok(AttributeName::CanvasReference),
             "layer" => Ok(AttributeName::Layer),
             "offset_y" => Ok(AttributeName::OffsetY),

diff --git a/crates/state/src/accessibility.rs b/crates/state/src/accessibility.rs
@@ -29,6 +29,8 @@ use freya_native_core_macro::partial_derive_state;
 
 use crate::{
     CustomAttributeValues,
+    Focusable,
+    Parse,
     ParseAttribute,
     ParseError,
 };
@@ -42,8 +44,8 @@ pub struct AccessibilityNodeState {
     pub a11y_role: Option<Role>,
     pub a11y_alt: Option<String>,
     pub a11y_name: Option<String>,
-    pub a11y_focusable: bool,
     pub a11y_auto_focus: bool,
+    pub a11y_focusable: Focusable,
 }
 
 impl ParseAttribute for AccessibilityNodeState {
@@ -57,6 +59,11 @@ impl ParseAttribute for AccessibilityNodeState {
                     attr.value
                 {
                     self.a11y_id = Some(*id);
+
+                    // Enable focus on nodes that pass a custom a11y id
+                    if self.a11y_focusable.is_unknown() {
+                        self.a11y_focusable = Focusable::Enabled;
+                    }
                 }
             }
             AttributeName::A11YRole => {
@@ -77,14 +84,14 @@ impl ParseAttribute for AccessibilityNodeState {
                     self.a11y_name = Some(attr.to_owned())
                 }
             }
-            AttributeName::A11YFocusable => {
+            AttributeName::A11YAutoFocus => {
                 if let OwnedAttributeValue::Text(attr) = attr.value {
-                    self.a11y_focusable = attr.parse().unwrap_or_default()
+                    self.a11y_auto_focus = attr.parse().unwrap_or_default()
                 }
             }
-            AttributeName::A11YAutoFocus => {
+            AttributeName::A11YFocusable => {
                 if let OwnedAttributeValue::Text(attr) = attr.value {
-                    self.a11y_auto_focus = attr.parse().unwrap_or_default()
+                    self.a11y_focusable = Focusable::parse(attr)?;
                 }
             }
             _ => {}
@@ -108,7 +115,6 @@ impl State<CustomAttributeValues> for AccessibilityNodeState {
             AttributeName::A11YRole,
             AttributeName::A11YAlt,
             AttributeName::A11YName,
-            AttributeName::A11YFocusable,
             AttributeName::A11YAutoFocus,
         ]));
 
@@ -171,7 +177,7 @@ impl State<CustomAttributeValues> for AccessibilityNodeState {
                 #[cfg(debug_assertions)]
                 tracing::info!("Assigned {id:?} to {:?}", node_view.node_id());
 
-                self.a11y_id = Some(id)
+                self.a11y_id = Some(id);
             }
 
             let was_just_created = !had_id && self.a11y_id.is_some();

diff --git a/crates/state/src/values/focusable.rs b/crates/state/src/values/focusable.rs
@@ -0,0 +1,32 @@
+use crate::{
+    Parse,
+    ParseError,
+};
+
+#[derive(Clone, Debug, PartialEq, Default, Eq)]
+pub enum Focusable {
+    #[default]
+    Unknown,
+    Disabled,
+    Enabled,
+}
+
+impl Focusable {
+    pub fn is_unknown(&self) -> bool {
+        matches!(self, Self::Unknown)
+    }
+
+    pub fn is_enabled(&self) -> bool {
+        matches!(self, Self::Enabled)
+    }
+}
+
+impl Parse for Focusable {
+    fn parse(value: &str) -> Result<Self, ParseError> {
+        Ok(match value {
+            "true" => Self::Enabled,
+            "false" => Self::Disabled,
+            _ => Self::Unknown,
+        })
+    }
+}
diff --git a/crates/state/src/values/mod.rs b/crates/state/src/values/mod.rs
@@ -6,6 +6,7 @@ mod corner_radius;
 mod cursor;
 mod decoration;
 mod fill;
+mod focusable;
 mod font;
 mod gaps;
 mod gradient;
@@ -21,6 +22,7 @@ pub use color::*;
 pub use corner_radius::*;
 pub use cursor::*;
 pub use fill::*;
+pub use focusable::*;
 pub use font::*;
 pub use gradient::*;
 pub use highlight::*;

diff --git a/crates/state/tests/parse_focusable.rs b/crates/state/tests/parse_focusable.rs
@@ -0,0 +1,23 @@
+use freya_engine::prelude::*;
+use freya_node_state::{
+    Focusable,
+    Parse,
+};
+
+#[test]
+fn parse_focusable_enabled() {
+    let enabled = Focusable::parse("true");
+    assert_eq!(enabled, Ok(Focusable::Enabled));
+}
+
+#[test]
+fn parse_focusable_disabled() {
+    let disabled = Focusable::parse("false");
+    assert_eq!(disabled, Ok(Focusable::Disabled));
+}
+
+#[test]
+fn parse_focusable_fallback() {
+    let fallback = Focusable::parse("hello!!");
+    assert_eq!(fallback, Ok(Focusable::Unknown));
+}