From 82ee4bf7e5c73e326ebb14af3bc86f7fceb30c6a Mon Sep 17 00:00:00 2001 From: agent-p1p Date: Sat, 18 Jul 2026 22:32:03 +0200 Subject: [PATCH] fix: preserve keyboard for message sheets Move edit history and message info onto the shared non-focusable popup so opening them preserves composer focus and the IME. Keep visual scrim and modal accessibility semantics, and make shared Back dismissal run before the IME callback. Fixes #1396 --- .../ui/conversation/messages/EditHistory.kt | 12 +-- .../messages/MessageFullScreen.kt | 12 +-- .../whitenoise/android/ui/design/AppSheets.kt | 88 +++++++++++++++++++ .../android/ui/design/KeyboardSafePopup.kt | 32 +++++-- ...yboardPreservingBottomSheetCoverageTest.kt | 82 +++++++++++++++++ .../design/KeyboardSafePopupCoverageTest.kt | 21 ++++- 6 files changed, 218 insertions(+), 29 deletions(-) create mode 100644 app/src/main/java/dev/ipf/whitenoise/android/ui/design/AppSheets.kt create mode 100644 app/src/test/java/dev/ipf/whitenoise/android/ui/KeyboardPreservingBottomSheetCoverageTest.kt diff --git a/app/src/main/java/dev/ipf/whitenoise/android/ui/conversation/messages/EditHistory.kt b/app/src/main/java/dev/ipf/whitenoise/android/ui/conversation/messages/EditHistory.kt index 4676f2118..1335aa3de 100644 --- a/app/src/main/java/dev/ipf/whitenoise/android/ui/conversation/messages/EditHistory.kt +++ b/app/src/main/java/dev/ipf/whitenoise/android/ui/conversation/messages/EditHistory.kt @@ -18,12 +18,9 @@ import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.shape.CircleShape import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.foundation.verticalScroll -import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.MaterialTheme -import androidx.compose.material3.ModalBottomSheet import androidx.compose.material3.Surface import androidx.compose.material3.Text -import androidx.compose.material3.rememberModalBottomSheetState import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.remember @@ -36,7 +33,7 @@ import androidx.compose.ui.unit.dp import dev.ipf.whitenoise.android.R import dev.ipf.whitenoise.android.core.EditState import dev.ipf.whitenoise.android.ui.common.rememberedRelativeTime -import dev.ipf.whitenoise.android.ui.theme.amoledSheetContainerColor +import dev.ipf.whitenoise.android.ui.design.KeyboardPreservingBottomSheet import dev.ipf.whitenoise.android.ui.theme.amoledSurfaceBorderStroke private data class EditHistoryRow( @@ -47,7 +44,6 @@ private data class EditHistoryRow( val isOriginal: Boolean, ) -@OptIn(ExperimentalMaterial3Api::class) @Composable internal fun EditHistorySheet( original: String, @@ -55,7 +51,6 @@ internal fun EditHistorySheet( editState: EditState, onDismissRequest: () -> Unit, ) { - val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true) // Newest first reads as "this is what's shown now ← earlier revisions ← original". val rows = remember(original, originalTimestamp, editState) { @@ -83,10 +78,9 @@ internal fun EditHistorySheet( ) } } - ModalBottomSheet( + KeyboardPreservingBottomSheet( + paneTitle = stringResource(R.string.edit_history), onDismissRequest = onDismissRequest, - sheetState = sheetState, - containerColor = amoledSheetContainerColor(), ) { // The header is anchored above the scroll region so the title and // count chip remain visible while the user pages through a long edit diff --git a/app/src/main/java/dev/ipf/whitenoise/android/ui/conversation/messages/MessageFullScreen.kt b/app/src/main/java/dev/ipf/whitenoise/android/ui/conversation/messages/MessageFullScreen.kt index c9caafc5a..9d4984145 100644 --- a/app/src/main/java/dev/ipf/whitenoise/android/ui/conversation/messages/MessageFullScreen.kt +++ b/app/src/main/java/dev/ipf/whitenoise/android/ui/conversation/messages/MessageFullScreen.kt @@ -9,7 +9,6 @@ import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height -import androidx.compose.foundation.layout.navigationBarsPadding import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.rememberScrollState @@ -30,12 +29,10 @@ import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme import androidx.compose.material3.MenuDefaults -import androidx.compose.material3.ModalBottomSheet import androidx.compose.material3.Scaffold import androidx.compose.material3.Surface import androidx.compose.material3.Text import androidx.compose.material3.TopAppBar -import androidx.compose.material3.rememberModalBottomSheetState import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf @@ -61,7 +58,7 @@ import dev.ipf.whitenoise.android.state.labelFor import dev.ipf.whitenoise.android.state.shortHex import dev.ipf.whitenoise.android.state.shouldShowOriginalTimestamp import dev.ipf.whitenoise.android.ui.common.Avatar -import dev.ipf.whitenoise.android.ui.theme.amoledSheetContainerColor +import dev.ipf.whitenoise.android.ui.design.KeyboardPreservingBottomSheet import dev.ipf.whitenoise.android.ui.theme.amoledSurfaceBorderStroke import java.time.ZoneId import java.util.Locale @@ -238,7 +235,6 @@ internal fun MessageFullScreenView( } } -@OptIn(ExperimentalMaterial3Api::class) @Composable internal fun MessageInfoSheet( item: TimelineMessage, @@ -290,16 +286,14 @@ internal fun MessageInfoSheet( val messageIdShort = shortHex(record.messageIdHex) val copyActionLabel = stringResource(R.string.copy_text) - ModalBottomSheet( - containerColor = amoledSheetContainerColor(), + KeyboardPreservingBottomSheet( + paneTitle = stringResource(R.string.message_info), onDismissRequest = onDismissRequest, - sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true), ) { Column( modifier = Modifier .fillMaxWidth() - .navigationBarsPadding() .padding(horizontal = 20.dp, vertical = 8.dp), verticalArrangement = Arrangement.spacedBy(12.dp), ) { diff --git a/app/src/main/java/dev/ipf/whitenoise/android/ui/design/AppSheets.kt b/app/src/main/java/dev/ipf/whitenoise/android/ui/design/AppSheets.kt new file mode 100644 index 000000000..850124cf7 --- /dev/null +++ b/app/src/main/java/dev/ipf/whitenoise/android/ui/design/AppSheets.kt @@ -0,0 +1,88 @@ +package dev.ipf.whitenoise.android.ui.design + +import androidx.compose.foundation.background +import androidx.compose.foundation.gestures.detectTapGestures +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.ColumnScope +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.navigationBarsPadding +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.widthIn +import androidx.compose.material3.BottomSheetDefaults +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.Surface +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.input.pointer.pointerInput +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.Role +import androidx.compose.ui.semantics.contentDescription +import androidx.compose.ui.semantics.dialog +import androidx.compose.ui.semantics.isTraversalGroup +import androidx.compose.ui.semantics.onClick +import androidx.compose.ui.semantics.paneTitle +import androidx.compose.ui.semantics.role +import androidx.compose.ui.semantics.semantics +import androidx.compose.ui.unit.dp +import dev.ipf.whitenoise.android.R +import dev.ipf.whitenoise.android.ui.theme.amoledSheetContainerColor + +/** + * Bottom-anchored sheet that leaves focus in its host window. + * + * Material's [androidx.compose.material3.ModalBottomSheet] opens a focusable dialog window. From a + * focused conversation that steals window focus from the composer and Android closes the IME. + * [KeyboardSafePopup] supplies the shared non-focusable popup, outside-tap consumption, and + * overlay-priority Back dismissal. This wrapper adds the sheet styling and accessibility semantics. + */ +@OptIn(ExperimentalMaterial3Api::class) +@Composable +internal fun KeyboardPreservingBottomSheet( + paneTitle: String, + onDismissRequest: () -> Unit, + modifier: Modifier = Modifier, + content: @Composable ColumnScope.() -> Unit, +) { + val dismissLabel = stringResource(R.string.dismiss) + KeyboardSafePopup( + expanded = true, + onDismissRequest = onDismissRequest, + popupPositionProvider = BottomAnchoredPopupPositionProvider, + scrimModifier = + Modifier + .background(BottomSheetDefaults.ScrimColor) + .semantics { + contentDescription = dismissLabel + role = Role.Button + onClick(label = dismissLabel) { + onDismissRequest() + true + } + }, + ) { + Surface( + modifier = + Modifier + .then(modifier) + .widthIn(max = BottomSheetDefaults.SheetMaxWidth) + .fillMaxWidth() + .semantics { + isTraversalGroup = true + dialog() + this.paneTitle = paneTitle + }.pointerInput(Unit) { + detectTapGestures { /* Consume blank sheet taps without dismissing. */ } + }, + shape = BottomSheetDefaults.ExpandedShape, + color = amoledSheetContainerColor(), + ) { + Column( + modifier = + Modifier + .navigationBarsPadding() + .padding(top = 8.dp), + content = content, + ) + } + } +} diff --git a/app/src/main/java/dev/ipf/whitenoise/android/ui/design/KeyboardSafePopup.kt b/app/src/main/java/dev/ipf/whitenoise/android/ui/design/KeyboardSafePopup.kt index 620610687..3dedcfc60 100644 --- a/app/src/main/java/dev/ipf/whitenoise/android/ui/design/KeyboardSafePopup.kt +++ b/app/src/main/java/dev/ipf/whitenoise/android/ui/design/KeyboardSafePopup.kt @@ -1,12 +1,18 @@ package dev.ipf.whitenoise.android.ui.design +import android.window.OnBackInvokedCallback +import android.window.OnBackInvokedDispatcher import androidx.activity.compose.BackHandler import androidx.compose.foundation.gestures.detectTapGestures import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.runtime.Composable +import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.rememberUpdatedState import androidx.compose.ui.Modifier import androidx.compose.ui.input.pointer.pointerInput +import androidx.compose.ui.platform.LocalView import androidx.compose.ui.unit.IntOffset import androidx.compose.ui.unit.IntRect import androidx.compose.ui.unit.IntSize @@ -28,9 +34,9 @@ import androidx.compose.ui.window.PopupProperties // explicitly here, without re-focusing (which would collapse the IME again): // 1. Back dismissal — Popup's dismissOnBackPress is a no-op while the popup is // non-focusable, so a Back press would fall through to the IME/activity -// instead of closing the overlay. A host-window BackHandler (same pattern -// as QuickActionFabMenu) closes it on Back. It runs in the conversation -// window and does not touch IME focus. +// instead of closing the overlay. A host-window overlay-priority callback +// closes it before the IME's default-priority callback. Preview/test hosts +// without a platform dispatcher fall back to BackHandler. // 2. Outside-tap click-through — events outside a non-focusable popup are // delivered to the windows beneath it, so a dismiss tap would also activate // the underlying chat content (open a profile, a link, the media viewer, @@ -86,29 +92,41 @@ internal fun KeyboardSafePopup( expanded: Boolean, onDismissRequest: () -> Unit, popupPositionProvider: PopupPositionProvider, + scrimModifier: Modifier = Modifier, content: @Composable () -> Unit, ) { + val currentOnDismissRequest by rememberUpdatedState(onDismissRequest) Box { if (!expanded) return@Box - BackHandler(enabled = true) { onDismissRequest() } + val backDispatcher = LocalView.current.findOnBackInvokedDispatcher() + DisposableEffect(backDispatcher) { + if (backDispatcher == null) return@DisposableEffect onDispose {} + val callback = OnBackInvokedCallback { currentOnDismissRequest() } + backDispatcher.registerOnBackInvokedCallback(OnBackInvokedDispatcher.PRIORITY_OVERLAY, callback) + onDispose { backDispatcher.unregisterOnBackInvokedCallback(callback) } + } + if (backDispatcher == null) { + BackHandler(enabled = true) { currentOnDismissRequest() } + } // Scrim popup: composed before the content popup so content renders on top. // Fills the window and swallows any tap as a pure dismissal. Popup( properties = keyboardSafePopupProperties, - onDismissRequest = onDismissRequest, + onDismissRequest = currentOnDismissRequest, ) { Box( modifier = Modifier .fillMaxSize() + .then(scrimModifier) .pointerInput(Unit) { - detectTapGestures { onDismissRequest() } + detectTapGestures { currentOnDismissRequest() } }, ) } Popup( popupPositionProvider = popupPositionProvider, - onDismissRequest = onDismissRequest, + onDismissRequest = currentOnDismissRequest, properties = keyboardSafePopupProperties, content = content, ) diff --git a/app/src/test/java/dev/ipf/whitenoise/android/ui/KeyboardPreservingBottomSheetCoverageTest.kt b/app/src/test/java/dev/ipf/whitenoise/android/ui/KeyboardPreservingBottomSheetCoverageTest.kt new file mode 100644 index 000000000..41f3588bc --- /dev/null +++ b/app/src/test/java/dev/ipf/whitenoise/android/ui/KeyboardPreservingBottomSheetCoverageTest.kt @@ -0,0 +1,82 @@ +package dev.ipf.whitenoise.android.ui + +import dev.ipf.whitenoise.android.functionBody +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import java.io.File + +class KeyboardPreservingBottomSheetCoverageTest { + @Test + fun bottomSheetDelegatesOverlayPlumbingToKeyboardSafePopup() { + val source = appSheetsSource().readText() + val body = source.functionBody("KeyboardPreservingBottomSheet") + + assertTrue("sheet must use the shared non-focusable overlay", "KeyboardSafePopup(" in body) + assertTrue( + "sheet must anchor above the visible-window bottom, including an open IME", + "popupPositionProvider = BottomAnchoredPopupPositionProvider" in body, + ) + assertFalse("sheet wrapper must not duplicate Popup plumbing", Regex("""\bPopup\(""").containsMatchIn(body)) + assertFalse("sheet wrapper must not duplicate Back handling", "BackHandler(" in body || "OnBackInvoked" in body) + } + + @Test + fun bottomSheetAddsVisualAndAccessibleModalSemantics() { + val body = appSheetsSource().readText().functionBody("KeyboardPreservingBottomSheet") + + assertTrue( + "shared dismissal scrim must remain visible and accessible", + "scrimModifier" in body && + ".background(BottomSheetDefaults.ScrimColor)" in body && + "contentDescription = dismissLabel" in body && + "role = Role.Button" in body && + "onClick(label = dismissLabel)" in body, + ) + assertTrue( + "sheet content must retain dialog, traversal, and pane semantics", + "isTraversalGroup = true" in body && + "dialog()" in body && + "this.paneTitle = paneTitle" in body, + ) + assertTrue( + "sheet must retain Material sizing, shape, and navigation-bar padding", + "BottomSheetDefaults.SheetMaxWidth" in body && + "BottomSheetDefaults.ExpandedShape" in body && + ".navigationBarsPadding()" in body, + ) + } + + @Test + fun remainingMessageSheetsUseKeyboardPreservingContainer() { + listOf( + editHistorySource().readText().functionBody("EditHistorySheet"), + messageFullScreenSource().readText().functionBody("MessageInfoSheet"), + ).forEach { body -> + assertTrue("conversation sheet must use KeyboardPreservingBottomSheet", "KeyboardPreservingBottomSheet(" in body) + assertFalse("conversation sheet must not open a focus-stealing ModalBottomSheet", "ModalBottomSheet(" in body) + } + } + + @Test + fun forwardSheetKeepsItsFocusableModalForSearch() { + val body = messageActionsSource().readText().functionBody("ForwardMessageSheet") + + assertTrue("the forward flow still needs a focusable ModalBottomSheet for its search field", "ModalBottomSheet(" in body) + } + + private fun appSheetsSource(): File = source("ui/design/AppSheets.kt") + + private fun editHistorySource(): File = source("ui/conversation/messages/EditHistory.kt") + + private fun messageFullScreenSource(): File = source("ui/conversation/messages/MessageFullScreen.kt") + + private fun messageActionsSource(): File = source("ui/conversation/messages/MessageActions.kt") + + private fun source(relativePath: String): File = + listOf( + File("src/main/java/dev/ipf/whitenoise/android/$relativePath"), + File("app/src/main/java/dev/ipf/whitenoise/android/$relativePath"), + ).firstOrNull { it.exists() } + ?: error("Missing $relativePath source file") +} diff --git a/app/src/test/java/dev/ipf/whitenoise/android/ui/design/KeyboardSafePopupCoverageTest.kt b/app/src/test/java/dev/ipf/whitenoise/android/ui/design/KeyboardSafePopupCoverageTest.kt index c3d811de6..a0316901f 100644 --- a/app/src/test/java/dev/ipf/whitenoise/android/ui/design/KeyboardSafePopupCoverageTest.kt +++ b/app/src/test/java/dev/ipf/whitenoise/android/ui/design/KeyboardSafePopupCoverageTest.kt @@ -37,7 +37,7 @@ class KeyboardSafePopupCoverageTest { ) assertTrue( "scrim must consume outside taps instead of letting them click through", - "detectTapGestures { onDismissRequest() }" in body, + "detectTapGestures { currentOnDismissRequest() }" in body, ) assertEquals( "scrim and content popups must share the same non-focusable properties", @@ -47,13 +47,26 @@ class KeyboardSafePopupCoverageTest { } @Test - fun keyboardSafePopupBackHandlerDismissesWhileExpanded() { + fun keyboardSafePopupDismissesBeforeTheImeBackCallback() { val body = keyboardSafePopupSource().readText().functionBody("KeyboardSafePopup") assertTrue( - "Back must dismiss via host-window BackHandler while non-focusable", - "BackHandler(enabled = true) { onDismissRequest() }" in body, + "Back must use overlay priority so the sheet closes before the IME", + "OnBackInvokedDispatcher.PRIORITY_OVERLAY" in body, ) + assertTrue( + "preview hosts without a platform dispatcher still need Back dismissal", + "BackHandler(enabled = true) { currentOnDismissRequest() }" in body, + ) + } + + @Test + fun keyboardSafePopupAcceptsCallerScrimStylingAndSemantics() { + val source = keyboardSafePopupSource().readText() + val body = source.functionBody("KeyboardSafePopup") + + assertTrue("scrim customization must remain optional", "scrimModifier: Modifier = Modifier" in source) + assertTrue("the full-window dismissal scrim must apply caller decoration", ".then(scrimModifier)" in body) } @Test