Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 103 additions & 10 deletions priv/templates/mob.new/android/app/src/main/java/MobBridge.kt.eex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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) }
}
Expand Down Expand Up @@ -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<JSONObject>()
.firstOrNull { detent -> detent.optString("type") == "content" }
val detents = rawDetents.filterIsInstance<String>()
.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) }
Expand Down Expand Up @@ -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) }
}
}
Expand Down Expand Up @@ -3746,10 +3839,10 @@ private fun tabDefsProp(props: Map<String, Any?>): List<Map<String, String>> {
// 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<String, Any?>): List<String> =
private fun sheetDetentsProp(props: Map<String, Any?>): List<Any?> =
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")
}

Expand Down
6 changes: 6 additions & 0 deletions test/mob_new/project_generator_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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")|
Expand Down
44 changes: 44 additions & 0 deletions test/mob_new/templates/android_box_accessibility_test.exs
Original file line number Diff line number Diff line change
@@ -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
45 changes: 45 additions & 0 deletions test/mob_new/templates/android_content_sheet_test.exs
Original file line number Diff line number Diff line change
@@ -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
Loading