Fix node visibility issue

JetBrains · Dec 11, 2024 · 7b2cd43 · 7b2cd43
1 parent 5b64139
commit 7b2cd43
Show file tree

Hide file tree

Showing 2 changed files with 90 additions and 14 deletions.
diff --git a/...entedTest/kotlin/androidx/compose/ui/accessibility/ComponentsAccessibilitySemanticTest.kt b/...entedTest/kotlin/androidx/compose/ui/accessibility/ComponentsAccessibilitySemanticTest.kt
@@ -19,6 +19,7 @@ package androidx.compose.ui.accessibility
 import androidx.compose.foundation.Image
 import androidx.compose.foundation.clickable
 import androidx.compose.foundation.layout.Column
+import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.text.selection.SelectionContainer
 import androidx.compose.material.Button
 import androidx.compose.material.Checkbox
@@ -39,6 +40,7 @@ import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.setValue
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.graphics.ImageBitmap
+import androidx.compose.ui.graphics.graphicsLayer
 import androidx.compose.ui.platform.testTag
 import androidx.compose.ui.semantics.Role
 import androidx.compose.ui.semantics.heading
@@ -529,4 +531,74 @@ class ComponentsAccessibilitySemanticTest {
             }
         }
     }
-}
+
+    @Test
+    fun testVisibleNodes() = runUIKitInstrumentedTest {
+        var alpha by mutableStateOf(0f)
+
+        setContentWithAccessibilityEnabled {
+            Text("Hidden", modifier = Modifier.graphicsLayer {
+                this.alpha = alpha
+            })
+        }
+
+        assertAccessibilityTree {
+            label = "Hidden"
+            isAccessibilityElement = false
+        }
+
+        alpha = 1f
+        assertAccessibilityTree {
+            label = "Hidden"
+            isAccessibilityElement = true
+        }
+    }
+
+    @Test
+    fun testVisibleNodeContainers() = runUIKitInstrumentedTest {
+        var alpha by mutableStateOf(0f)
+
+        setContentWithAccessibilityEnabled {
+            Column {
+                Text("Text 1")
+                Row(modifier = Modifier.graphicsLayer {
+                    this.alpha = alpha
+                }) {
+                    Text("Text 2")
+                    Text("Text 3")
+                }
+            }
+        }
+
+        assertAccessibilityTree {
+            node {
+                label = "Text 1"
+                isAccessibilityElement = true
+            }
+            node {
+                label = "Text 2"
+                isAccessibilityElement = false
+            }
+            node {
+                label = "Text 3"
+                isAccessibilityElement = false
+            }
+        }
+
+        alpha = 1f
+        assertAccessibilityTree {
+            node {
+                label = "Text 1"
+                isAccessibilityElement = true
+            }
+            node {
+                label = "Text 2"
+                isAccessibilityElement = true
+            }
+            node {
+                label = "Text 3"
+                isAccessibilityElement = true
+            }
+        }
+    }
+}
diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/Accessibility.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/Accessibility.uikit.kt
@@ -115,7 +115,6 @@ private class CachedAccessibilityPropertyKey<V>
 
 private object CachedAccessibilityPropertyKeys {
     val accessibilityLabel = CachedAccessibilityPropertyKey<String?>()
-    val isAccessibilityElement = CachedAccessibilityPropertyKey<Boolean>()
     val accessibilityIdentifier = CachedAccessibilityPropertyKey<String?>()
     val accessibilityHint = CachedAccessibilityPropertyKey<String?>()
     val accessibilityCustomActions = CachedAccessibilityPropertyKey<List<UIAccessibilityCustomAction>>()
@@ -620,10 +619,11 @@ private class AccessibilityElement(
         }
     }
 
-    override fun isAccessibilityElement(): Boolean =
-        getOrElse(CachedAccessibilityPropertyKeys.isAccessibilityElement) {
-            semanticsNode.isAccessibilityElement
-        }
+    override fun isAccessibilityElement(): Boolean {
+        // Node visibility changes don't trigger accessibility semantic recalculation.
+        // This value should not be cached. See [SemanticsNode.isHidden]
+        return semanticsNode.isAccessibilityElement
+    }
 
     override fun accessibilityIdentifier(): String? =
         getOrElse(CachedAccessibilityPropertyKeys.accessibilityIdentifier) {
@@ -863,7 +863,6 @@ private class AccessibilityElement(
         logger.apply {
             log("${indent}AccessibilityElement_$semanticsNodeId")
             log("$indent  containmentChain: ${debugContainmentChain()}")
-            log("$indent  isAccessibilityElement: $isAccessibilityElement")
             log("$indent  accessibilityLabel: $accessibilityLabel")
             log("$indent  accessibilityValue: $accessibilityValue")
             log("$indent  accessibilityTraits: $accessibilityTraits")
@@ -879,7 +878,7 @@ private class AccessibilityElement(
         }
 
         val containsPoint = semanticsNode.boundsInWindow.contains(offsetInWindow)
-        if (containsPoint && isAccessibilityElement) {
+        if (containsPoint && semanticsNode.isAccessibilityElement) {
             return this
         }
 
@@ -1162,6 +1161,7 @@ internal class AccessibilityMediator(
         accessibilityDebugLogger?.log("AccessibilityMediator for $view created")
 
         view.accessibilityElements = listOf<NSObject>()
+        var notificationName = UIAccessibilityScreenChangedNotification
         coroutineScope.launch {
             // The main loop that listens for invalidations and performs the tree syncing
             // Will exit on CancellationException from within await on `invalidationChannel.receive()`
@@ -1185,8 +1185,10 @@ internal class AccessibilityMediator(
 
                     debugLogger?.log("AccessibilityMediator.sync took $time")
                     debugLogger?.log("LayoutChanged, newElementToFocus: ${result.newElementToFocus}")
+                    UIAccessibilityPostNotification(notificationName, result.newElementToFocus)
 
-                    UIAccessibilityPostNotification(UIAccessibilityLayoutChangedNotification, result.newElementToFocus)
+                    // Post screen change notification only once
+                    notificationName = UIAccessibilityLayoutChangedNotification
                 } else {
                     if (view.accessibilityElements?.isEmpty() != true) {
                         view.accessibilityElements = listOf<NSObject>()
@@ -1552,17 +1554,19 @@ private val SemanticsNode.isAccessibilityElement: Boolean
 // Simplified version of the isScreenReaderFocusable() from the
 // AndroidComposeViewAccessibilityDelegateCompat.android.kt
 private fun SemanticsNode.isScreenReaderFocusable(): Boolean {
-    val isSpeakingNode = unmergedConfig.contains(SemanticsProperties.ContentDescription) ||
+    return !isHidden &&
+        (unmergedConfig.isMergingSemanticsOfDescendants ||
+            isUnmergedLeafNode && isSpeakingNode)
+}
+
+private val SemanticsNode.isSpeakingNode: Boolean get() {
+    return unmergedConfig.contains(SemanticsProperties.ContentDescription) ||
         unmergedConfig.contains(SemanticsProperties.EditableText) ||
         unmergedConfig.contains(SemanticsProperties.Text) ||
         unmergedConfig.contains(SemanticsProperties.StateDescription) ||
         unmergedConfig.contains(SemanticsProperties.ToggleableState) ||
         unmergedConfig.contains(SemanticsProperties.Selected) ||
         unmergedConfig.contains(SemanticsProperties.ProgressBarRangeInfo)
-
-    return !isHidden &&
-        (unmergedConfig.isMergingSemanticsOfDescendants ||
-            isUnmergedLeafNode && isSpeakingNode)
 }
 
 @OptIn(ExperimentalComposeUiApi::class)