From c855e846f1b5c87d06f7d36deed1b1082b2d9932 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20Krzy=C5=BCanowski?= Date: Mon, 9 Nov 2026 07:28:35 +0100 Subject: [PATCH] fix(android): keep a closing sheet from reopening --- resources/android/BottomSheetRenderer.kt | 14 ++++-- tests/BottomSheetRendererTest.php | 19 +++++++- tests/android/README.md | 7 ++- tests/android/build.gradle.kts | 2 +- tests/android/src/main/kotlin/Bridge.kt | 1 + tests/android/src/main/kotlin/NodeView.kt | 4 ++ .../src/test/kotlin/BottomSheetCloseTest.kt | 47 +++++++++++++++++++ 7 files changed, 88 insertions(+), 6 deletions(-) create mode 100644 tests/android/src/test/kotlin/BottomSheetCloseTest.kt diff --git a/resources/android/BottomSheetRenderer.kt b/resources/android/BottomSheetRenderer.kt index 9bac9e9..c7aa577 100644 --- a/resources/android/BottomSheetRenderer.kt +++ b/resources/android/BottomSheetRenderer.kt @@ -51,7 +51,11 @@ object BottomSheetRenderer { val p = node.props val visible = p.getBool("visible") val onDismissCb = p.getCallbackId("on_dismiss") - val detentsStr = p.getString("detents", "medium,large") + // A closing sheet keeps the detents it opened with: new ones would get + // it a new SheetState, and ModalBottomSheet shows every new state. + val requestedDetents = p.getString("detents", "medium,large") + var detentsStr by remember { mutableStateOf(requestedDetents) } + if (visible) detentsStr = requestedDetents val a11yLabel = p.getString("a11y_label") // Counterpart of iOS `.interactiveDismissDisabled`: block drag-to-hide // and back-press, and swallow scrim-tap dismiss requests. NOTE: @@ -75,8 +79,12 @@ object BottomSheetRenderer { if (visible) { shown = true } else if (shown) { - if (!permanent) sheetState.hide() - shown = false + // A drag or M3's own show() cancels hide(); the sheet still goes. + try { + if (!permanent) sheetState.hide() + } finally { + shown = false + } } } diff --git a/tests/BottomSheetRendererTest.php b/tests/BottomSheetRendererTest.php index c259431..4e1aee4 100644 --- a/tests/BottomSheetRendererTest.php +++ b/tests/BottomSheetRendererTest.php @@ -19,10 +19,27 @@ expect($renderer) ->toBeString() ->not->toMatch('/if \(!visible\) return/') - ->toMatch('/LaunchedEffect\(visible\) \{.*?sheetState\.hide\(\)\s*shown = false/s') + ->toMatch('/LaunchedEffect\(visible\) \{.*?sheetState\.hide\(\).*?shown = false/s') ->toMatch('/if \(!shown\) return/'); }); +test('takes an Android bottom sheet away even when hiding it is cancelled', function () { + $renderer = file_get_contents(dirname(__DIR__).'/resources/android/BottomSheetRenderer.kt'); + + expect($renderer) + ->toBeString() + ->toMatch('/try \{\s*if \(!permanent\) sheetState\.hide\(\)\s*\} finally \{\s*shown = false\s*\}/'); +}); + +test('keeps the detents an Android bottom sheet opened with while it closes', function () { + $renderer = file_get_contents(dirname(__DIR__).'/resources/android/BottomSheetRenderer.kt'); + + expect($renderer) + ->toBeString() + ->toMatch('/var detentsStr by remember \{ mutableStateOf\(requestedDetents\) \}\s*if \(visible\) detentsStr = requestedDetents/') + ->not->toMatch('/val detentsStr = p\.getString/'); +}); + test('wraps an Android fit sheet around its content, opened in full', function () { $renderer = file_get_contents(dirname(__DIR__).'/resources/android/BottomSheetRenderer.kt'); diff --git a/tests/android/README.md b/tests/android/README.md index b2beeb7..c42eed5 100644 --- a/tests/android/README.md +++ b/tests/android/README.md @@ -2,7 +2,7 @@ This standalone Robolectric/Compose module compiles the production filled and outlined editors, their shared dispatcher, the production Button renderer and -the production gesture area, list, virtual list and pager renderers. +the production gesture area, list, virtual list, pager and bottom sheet renderers. It records the bridge event boundary; it does not load PHP/JNI, render icons or menus, or prove device IME behaviour. The Mobile policy and node models are read from an explicit checkout, not substituted by test implementations. @@ -48,4 +48,9 @@ the list doesn't repeat an applied request, disabling then restoring an index re-arms it, and saved-state restoration keeps the reader's position unless the request changed while it was saved. Assertions check which row is at the viewport's top, not just that a scroll was invoked. + +`BottomSheetCloseTest` closes an open sheet and checks its content leaves +composition: when its detents change in the same render, and when a finger +grabs it while it slides away. `RenderNode` here draws each child as +`NodeView` does. These are JVM checks; SwiftUI and native device behaviour need separate testing. diff --git a/tests/android/build.gradle.kts b/tests/android/build.gradle.kts index 05c4fc7..257bed6 100644 --- a/tests/android/build.gradle.kts +++ b/tests/android/build.gradle.kts @@ -5,7 +5,7 @@ plugins { } val stageSources = tasks.register("stageSources") { from(layout.projectDirectory.dir("../../resources/android")) { - include("FilledTextInputRenderer.kt", "OutlinedTextInputRenderer.kt", "BareTextInputRenderer.kt", "TextInputShared.kt", "NativeUIFontResolver.kt", "NativeUITheme.kt", "ButtonRenderer.kt", "GestureAreaRenderer.kt", "ListRenderer.kt", "VirtualListRenderer.kt", "PagerRenderer.kt") + include("FilledTextInputRenderer.kt", "OutlinedTextInputRenderer.kt", "BareTextInputRenderer.kt", "TextInputShared.kt", "NativeUIFontResolver.kt", "NativeUITheme.kt", "ButtonRenderer.kt", "GestureAreaRenderer.kt", "ListRenderer.kt", "VirtualListRenderer.kt", "PagerRenderer.kt", "BottomSheetRenderer.kt") } from(providers.gradleProperty("mobileRoot").map { "$it/resources/androidstudio/app/src/main/java/com/nativephp/mobile/ui/nativerender" }) { include("KeyboardFocusPolicy.kt", "NativeUINode.kt", "SharedValueStore.kt") diff --git a/tests/android/src/main/kotlin/Bridge.kt b/tests/android/src/main/kotlin/Bridge.kt index 73d040d..fe3d923 100644 --- a/tests/android/src/main/kotlin/Bridge.kt +++ b/tests/android/src/main/kotlin/Bridge.kt @@ -13,5 +13,6 @@ object NativeUIBridge { fun sendPressEvent(callbackId: Int, nodeId: Int) { events += "press:$callbackId" } fun sendSliderChangeEvent(callbackId: Int, nodeId: Int, value: Float) { events += "slider:$callbackId:$value" } fun sendTabChangeEvent(callbackId: Int, nodeId: Int, index: Int) { events += "tab:$callbackId:$index" } + fun sendSheetDismissEvent(callbackId: Int, nodeId: Int) { events += "dismiss:$callbackId" } } fun argbToComposeColor(argb: Int) = Color(argb) diff --git a/tests/android/src/main/kotlin/NodeView.kt b/tests/android/src/main/kotlin/NodeView.kt index c302bf6..7faa547 100644 --- a/tests/android/src/main/kotlin/NodeView.kt +++ b/tests/android/src/main/kotlin/NodeView.kt @@ -21,3 +21,7 @@ fun NodeView(node: NativeUINode, overrideModifier: Modifier? = null) { } Box(modifier) } + +/** Stands in for core's `RenderNode`, which the bottom sheet draws its children with. */ +@Composable +fun RenderNode(node: NativeUINode, modifier: Modifier? = null) = NodeView(node, modifier) diff --git a/tests/android/src/test/kotlin/BottomSheetCloseTest.kt b/tests/android/src/test/kotlin/BottomSheetCloseTest.kt new file mode 100644 index 0000000..85e26a4 --- /dev/null +++ b/tests/android/src/test/kotlin/BottomSheetCloseTest.kt @@ -0,0 +1,47 @@ +package com.nativephp.plugins.native_ui.ui + +import androidx.compose.material3.MaterialTheme +import androidx.compose.runtime.mutableStateOf +import androidx.compose.ui.Modifier +import androidx.compose.ui.geometry.Offset +import androidx.compose.ui.test.* +import androidx.compose.ui.test.junit4.createComposeRule +import com.nativephp.mobile.ui.nativerender.* +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [35]) +class BottomSheetCloseTest { + @get:Rule val compose = createComposeRule() + + private val sheet = mutableStateOf(sheet(visible = true, detents = "large")) + + @Before fun open() { + compose.setContent { MaterialTheme { BottomSheetRenderer.Render(sheet.value, Modifier) } } + compose.onNodeWithTag("node-2").assertExists() + } + + @Test fun aSheetClosedWithNewDetentsDoesNotOpenAgain() { + compose.runOnIdle { sheet.value = sheet(visible = false, detents = "medium") } + compose.waitForIdle() + compose.onNodeWithTag("node-2").assertDoesNotExist() + } + + @Test fun aSheetGrabbedWhileSlidingAwayStillLeaves() { + compose.mainClock.autoAdvance = false + compose.runOnIdle { sheet.value = sheet(visible = false, detents = "large") } + compose.mainClock.advanceTimeBy(100) + compose.onNodeWithTag("node-2").performTouchInput { down(center); moveBy(Offset(0f, -100f)) } + compose.mainClock.advanceTimeBy(1000) + compose.onNodeWithTag("node-2").assertDoesNotExist() + } + + private fun sheet(visible: Boolean, detents: String) = NativeUINode( + 1, "bottom_sheet", null, null, GenericProps(mapOf("visible" to visible, "detents" to detents, "on_dismiss" to 9)), 0, 0, + listOf(NativeUINode(2, "", null, null, GenericProps(), 0, 0, emptyList()))) +}