From 0b160d07c7b25e09e6cb0de9ce56edff22851bcd Mon Sep 17 00:00:00 2001 From: Alex Hart Date: Wed, 9 Sep 2026 10:34:08 -0300 Subject: [PATCH] Fix several keyboard issues. --- .../mediakeyboard/MediaKeyboardController.kt | 2 +- .../mediakeyboard/MediaKeyboardScaffold.kt | 31 ++++++++----- .../conversation/v2/ChatInputController.kt | 33 ++++++++++--- .../conversation/v2/ConversationFragment.kt | 46 +++++++++++++++++-- .../securesms/util/ViewExtensions.kt | 18 ++++++++ 5 files changed, 108 insertions(+), 22 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/compose/mediakeyboard/MediaKeyboardController.kt b/app/src/main/java/org/thoughtcrime/securesms/components/compose/mediakeyboard/MediaKeyboardController.kt index 2eb1dced2b..908ce679d6 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/compose/mediakeyboard/MediaKeyboardController.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/compose/mediakeyboard/MediaKeyboardController.kt @@ -30,7 +30,7 @@ class MediaKeyboardController(initialKeyboardHeightPx: Int = 0) { var current: MediaKeyboardKey? by mutableStateOf(null) private set - /** Whether the system keyboard is up, as reported by the IME inset. */ + /** Whether the system keyboard is up, as reported by the target of the IME inset animation. */ var isSystemKeyboardVisible: Boolean by mutableStateOf(false) internal set diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/compose/mediakeyboard/MediaKeyboardScaffold.kt b/app/src/main/java/org/thoughtcrime/securesms/components/compose/mediakeyboard/MediaKeyboardScaffold.kt index f530677626..111358d352 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/compose/mediakeyboard/MediaKeyboardScaffold.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/compose/mediakeyboard/MediaKeyboardScaffold.kt @@ -19,9 +19,8 @@ import androidx.compose.foundation.layout.displayCutout import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height -import androidx.compose.foundation.layout.imeAnimationSource +import androidx.compose.foundation.layout.ime import androidx.compose.foundation.layout.imeAnimationTarget -import androidx.compose.foundation.layout.isImeVisible import androidx.compose.foundation.layout.navigationBarsPadding import androidx.compose.foundation.layout.onConsumedWindowInsetsChanged import androidx.compose.foundation.layout.safeDrawing @@ -37,6 +36,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.Immutable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.SideEffect +import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf @@ -58,7 +58,7 @@ import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import kotlinx.coroutines.delay import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.drop +import kotlinx.coroutines.flow.dropWhile import kotlinx.coroutines.flow.filter import kotlinx.coroutines.launch import org.signal.core.ui.getWindowSizeClass @@ -108,7 +108,7 @@ fun MediaKeyboardScaffold( val registry = remember(keyboardsProvider) { MediaKeyboardRegistry().apply(keyboardsProvider) } val density = LocalDensity.current - val imeAnimationSource = WindowInsets.imeAnimationSource + val imeInsets = WindowInsets.ime val imeAnimationTarget = WindowInsets.imeAnimationTarget val windowHeightPx = LocalWindowInfo.current.containerSize.height @@ -121,9 +121,16 @@ fun MediaKeyboardScaffold( val activeKey = controller.current?.takeIf { registry.isEnabled(it) } - // The target state, so it does not read as hidden for the whole closing animation. - val systemKeyboardVisible = WindowInsets.isImeVisible - val systemKeyboardAnimating = imeAnimationSource.getBottom(density) != imeAnimationTarget.getBottom(density) + // The target, so a hide reads as gone the moment it is asked for rather than a whole animation later. + val systemKeyboardVisible = imeAnimationTarget.getBottom(density) > 0 + + // Whether the live inset has yet to catch up with the target. Deliberately not imeAnimationSource, + // which the platform leaves behind whenever an animation ends without a duration to run down, and + // which would then read as animating until the next keyboard came and went. Derived, so the frames + // the live inset walks through are not each a recomposition. + val systemKeyboardAnimating by remember(imeInsets, imeAnimationTarget, density) { + derivedStateOf { imeInsets.getBottom(density) != imeAnimationTarget.getBottom(density) } + } // Written together, so nothing waiting on the controller can see a keyboard that is neither // visible nor still animating out. @@ -162,11 +169,13 @@ fun MediaKeyboardScaffold( } } - LaunchedEffect(imeAnimationSource, imeAnimationTarget, density) { - snapshotFlow { imeAnimationSource.getBottom(density) == imeAnimationTarget.getBottom(density) } + // dropWhile rather than drop, so a composition that starts mid-animation still reports the settle + // it is in the middle of instead of swallowing it as the initial state. + LaunchedEffect(imeInsets, imeAnimationTarget, density) { + snapshotFlow { imeInsets.getBottom(density) == imeAnimationTarget.getBottom(density) } .distinctUntilChanged() + .dropWhile { settled -> settled } .filter { settled -> settled } - .drop(1) .collect { controller.awaitingSystemKeyboard = false onEvent(MediaKeyboardEvents.SystemKeyboardAnimationEnded) @@ -232,7 +241,7 @@ fun MediaKeyboardScaffold( } val systemKeyboardTakingOverSpace = activeKey == null && - (controller.awaitingSystemKeyboard || (imeAnimationTarget.getBottom(density) > 0 && systemKeyboardAnimating)) + (controller.awaitingSystemKeyboard || (systemKeyboardVisible && systemKeyboardAnimating)) val claimedBottomPx = { if (activeKey != null) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ChatInputController.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ChatInputController.kt index 4b6dffd51e..df14bad5d1 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ChatInputController.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ChatInputController.kt @@ -6,10 +6,17 @@ package org.thoughtcrime.securesms.conversation.v2 import android.content.Context +import android.view.View import android.widget.EditText +import kotlinx.coroutines.withTimeoutOrNull import org.thoughtcrime.securesms.components.compose.mediakeyboard.MediaKeyboardController import org.thoughtcrime.securesms.components.compose.mediakeyboard.MediaKeyboardKey import org.thoughtcrime.securesms.util.ViewUtil +import org.thoughtcrime.securesms.util.awaitAfterNextLayout +import kotlin.time.Duration.Companion.milliseconds + +/** Longest a keyboard's exit is waited on. Comfortably past the platform's own hide animation. */ +private val SETTLE_TIMEOUT = 500.milliseconds /** * Adapts [MediaKeyboardController] to the conversation's view code, which asks for keyboards from @@ -74,9 +81,10 @@ class ChatInputController( listeners.toList().forEach { it.onInputHidden() } } - fun showSoftkey(editText: EditText) { + /** @param imeTarget The field to bring the system keyboard up for, which need not be the input panel's. */ + fun showSoftkey(imeTarget: View) { controller.hideForSystemKeyboard() - ViewUtil.focusAndShowKeyboard(editText) + ViewUtil.focusAndShowKeyboard(imeTarget) } fun hideAll(imeTarget: EditText) { @@ -90,10 +98,12 @@ class ChatInputController( controller.hide() } + /** + * Unconditional, since [isKeyboardShowing] only knows about keyboards that claim space. A floating + * one reports no inset to read it from and still needs putting away. + */ fun hideKeyboard(imeTarget: EditText) { - if (isKeyboardShowing) { - ViewUtil.hideKeyboard(context, imeTarget) - } + ViewUtil.hideKeyboard(context, imeTarget) } fun runAfterAllHidden(imeTarget: EditText, onHidden: () -> Unit) { @@ -127,14 +137,23 @@ class ChatInputController( * Like [runAfterAllHidden], but suspends until the keyboards have finished animating out rather * than returning as soon as the hide has been asked for. For callers that measure themselves * against the content area, which stays shrunk for the length of that animation. + * + * Gives up after [SETTLE_TIMEOUT]. A keyboard that never reports its exit should leave a caller + * measuring against a stale content area, not stranded. + * + * @param contentView The area the keyboards resize. The settle itself lands in the middle of an + * inset dispatch, a frame before the space is handed back, so this is waited on as well. */ - suspend fun hideAllAndAwaitSettled(imeTarget: EditText) { + suspend fun hideAllAndAwaitSettled(imeTarget: EditText, contentView: View) { if (controller.isSettled) { return } hideAll(imeTarget) - controller.awaitSettled() + withTimeoutOrNull(SETTLE_TIMEOUT) { + controller.awaitSettled() + contentView.awaitAfterNextLayout() + } } /** diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationFragment.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationFragment.kt index aadb36214b..02bf5cc93d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationFragment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationFragment.kt @@ -42,6 +42,7 @@ import android.view.ViewTreeObserver import android.view.WindowManager import android.view.animation.AnimationUtils import android.view.inputmethod.EditorInfo +import android.widget.EditText import android.widget.ImageButton import android.widget.ImageView import android.widget.TextView @@ -98,6 +99,7 @@ import io.reactivex.rxjava3.disposables.Disposable import io.reactivex.rxjava3.kotlin.subscribeBy import io.reactivex.rxjava3.schedulers.Schedulers import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -670,6 +672,23 @@ class ConversationFragment : private val scheduledMessagesStub: Stub by lazy { Stub(binding.scheduledMessagesStub) } + /** + * The long press waiting on the keyboards to clear before the overlay can measure itself. The list + * has to stop taking taps for that whole wait, or a tap lands on a message that is about to be + * covered by the overlay. + */ + private var pendingReactionOverlayJob: Job? = null + + private val isReactionOverlayPending: Boolean + get() = pendingReactionOverlayJob?.isActive == true + + /** Swallows list touches for the length of [pendingReactionOverlayJob]. */ + private val pendingReactionOverlayTouchGuard = object : RecyclerView.OnItemTouchListener { + override fun onInterceptTouchEvent(recyclerView: RecyclerView, event: MotionEvent): Boolean = isReactionOverlayPending + override fun onTouchEvent(recyclerView: RecyclerView, event: MotionEvent) = Unit + override fun onRequestDisallowInterceptTouchEvent(disallowIntercept: Boolean) = Unit + } + private val reactionDelegate: ConversationReactionDelegate by lazy(LazyThreadSafetyMode.NONE) { val conversationReactionStub = Stub(binding.conversationReactionScrubberStub) val delegate = ConversationReactionDelegate(conversationReactionStub) @@ -900,6 +919,10 @@ class ConversationFragment : override fun onPause() { super.onPause() + // Abandoned rather than resumed on the way back in, where the long press is no longer the last + // thing the user did. + pendingReactionOverlayJob?.cancel() + ConversationUtil.refreshRecipientShortcuts() if (!args.conversationScreenType.isInBubble) { @@ -2319,6 +2342,7 @@ class ConversationFragment : binding.conversationItemRecycler.layoutManager = layoutManager scrollListener = ScrollListener() binding.conversationItemRecycler.addOnScrollListener(scrollListener!!) + binding.conversationItemRecycler.addOnItemTouchListener(pendingReactionOverlayTouchGuard) adapter = ConversationAdapterV2( lifecycleOwner = viewLifecycleOwner, @@ -3400,6 +3424,17 @@ class ConversationFragment : return isScrolledToBottom() || layoutManager.findFirstVisibleItemPosition() <= 0 } + /** + * The open search view's text field, or null when search is closed. Focusing the [SearchView] itself + * does not bring the keyboard up, since the keyboard follows the focused field rather than its host. + */ + private fun expandedSearchField(): EditText? { + return searchMenuItem + ?.takeIf { it.isActionViewExpanded } + ?.actionView + ?.findViewById(androidx.appcompat.R.id.search_src_text) + } + private fun closeChatSearch() { isSearchRequested = false searchViewModel.onSearchClosed() @@ -4132,8 +4167,9 @@ class ConversationFragment : // The overlay sizes itself to the content area, so every keyboard has to be all the way // out before it measures. Mid-animation it has half a screen to fit the menu into. - viewLifecycleOwner.lifecycleScope.launch { - container.hideAllAndAwaitSettled(composeText) + pendingReactionOverlayJob?.cancel() + pendingReactionOverlayJob = viewLifecycleOwner.lifecycleScope.launch { + container.hideAllAndAwaitSettled(composeText, conversationContent) showReactionOverlay(itemView, item, target, focusedView) } } @@ -4230,7 +4266,11 @@ class ConversationFragment : multiselectItemDecoration.hideShade(binding.conversationItemRecycler) ViewUtil.fadeOut(binding.reactionsShade, resources.getInteger(R.integer.reaction_scrubber_hide_duration), View.GONE) - if (focusedView == composeText || searchMenuItem?.isActionViewExpanded == true) { + val searchField = expandedSearchField() + if (searchField != null && focusedView == searchField) { + // The input panel is gone while search is open, so composeText cannot take the keyboard back. + container.showSoftkey(searchField) + } else if (focusedView == composeText) { container.showSoftkey(composeText) } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/ViewExtensions.kt b/app/src/main/java/org/thoughtcrime/securesms/util/ViewExtensions.kt index 65f4a7b063..852f2a8d8b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/ViewExtensions.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/util/ViewExtensions.kt @@ -11,6 +11,8 @@ import androidx.core.view.doOnNextLayout import androidx.fragment.app.Fragment import androidx.fragment.app.findFragment import androidx.lifecycle.Lifecycle +import kotlinx.coroutines.suspendCancellableCoroutine +import kotlin.coroutines.resume var View.visible: Boolean get() { @@ -48,6 +50,22 @@ inline fun View.doAfterNextLayout(crossinline action: () -> Unit) { } } +/** + * The suspending form of [doAfterNextLayout]. Resumes once the traversal that laid this view out has + * finished, so a caller both reads the size it was just handed and is free to touch the hierarchy. + */ +suspend fun View.awaitAfterNextLayout(): Unit = suspendCancellableCoroutine { continuation -> + val listener = object : View.OnLayoutChangeListener { + override fun onLayoutChange(view: View, left: Int, top: Int, right: Int, bottom: Int, oldLeft: Int, oldTop: Int, oldRight: Int, oldBottom: Int) { + view.removeOnLayoutChangeListener(this) + view.post { continuation.resume(Unit) } + } + } + + addOnLayoutChangeListener(listener) + continuation.invokeOnCancellation { removeOnLayoutChangeListener(listener) } +} + fun TextView.setRelativeDrawables( @DrawableRes start: Int = 0, @DrawableRes top: Int = 0,