feat: Optional uncontrolled accessibility IDs (#867)

* feat: Optional uncontrolled accessibility IDs

* chore: test and fixes

crates/common/src/accessibility.rs

@@ -1,3 +1,8 @@
+use std::sync::atomic::{
+    AtomicU64,
+    Ordering,
+};
+
 use freya_native_core::NodeId;
 use rustc_hash::{
     FxHashMap,
@@ -24,3 +29,21 @@ impl AccessibilityDirtyNodes {
         self.removed.clear();
     }
 }
+
+pub struct AccessibilityGenerator {
+    counter: AtomicU64,
+}
+
+impl Default for AccessibilityGenerator {
+    fn default() -> Self {
+        Self {
+            counter: AtomicU64::new(1), // Must start at 1 because 0 is reserved for the Root
+        }
+    }
+}
+
+impl AccessibilityGenerator {
+    pub fn new_id(&self) -> u64 {
+        self.counter.fetch_add(1, Ordering::Relaxed)
+    }
+}

crates/core/src/accessibility/tree.rs

@@ -167,16 +167,22 @@ impl AccessibilityTree {
         let mut nodes = Vec::new();
         for node_id in added_or_updated_ids {
             let node_ref = rdom.get(node_id).unwrap();
-            let layout_node = layout.get(node_id).unwrap();
-            let node_accessibility_state = node_ref.get::<AccessibilityNodeState>().unwrap();
-            let accessibility_node =
-                Self::create_node(&node_ref, layout_node, &node_accessibility_state);
+            let node_accessibility_state = node_ref.get::<AccessibilityNodeState>();
+            let layout_node = layout.get(node_id);
+
+            if let Some((node_accessibility_state, layout_node)) =
+                node_accessibility_state.as_ref().zip(layout_node)
+            {
+                let accessibility_node =
+                    Self::create_node(&node_ref, layout_node, node_accessibility_state);
 
-            let accessibility_id = node_ref.get_accessibility_id().unwrap();
+                let accessibility_id = node_ref.get_accessibility_id().unwrap();
 
-            nodes.push((accessibility_id, accessibility_node));
+                nodes.push((accessibility_id, accessibility_node));
+            }
         }
 
+        // Fallback the focused id to the root if the focused node no longer exists
         if !self.map.contains_key(&self.focused_id) {
             self.focused_id = ACCESSIBILITY_ROOT_ID;
         }

crates/core/src/dom/doms.rs

@@ -7,6 +7,7 @@ use std::sync::{
 use dioxus_core::VirtualDom;
 use freya_common::{
     AccessibilityDirtyNodes,
+    AccessibilityGenerator,
     CompositorDirtyNodes,
     Layers,
     ParagraphElements,
@@ -128,6 +129,7 @@ pub struct FreyaDOM {
     compositor_dirty_area: Arc<Mutex<CompositorDirtyArea>>,
     compositor_cache: Arc<Mutex<CompositorCache>>,
     accessibility_dirty_nodes: Arc<Mutex<AccessibilityDirtyNodes>>,
+    accessibility_generator: Arc<AccessibilityGenerator>,
 }
 
 impl Default for FreyaDOM {
@@ -154,6 +156,7 @@ impl Default for FreyaDOM {
             compositor_dirty_area: Arc::default(),
             compositor_cache: Arc::default(),
             accessibility_dirty_nodes: Arc::default(),
+            accessibility_generator: Arc::default(),
         }
     }
 }
@@ -187,6 +190,10 @@ impl FreyaDOM {
         self.accessibility_dirty_nodes.lock().unwrap()
     }
 
+    pub fn accessibility_generator(&self) -> &Arc<AccessibilityGenerator> {
+        &self.accessibility_generator
+    }
+
     /// Create the initial DOM from the given Mutations
     pub fn init_dom(&mut self, vdom: &mut VirtualDom, scale_factor: f32) {
         // Build the RealDOM
@@ -211,6 +218,7 @@ impl FreyaDOM {
         ctx.insert(self.compositor_dirty_nodes.clone());
         ctx.insert(self.accessibility_dirty_nodes.clone());
         ctx.insert(self.rdom.root_id());
+        ctx.insert(self.accessibility_generator.clone());
 
         self.rdom.update_state(ctx);
     }
@@ -240,6 +248,7 @@ impl FreyaDOM {
         ctx.insert(self.compositor_dirty_nodes.clone());
         ctx.insert(self.accessibility_dirty_nodes.clone());
         ctx.insert(self.rdom.root_id());
+        ctx.insert(self.accessibility_generator.clone());
 
         // Update the Node's states
         let (_, diff) = self.rdom.update_state(ctx);

crates/hooks/src/use_focus.rs

@@ -1,3 +1,5 @@
+use std::sync::Arc;
+
 use dioxus_core::{
     use_hook,
     AttributeValue,
@@ -12,6 +14,7 @@ use dioxus_signals::{
     Signal,
     Writable,
 };
+use freya_common::AccessibilityGenerator;
 use freya_core::{
     accessibility::ACCESSIBILITY_ROOT_ID,
     platform_state::NavigationMode,
@@ -26,7 +29,6 @@ use freya_node_state::CustomAttributeValues;
 
 use crate::{
     use_platform,
-    AccessibilityIdCounter,
     NavigationMark,
     UsePlatform,
 };
@@ -102,17 +104,13 @@ impl UseFocus {
 
 /// Create a focus manager for a node.
 pub fn use_focus() -> UseFocus {
-    let accessibility_id_counter = use_context::<AccessibilityIdCounter>();
+    let accessibility_generator = use_context::<Arc<AccessibilityGenerator>>();
     let focused_id = use_context::<Signal<AccessibilityId>>();
     let navigation_mode = use_context::<Signal<NavigationMode>>();
     let navigation_mark = use_context::<Signal<NavigationMark>>();
     let platform = use_platform();
 
-    let id = use_hook(|| {
-        let mut counter = accessibility_id_counter.borrow_mut();
-        *counter += 1;
-        AccessibilityId(*counter)
-    });
+    let id = use_hook(|| AccessibilityId(accessibility_generator.new_id()));
 
     let is_focused = use_memo(move || id == *focused_id.read());
 

crates/hooks/src/use_init_native_platform.rs

@@ -1,8 +1,3 @@
-use std::{
-    cell::RefCell,
-    rc::Rc,
-};
-
 use dioxus_core::{
     prelude::{
         consume_context,
@@ -20,7 +15,6 @@ use dioxus_signals::{
 use freya_core::prelude::NativePlatformReceiver;
 
 use crate::use_init_asset_cacher;
-pub type AccessibilityIdCounter = Rc<RefCell<u64>>;
 
 #[derive(Clone)]
 pub struct NavigationMark(bool);
@@ -45,9 +39,6 @@ pub fn use_init_native_platform() -> UsePlatformEvents {
     // Inithe global asset cacher
     use_init_asset_cacher();
 
-    // Init the Accessibility Node ID generator
-    use_context_provider(|| Rc::new(RefCell::new(0u64)));
-
     // Init the NavigationMark signal
     let navigation_mark = use_context_provider(|| Signal::new(NavigationMark(true)));
 
@@ -156,4 +147,69 @@ mod test {
         assert_ne!(first_focus_id, second_focus_id);
         assert_ne!(second_focus_id, ACCESSIBILITY_ROOT_ID);
     }
+
+    #[tokio::test]
+    pub async fn uncontrolled_focus_accessibility() {
+        #[allow(non_snake_case)]
+        fn OtherChild() -> Element {
+            rsx!(rect {
+                role: "genericContainer",
+                width: "100%",
+                height: "50%",
+            })
+        }
+
+        fn use_focus_app() -> Element {
+            rsx!(
+                rect {
+                    width: "100%",
+                    height: "100%",
+                    OtherChild {},
+                    OtherChild {}
+                }
+            )
+        }
+
+        let mut utils = launch_test_with_config(
+            use_focus_app,
+            TestingConfig {
+                size: (100.0, 100.0).into(),
+                ..TestingConfig::default()
+            },
+        );
+
+        // Initial state
+        utils.wait_for_update().await;
+        assert_eq!(utils.focus_id(), ACCESSIBILITY_ROOT_ID);
+
+        // Navigate to the first rect
+        utils.push_event(PlatformEvent::Keyboard {
+            name: EventName::KeyDown,
+            key: Key::Tab,
+            code: Code::Tab,
+            modifiers: Modifiers::default(),
+        });
+        utils.wait_for_update().await;
+
+        // First rect is now focused
+        utils.wait_for_update().await;
+        utils.wait_for_update().await;
+        let first_focus_id = utils.focus_id();
+        assert_ne!(first_focus_id, ACCESSIBILITY_ROOT_ID);
+
+        // Navigate to the second rect
+        utils.push_event(PlatformEvent::Keyboard {
+            name: EventName::KeyDown,
+            key: Key::Tab,
+            code: Code::Tab,
+            modifiers: Modifiers::default(),
+        });
+        utils.wait_for_update().await;
+
+        utils.wait_for_update().await;
+        utils.wait_for_update().await;
+        let second_focus_id = utils.focus_id();
+        assert_ne!(first_focus_id, second_focus_id);
+        assert_ne!(second_focus_id, ACCESSIBILITY_ROOT_ID);
+    }
 }

crates/renderer/src/app.rs

@@ -139,6 +139,8 @@ impl Application {
             .insert_any_root_context(Box::new(self.platform_receiver.clone()));
         self.vdom
             .insert_any_root_context(Box::new(Arc::new(self.ticker_sender.subscribe())));
+        self.vdom
+            .insert_any_root_context(Box::new(self.sdom.get().accessibility_generator().clone()));
     }
 
     /// Make the first build of the VirtualDOM and sync it with the RealDOM.

crates/state/Cargo.toml

@@ -30,6 +30,7 @@ tokio = { workspace = true }
 accesskit = { workspace = true }
 shipyard = { workspace = true }
 rustc-hash= { workspace = true }
+tracing = { workspace = true }
 
 uuid = { workspace = true }
 bytes = "1.5.0"

crates/state/src/accessibility.rs

@@ -7,7 +7,10 @@ use accesskit::{
     NodeId as AccessibilityId,
     Role,
 };
-use freya_common::AccessibilityDirtyNodes;
+use freya_common::{
+    AccessibilityDirtyNodes,
+    AccessibilityGenerator,
+};
 use freya_native_core::{
     attributes::AttributeName,
     exports::shipyard::Component,
@@ -114,6 +117,7 @@ impl State<CustomAttributeValues> for AccessibilityNodeState {
         let accessibility_dirty_nodes = context
             .get::<Arc<Mutex<AccessibilityDirtyNodes>>>()
             .unwrap();
+        let accessibility_generator = context.get::<Arc<AccessibilityGenerator>>().unwrap();
         let mut accessibility = AccessibilityNodeState {
             node_id: node_view.node_id(),
             ..Default::default()
@@ -149,17 +153,27 @@ impl State<CustomAttributeValues> for AccessibilityNodeState {
 
         let changed = &accessibility != self;
 
+        *self = accessibility;
+
         if changed {
+            // Assign an accessibility ID if none was passed but the node has a role
+            if self.accessibility_id.is_none() && self.role.is_some() {
+                let id = AccessibilityId(accessibility_generator.new_id());
+                #[cfg(debug_assertions)]
+                tracing::info!("Assigned {id:?} to {:?}", node_view.node_id());
+
+                self.accessibility_id = Some(id)
+            }
+
             // Add or update this node if it is the Root or if it has an accessibility ID
-            if accessibility.accessibility_id.is_some() || node_view.node_id() == *root_id {
+            if self.accessibility_id.is_some() || node_view.node_id() == *root_id {
                 accessibility_dirty_nodes
                     .lock()
                     .unwrap()
                     .add_or_update(node_view.node_id())
             }
         }
 
-        *self = accessibility;
         changed
     }
 }

crates/testing/src/test_handler.rs

@@ -94,6 +94,13 @@ impl TestingHandler {
             .insert_any_root_context(Box::new(self.platform_receiver.clone()));
         self.vdom
             .insert_any_root_context(Box::new(Arc::new(self.ticker_sender.subscribe())));
+        let accessibility_generator = {
+            let sdom = self.sdom();
+            let fdom = sdom.get();
+            fdom.accessibility_generator().clone()
+        };
+        self.vdom
+            .insert_any_root_context(Box::new(accessibility_generator));
     }
 
     /// Wait and apply new changes