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.

Sign up for GitHub

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

Jump to bottom

Support a11y for interop views #1241

Merged
merged 8 commits into from
Apr 5, 2024
Merged

Support a11y for interop views #1241

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a7aa4ae
Add platform-specific semantics key and wire it in
elijah-semyonov Apr 5, 2024
d4a1c2c
Fix iOS Demo
elijah-semyonov Apr 5, 2024
2c79d05
Introduce CMPInteropWrappingView
elijah-semyonov Apr 5, 2024
b48adcb
Add native accessibilityLabel to cats example
elijah-semyonov Apr 5, 2024
e46f941
Fix helper ObjC framework
elijah-semyonov Apr 5, 2024
b29bf72
Wire InteropWrappingView on UIKitView API side
elijah-semyonov Apr 5, 2024
ba9e8cf
Rework resolution logic to point to native views
elijah-semyonov Apr 5, 2024
ab9c072
Fix license
elijah-semyonov Apr 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 1 addition & 4 deletions compose/mpp/demo/src/commonMain/kotlin/androidx/compose/mpp/demo/App.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,7 @@ class App(
)
}
) {
buildScreen(MainScreen, navController)
for (i in extraScreens) {
buildScreen(i, navController)
}
buildScreen(MainScreen.mergedWith(extraScreens), navController)
}
}

Expand Down
2 changes: 2 additions & 0 deletions compose/mpp/demo/src/uikitMain/kotlin/NativePopupExample.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@ private fun NativeNavigationPage() {
UIKitView(
factory = {
val view = UIImageView()
view.isAccessibilityElement = true
view.accessibilityLabel = "Cat image $index"
view.contentMode = UIViewContentMode.UIViewContentModeScaleAspectFill
view.clipsToBounds = true
view
Expand Down
6 changes: 6 additions & 0 deletions compose/ui/ui-uikit/src/uikitMain/objc/CMPUIKitUtils/CMPUIKitUtils.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
EA70A7EC2B27106100300068 /* CMPAccessibilityContainer.m in Sources */ = {isa = PBXBuildFile; fileRef = EA70A7E92B27106100300068 /* CMPAccessibilityContainer.m */; };
EA82F4F92B86144E00465418 /* CMPOSLogger.m in Sources */ = {isa = PBXBuildFile; fileRef = EA82F4F82B86144E00465418 /* CMPOSLogger.m */; };
EA82F4FC2B86184F00465418 /* CMPOSLoggerInterval.m in Sources */ = {isa = PBXBuildFile; fileRef = EA82F4FB2B86184F00465418 /* CMPOSLoggerInterval.m */; };
EABD912B2BC02B5F00455279 /* CMPInteropWrappingView.m in Sources */ = {isa = PBXBuildFile; fileRef = EABD912A2BC02B5F00455279 /* CMPInteropWrappingView.m */; };
EAC703E32B8C826E001ECDA6 /* CMPAccessibilityElement.m in Sources */ = {isa = PBXBuildFile; fileRef = EA70A7E82B27106100300068 /* CMPAccessibilityElement.m */; };
EAC703E42B8C826E001ECDA6 /* CMPViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = 997DFCDD2B18D135000B56B5 /* CMPViewController.m */; };
EAC703E52B8C826E001ECDA6 /* CMPOSLogger.m in Sources */ = {isa = PBXBuildFile; fileRef = EA82F4F82B86144E00465418 /* CMPOSLogger.m */; };
Expand Down Expand Up @@ -75,6 +76,8 @@
EA82F4F82B86144E00465418 /* CMPOSLogger.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = CMPOSLogger.m; sourceTree = "<group>"; };
EA82F4FA2B86184F00465418 /* CMPOSLoggerInterval.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = CMPOSLoggerInterval.h; sourceTree = "<group>"; };
EA82F4FB2B86184F00465418 /* CMPOSLoggerInterval.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = CMPOSLoggerInterval.m; sourceTree = "<group>"; };
EABD91292BC02B5F00455279 /* CMPInteropWrappingView.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = CMPInteropWrappingView.h; sourceTree = "<group>"; };
EABD912A2BC02B5F00455279 /* CMPInteropWrappingView.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = CMPInteropWrappingView.m; sourceTree = "<group>"; };
EAC703DF2B8C8154001ECDA6 /* CMPUIKitUtilsTestApp-Bridging-Header.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "CMPUIKitUtilsTestApp-Bridging-Header.h"; sourceTree = "<group>"; };
/* End PBXFileReference section */

Expand Down Expand Up @@ -119,6 +122,8 @@
EA82F4F82B86144E00465418 /* CMPOSLogger.m */,
EA82F4FA2B86184F00465418 /* CMPOSLoggerInterval.h */,
EA82F4FB2B86184F00465418 /* CMPOSLoggerInterval.m */,
EABD91292BC02B5F00455279 /* CMPInteropWrappingView.h */,
EABD912A2BC02B5F00455279 /* CMPInteropWrappingView.m */,
);
path = CMPUIKitUtils;
sourceTree = "<group>";
Expand Down Expand Up @@ -293,6 +298,7 @@
buildActionMask = 2147483647;
files = (
997DFCDE2B18D135000B56B5 /* CMPViewController.m in Sources */,
EABD912B2BC02B5F00455279 /* CMPInteropWrappingView.m in Sources */,
EA70A7EC2B27106100300068 /* CMPAccessibilityContainer.m in Sources */,
EA82F4F92B86144E00465418 /* CMPOSLogger.m in Sources */,
EA70A7EB2B27106100300068 /* CMPAccessibilityElement.m in Sources */,
Expand Down
19 changes: 19 additions & 0 deletions compose/ui/ui-uikit/src/uikitMain/objc/CMPUIKitUtils/CMPUIKitUtils/CMPInteropWrappingView.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//
// CMPInteropWrappingView.h
// CMPUIKitUtils
//
// Created by Ilia.Semenov on 05/04/2024.
//

#import <UIKit/UIKit.h>
#import "CMPMacros.h"

NS_ASSUME_NONNULL_BEGIN

@interface CMPInteropWrappingView : UIView
Copy link
Collaborator

Choose a reason for hiding this comment

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

LGTM, One thing to think about:
Creating new subclass of UIView / UIAccessibilityElement for every new use-case (which is now) VS create one subclass of UIView, one of UIAccessibilityElement with all default implementations of necessary methods from Accessibility protocols.

Copy link
Author

Choose a reason for hiding this comment

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

The whole point of this is to make the UIAccessibility methods be visible to Kotlin in the first place. Cinterop doesn't see some of them because they are declared in a category and struggles to do overriding because of duality of property and selectors in ObjC which is not the case for fields and methods in Kotlin. Keeping it in ObjC and avoiding presence of subclasses in Kotlin altogether would require adding a lot of code to ObjC to map the values and types from compose core. Thus I find the current solution the most optimal trade-off.


- (__nullable id)accessibilityContainer CMP_MUST_BE_OVERRIDED;

@end

NS_ASSUME_NONNULL_END
16 changes: 16 additions & 0 deletions compose/ui/ui-uikit/src/uikitMain/objc/CMPUIKitUtils/CMPUIKitUtils/CMPInteropWrappingView.m
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//
// CMPInteropWrappingView.m
// CMPUIKitUtils
//
// Created by Ilia.Semenov on 05/04/2024.
//

#import "CMPInteropWrappingView.h"

@implementation CMPInteropWrappingView

- (__nullable id)accessibilityContainer {
CMP_MUST_BE_OVERRIDED_INVARIANT_VIOLATION
}

@end
49 changes: 39 additions & 10 deletions compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/interop/UIKitView.uikit.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ import androidx.compose.ui.layout.EmptyLayout
import androidx.compose.ui.layout.onGloballyPositioned
import androidx.compose.ui.layout.positionInRoot
import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.semantics.AccessibilityKey
import androidx.compose.ui.semantics.SemanticsPropertyReceiver
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.uikit.toUIColor
import androidx.compose.ui.unit.IntOffset
import androidx.compose.ui.unit.IntRect
Expand All @@ -54,11 +57,33 @@ import platform.UIKit.addChildViewController
import platform.UIKit.didMoveToParentViewController
import platform.UIKit.removeFromParentViewController
import platform.UIKit.willMoveToParentViewController
import androidx.compose.ui.uikit.utils.CMPInteropWrappingView
import kotlinx.cinterop.readValue
import platform.CoreGraphics.CGRectZero

private val STUB_CALLBACK_WITH_RECEIVER: Any.() -> Unit = {}
private val DefaultViewResize: UIView.(CValue<CGRect>) -> Unit = { rect -> this.setFrame(rect) }
private val DefaultViewControllerResize: UIViewController.(CValue<CGRect>) -> Unit = { rect -> this.view.setFrame(rect) }

internal class InteropWrappingView: CMPInteropWrappingView(frame = CGRectZero.readValue()) {
var actualAccessibilityContainer: Any? = null

override fun accessibilityContainer(): Any? {
return actualAccessibilityContainer
}
}

internal val NativeViewSemanticsKey = AccessibilityKey<InteropWrappingView>(
name = "InteropView",
mergePolicy = { _, _ ->
throw IllegalStateException(
"Can't merge NativeView semantics property."
)
}
)

private var SemanticsPropertyReceiver.nativeView by NativeViewSemanticsKey

/**
* @param factory The block creating the [UIView] to be composed.
* @param modifier The modifier to be applied to the layout. Size should be specified in modifier.
Expand Down Expand Up @@ -103,7 +128,7 @@ fun <T : UIView> UIKitView(
val rect = newRectInPixels.toRect().toDpRect(density)

interopContext.deferAction {
embeddedInteropComponent.container.setFrame(rect.asCGRect())
embeddedInteropComponent.wrappingView.setFrame(rect.asCGRect())
}

if (rectInPixels.width != newRectInPixels.width || rectInPixels.height != newRectInPixels.height) {
Expand All @@ -119,12 +144,14 @@ fun <T : UIView> UIKitView(
}.drawBehind {
// Clear interop area to make visible the component under our canvas.
drawRect(Color.Transparent, blendMode = BlendMode.Clear)
}.trackUIKitInterop(embeddedInteropComponent.container).let {
}.trackUIKitInterop(embeddedInteropComponent.wrappingView).let {
if (interactive) {
it.then(InteropViewCatchPointerModifier())
} else {
it
}
}.semantics {
nativeView = embeddedInteropComponent.wrappingView
}
)

Expand Down Expand Up @@ -203,7 +230,7 @@ fun <T : UIViewController> UIKitViewController(
val rect = newRectInPixels.toRect().toDpRect(density)

interopContext.deferAction {
embeddedInteropComponent.container.setFrame(rect.asCGRect())
embeddedInteropComponent.wrappingView.setFrame(rect.asCGRect())
}

if (rectInPixels.width != newRectInPixels.width || rectInPixels.height != newRectInPixels.height) {
Expand All @@ -219,12 +246,14 @@ fun <T : UIViewController> UIKitViewController(
}.drawBehind {
// Clear interop area to make visible the component under our canvas.
drawRect(Color.Transparent, blendMode = BlendMode.Clear)
}.trackUIKitInterop(embeddedInteropComponent.container).let {
}.trackUIKitInterop(embeddedInteropComponent.wrappingView).let {
if (interactive) {
it.then(InteropViewCatchPointerModifier())
} else {
it
}
}.semantics {
nativeView = embeddedInteropComponent.wrappingView
}
)

Expand Down Expand Up @@ -260,29 +289,29 @@ private abstract class EmbeddedInteropComponent<T : Any>(
val interopContainer: UIKitInteropContainer,
val onRelease: (T) -> Unit
) {
var container = UIView()
val wrappingView = InteropWrappingView()
lateinit var component: T
lateinit var updater: Updater<T>

fun setBackgroundColor(color: Color) {
if (color == Color.Unspecified) {
container.backgroundColor = interopContainer.containerView.backgroundColor
wrappingView.backgroundColor = interopContainer.containerView.backgroundColor
} else {
container.backgroundColor = color.toUIColor()
wrappingView.backgroundColor = color.toUIColor()
}
}

abstract fun addToHierarchy()
abstract fun removeFromHierarchy()

protected fun addViewToHierarchy(view: UIView) {
container.addSubview(view)
interopContainer.addInteropView(container)
wrappingView.addSubview(view)
interopContainer.addInteropView(wrappingView)
}

protected fun removeViewFromHierarchy(view: UIView) {
view.removeFromSuperview()
interopContainer.removeInteropView(container)
interopContainer.removeInteropView(wrappingView)
updater.dispose()
onRelease(component)
}
Expand Down
31 changes: 22 additions & 9 deletions compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/platform/Accessibility.uikit.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package androidx.compose.ui.platform
import androidx.compose.runtime.ExperimentalComposeApi
import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.geometry.Rect
import androidx.compose.ui.interop.InteropWrappingView
import androidx.compose.ui.interop.NativeViewSemanticsKey
import androidx.compose.ui.semantics.Role
import androidx.compose.ui.semantics.SemanticsActions
import androidx.compose.ui.semantics.SemanticsConfiguration
Expand Down Expand Up @@ -115,6 +117,7 @@ private object CachedAccessibilityPropertyKeys {
val accessibilityTraits = CachedAccessibilityPropertyKey<UIAccessibilityTraits>()
val accessibilityValue = CachedAccessibilityPropertyKey<String?>()
val accessibilityFrame = CachedAccessibilityPropertyKey<CValue<CGRect>>()
val nativeView = CachedAccessibilityPropertyKey<InteropWrappingView?>()
}

/**
Expand Down Expand Up @@ -185,6 +188,12 @@ private class AccessibilityElement(

private var children = mutableListOf<AccessibilityElement>()

private val nativeView: InteropWrappingView?
get() = getOrElse(CachedAccessibilityPropertyKeys.nativeView) {
cachedConfig.getOrNull(NativeViewSemanticsKey)?.also {
it.actualAccessibilityContainer = parent?.accessibilityContainer
}
}

/**
* Constructed lazily if :
Expand All @@ -202,6 +211,8 @@ private class AccessibilityElement(
/**
* Returns accessibility element communicated to iOS Accessibility services for the given [index].
* Takes a child at [index].
* If the child is constructed from a [SemanticsNode] with [NativeViewSemanticsKey],
* then the element at the given index is a native view.
* If the child has its own children, then the element at the given index is the synthesized container
* for the child. Otherwise, the element at the given index is the child itself.
*/
Expand All @@ -211,7 +222,11 @@ private class AccessibilityElement(
return if (i in children.indices) {
val child = children[i]

if (child.hasChildren) {
val nativeView = child.nativeView

if (nativeView != null) {
return nativeView
} else if (child.hasChildren) {
child.accessibilityContainer
} else {
child
Expand All @@ -231,14 +246,12 @@ private class AccessibilityElement(
for (index in 0 until children.size) {
val child = children[index]

if (child.hasChildren) {
if (element == child.accessibilityContainer) {
return index.toLong()
}
} else {
if (element == child) {
return index.toLong()
}
if (element == child.nativeView) {
return index.toLong()
} else if (child.hasChildren && element == child.accessibilityContainer) {
return index.toLong()
} else if (element == child) {
return index.toLong()
}
}

Expand Down