diff --git a/priv/templates/mob.new/android/app/src/main/java/MobBridge.kt.eex b/priv/templates/mob.new/android/app/src/main/java/MobBridge.kt.eex index b6f1ba3..c5bac31 100644 --- a/priv/templates/mob.new/android/app/src/main/java/MobBridge.kt.eex +++ b/priv/templates/mob.new/android/app/src/main/java/MobBridge.kt.eex @@ -81,6 +81,11 @@ import androidx.compose.foundation.verticalScroll import androidx.compose.ui.layout.onGloballyPositioned import androidx.compose.ui.layout.boundsInWindow import androidx.compose.ui.platform.testTag +import androidx.compose.ui.semantics.Role +import androidx.compose.ui.semantics.contentDescription +import androidx.compose.ui.semantics.disabled +import androidx.compose.ui.semantics.role +import androidx.compose.ui.semantics.semantics import android.view.PixelCopy import android.view.WindowManager import kotlinx.coroutines.CoroutineScope @@ -2229,9 +2234,32 @@ private fun RenderNodeInner(node: MobNode, modifier: Modifier) { // button installs its own onClick via the Button composable. Mirrors iOS, // where most node types pick up onTapGesture via .ifLet(node.onTap). val tapHandle = intProp(node.props, "on_tap") - val tapModifier = if (tapHandle != null && node.type != "button") { - modifier.clickable { MobBridge.nativeSendTap(tapHandle) } - } else modifier + val isDisabled = boolProp(node.props, "disabled") ?: false + val accessibilityRole = node.props["accessibility_role"] as? String + val isButton = node.type == "box" && accessibilityRole == "button" + val tapModifier = when { + // Require a handler here. Compose's ClickableSemanticsNode publishes + // disabled() whenever `enabled` is false, so encoding "no tap handler" + // as enabled = false made a perfectly live box whose tap is handled by + // an ancestor announce as "…, button, disabled". With no handler we + // fall through to the semantics-only path below, which still sets the + // button role. + isButton && tapHandle != null -> + modifier.clickable(enabled = !isDisabled, role = Role.Button) { + MobBridge.nativeSendTap(tapHandle) + } + + // `enabled = !isDisabled` here too, not just on the button arm above. + // Without it a box with `disabled: true` and no explicit + // accessibility_role still dispatched taps, while the semantics block + // below simultaneously marked it disabled() — announced as disabled to + // TalkBack and still firing. iOS applies .disabled() to every box + // regardless of role, so this also keeps the platforms in step. + tapHandle != null && node.type != "button" -> + modifier.clickable(enabled = !isDisabled) { MobBridge.nativeSendTap(tapHandle) } + + else -> modifier + } val base = tapModifier.then(nodeModifier(node.props)) // Track on-screen frame + set a testTag for any node carrying an :id, so the // agent can read positions (Mob.Test.element_frames) without a screenshot. @@ -2257,7 +2285,21 @@ private fun RenderNodeInner(node: MobNode, modifier: Modifier) { // "top_leading" / etc.) — defaults to TopStart for back-compat. "box" -> { val hasWidth = floatProp(node.props, "width") != null - val boxModifier = if (hasWidth) m else m.fillMaxWidth() + val accessibilityLabel = node.props["accessibility_label"] as? String + // Merge for a label OR an explicit button role. Setting + // role/disabled without merging leaves them on the container while + // each child stays its own node, so TalkBack walks into a + // "button" and reads its children as separate elements. Matches + // the iOS side, which collapses on the same condition. + val accessibilityModifier = Modifier.semantics( + mergeDescendants = accessibilityLabel != null || isButton, + ) { + if (accessibilityLabel != null) contentDescription = accessibilityLabel + if (isButton) role = Role.Button + if (isDisabled) disabled() + } + val boxModifier = (if (hasWidth) m else m.fillMaxWidth()) + .then(accessibilityModifier) Box(modifier = boxModifier, contentAlignment = boxAlignProp(node.props)) { node.children.forEach { RenderNode(it) } } @@ -3112,12 +3154,24 @@ private fun MobGpuView(node: MobNode, modifier: Modifier) { // container paint the same way M3's Button does, and can't take those // via a modifier chain). Content-area props are read directly off // `node.props` with those two keys stripped instead. +// Node types that install their own scrollable container. A sheet must not +// wrap these in another vertical scroll: Compose throws on a scrollable +// measured with infinite max height. +private fun isScrollableNode(node: MobNode): Boolean = + node.type == "scroll" || node.type == "lazy_list" || node.children.any(::isScrollableNode) + @OptIn(ExperimentalMaterial3Api::class) @Composable private fun MobSheet(node: MobNode) { - val detents = detentsProp(node.props) + val rawDetents = sheetDetentsProp(node.props) + val contentDetent = rawDetents.filterIsInstance() + .firstOrNull { detent -> detent.optString("type") == "content" } + val detents = rawDetents.filterIsInstance() + .filter { detent -> detent == "medium" || detent == "large" } + .ifEmpty { if (contentDetent == null) listOf("medium", "large") else emptyList() } + val contentOnly = contentDetent != null val allowsMedium = "medium" in detents - val allowsLarge = "large" in detents + val allowsLarge = "large" in detents || contentOnly val mediumOnly = allowsMedium && !allowsLarge var visible by remember { mutableStateOf(true) } @@ -3205,13 +3259,52 @@ private fun MobSheet(node: MobNode) { // half the measured viewport height in that case only — full // medium+large sheets size to their natural content height as usual. BoxWithConstraints(modifier = Modifier.fillMaxWidth()) { + val configuredMaximumHeight = contentDetent + ?.takeIf { detent -> detent.has("max_height") } + ?.optDouble("max_height") + ?.toFloat() + ?.dp + val contentMaximumHeight = configuredMaximumHeight + ?.let(maxHeight::coerceAtMost) + ?: maxHeight val mediumDetentModifier = if (mediumOnly) { Modifier.heightIn(min = maxHeight * 0.5f + 1.dp) } else { Modifier } - Column(modifier = mediumDetentModifier.then(contentModifier).fillMaxWidth()) { + // Only add our own scroll when the content doesn't already + // contain one. Compose's checkScrollableContainerConstraints + // THROWS when a scrollable is measured with an infinite max + // height, which is exactly what wrapping verticalScroll around a + // `scroll` or `lazy_list` child does — so an intrinsic sheet + // containing a list (the most natural use of one) crashed at first + // measure. iOS survives the equivalent nesting because SwiftUI + // tolerates it; Compose does not. Cap the height either way; let + // the child own the scrolling when it has its own. + val hasScrollableChild = node.children.any(::isScrollableNode) + val contentDetentModifier = when { + contentOnly && hasScrollableChild -> + Modifier.heightIn(max = contentMaximumHeight) + + contentOnly -> + Modifier + .heightIn(max = contentMaximumHeight) + .verticalScroll(rememberScrollState()) + + else -> Modifier + } + + Column( + // Cap BEFORE the node's own padding, so padding counts against + // max_height instead of being added outside it — otherwise a + // sheet with padding overshoots the documented cap, and + // disagrees with iOS, which caps the already-padded body. + modifier = mediumDetentModifier + .then(contentDetentModifier) + .then(contentModifier) + .fillMaxWidth() + ) { node.children.forEach { RenderNode(it) } } } @@ -3746,10 +3839,10 @@ private fun tabDefsProp(props: Map): List> { // pattern). The `is List<*>` branch stays as a fallback purely so a // directly-constructed MobNode (e.g. an instrumentation test building props // by hand instead of through JSON parsing) still works. -private fun detentsProp(props: Map): List = +private fun sheetDetentsProp(props: Map): List = when (val raw = props["detents"]) { - is JSONArray -> (0 until raw.length()).map { raw.getString(it) } - is List<*> -> raw.map { it.toString() } + is JSONArray -> (0 until raw.length()).map { raw.get(it) } + is List<*> -> raw else -> listOf("medium", "large") } diff --git a/test/mob_new/project_generator_test.exs b/test/mob_new/project_generator_test.exs index 7b2dbaa..6942616 100644 --- a/test/mob_new/project_generator_test.exs +++ b/test/mob_new/project_generator_test.exs @@ -787,6 +787,12 @@ defmodule MobNew.ProjectGeneratorTest do assert content =~ "BoxWithConstraints(modifier = Modifier.fillMaxWidth())" assert content =~ "maxHeight * 0.5f + 1.dp" + # Content detents wrap short bodies, cap at either the configured + # maximum or viewport, then scroll overflow internally. + assert content =~ ~S|detent.optString("type") == "content"| + assert content =~ "heightIn(max = contentMaximumHeight)" + assert content =~ "verticalScroll(rememberScrollState())" + # Sheet-owned background/corner_radius must not double-apply onto the # child content modifier (see the lint check for the structural guard). assert content =~ ~s|node.props - listOf("background", "corner_radius")| diff --git a/test/mob_new/templates/android_box_accessibility_test.exs b/test/mob_new/templates/android_box_accessibility_test.exs new file mode 100644 index 0000000..c336842 --- /dev/null +++ b/test/mob_new/templates/android_box_accessibility_test.exs @@ -0,0 +1,44 @@ +defmodule MobNew.Templates.AndroidBoxAccessibilityTest do + use ExUnit.Case, async: true + + @bridge Path.expand( + "../../../priv/templates/mob.new/android/app/src/main/java/MobBridge.kt.eex", + __DIR__ + ) + + test "generated boxes expose labels, action roles, and disabled semantics" do + source = File.read!(@bridge) + + assert source =~ "val isButton = node.type == \"box\" && accessibilityRole == \"button\"" + assert source =~ "isButton && tapHandle != null ->" + assert source =~ "clickable(enabled = !isDisabled, role = Role.Button)" + + # Compose publishes disabled() semantics whenever clickable's `enabled` is + # false, so "no handler" must not be encoded as enabled = false — that + # announced a live box as disabled. + refute source =~ "enabled = !isDisabled && tapHandle != null" + assert source =~ "val accessibilityModifier = Modifier.semantics(" + assert source =~ "mergeDescendants = accessibilityLabel != null" + assert source =~ "contentDescription = accessibilityLabel" + assert source =~ "if (isButton) role = Role.Button" + assert source =~ "if (isDisabled) disabled()" + + # A disabled box must not dispatch regardless of whether it carries an + # explicit button role. The non-button arm originally had no enabled + # check, so `disabled: true` without a role announced as disabled to + # TalkBack and still fired. + assert source =~ + "modifier.clickable(enabled = !isDisabled) { MobBridge.nativeSendTap(tapHandle) }" + + # Merge on label OR button role, matching iOS. Role without merging leaves + # children as separate accessibility nodes inside a "button". + assert source =~ "mergeDescendants = accessibilityLabel != null || isButton" + end + + test "only boxes with an explicit button role receive button semantics" do + source = File.read!(@bridge) + + assert source =~ "if (isButton)" + assert source =~ "tapHandle != null && node.type != \"button\"" + end +end diff --git a/test/mob_new/templates/android_content_sheet_test.exs b/test/mob_new/templates/android_content_sheet_test.exs new file mode 100644 index 0000000..9ab23ab --- /dev/null +++ b/test/mob_new/templates/android_content_sheet_test.exs @@ -0,0 +1,45 @@ +defmodule MobNew.Templates.AndroidContentSheetTest do + use ExUnit.Case, async: true + + @bridge Path.expand( + "../../../priv/templates/mob.new/android/app/src/main/java/MobBridge.kt.eex", + __DIR__ + ) + + test "Compose sheet preserves built-ins and recognizes encoded content detents" do + source = File.read!(@bridge) + + assert source =~ ~S|detent.optString("type") == "content"| + assert source =~ "val contentOnly = contentDetent != null" + assert source =~ "skipPartiallyExpanded = !allowsMedium" + assert source =~ "allowsLarge || value != SheetValue.Expanded" + end + + test "content sheet wraps naturally, caps height, and scrolls overflow" do + source = File.read!(@bridge) + + assert source =~ ~S|detent.has("max_height")| + assert source =~ ~S|?.optDouble("max_height")| + assert source =~ "let(maxHeight::coerceAtMost)" + assert source =~ "heightIn(max = contentMaximumHeight)" + assert source =~ "verticalScroll(rememberScrollState())" + end + + test "a content sheet does not nest its own scroll around a scrollable child" do + source = File.read!(@bridge) + + # Compose THROWS when a scrollable is measured with an infinite max height, + # so wrapping verticalScroll around a `scroll`/`lazy_list` child crashed the + # app at first measure. The cap still applies; the child owns the scrolling. + assert source =~ "val hasScrollableChild = node.children.any(::isScrollableNode)" + assert source =~ "contentOnly && hasScrollableChild ->" + assert source =~ "node.type == \"scroll\"" + assert source =~ "node.type == \"lazy_list\"" + + # The height cap must sit inside the node's own padding, or padding is + # added outside max_height and overshoots the documented cap. + detent_at = :binary.match(source, ".then(contentDetentModifier)") |> elem(0) + padding_at = :binary.match(source, ".then(contentModifier)") |> elem(0) + assert detent_at < padding_at, "height cap must be applied before the node's padding" + end +end