From 78991a289e37d646c0dff005ff917cec2789dc2e Mon Sep 17 00:00:00 2001 From: Moe Hamade <69801237+moehamade@users.noreply.github.com> Date: Tue, 25 Aug 2026 23:52:52 +0300 Subject: [PATCH 1/3] fix(ui): register the chat back handler once instead of per recomposition The chat branch of OnboardingFlowScreen built an OnBackPressedCallback and called addCallback(this, ...) from inside a composable body. Composable bodies re-run on recomposition, and that function observes eight MainViewModel StateFlows, so every Bluetooth, location, or loading change registered another callback. They were bound to the activity, so none were released until onDestroy. Each accumulated callback also multiplied the work of an unhandled Back press: the callback disabled itself, re-dispatched, and the next one down consulted ChatViewModel again. Five recompositions meant one Back press consulted it five times before reaching the system. BackHandler keeps a single registration across recompositions and disposes it when the branch leaves composition. Driving its enabled flag from ChatState.canHandleBack also retires the disable/re-dispatch/re-enable dance: with nothing to unwind the handler is simply disabled, so the dispatcher falls through to the system. canHandleBack mirrors the branches of ChatViewModel.handleBackPressed and is covered by unit tests, since the handler being enabled and the press being consumed have to agree. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MdKgoKZbL5K1wsn1WsES26 --- .../java/com/bitchat/android/MainActivity.kt | 24 ++--- .../java/com/bitchat/android/ui/ChatState.kt | 22 +++++ .../com/bitchat/android/ui/ChatViewModel.kt | 1 + .../android/ui/ChatStateBackNavigationTest.kt | 93 +++++++++++++++++++ 4 files changed, 123 insertions(+), 17 deletions(-) create mode 100644 app/src/test/kotlin/com/bitchat/android/ui/ChatStateBackNavigationTest.kt diff --git a/app/src/main/java/com/bitchat/android/MainActivity.kt b/app/src/main/java/com/bitchat/android/MainActivity.kt index 6f80052a..8f0b84c1 100644 --- a/app/src/main/java/com/bitchat/android/MainActivity.kt +++ b/app/src/main/java/com/bitchat/android/MainActivity.kt @@ -4,7 +4,7 @@ import android.content.Intent import android.os.Build import android.os.Bundle import android.util.Log -import androidx.activity.OnBackPressedCallback +import androidx.activity.compose.BackHandler import androidx.activity.compose.setContent import androidx.activity.enableEdgeToEdge import androidx.activity.viewModels @@ -313,23 +313,13 @@ class MainActivity : OrientationAwareActivity() { } OnboardingState.CHECKING, OnboardingState.INITIALIZING, OnboardingState.COMPLETE -> { - // Set up back navigation handling for the chat screen - val backCallback = object : OnBackPressedCallback(true) { - override fun handleOnBackPressed() { - // Let ChatViewModel handle navigation state - val handled = chatViewModel.handleBackPressed() - if (!handled) { - // If ChatViewModel doesn't handle it, disable this callback - // and let the system handle it (which will exit the app) - this.isEnabled = false - onBackPressedDispatcher.onBackPressed() - this.isEnabled = true - } - } + // Intercept Back only while the chat has navigation state to + // unwind. Staying disabled otherwise lets the dispatcher fall + // through to the system, which exits the app. + val canHandleBack by chatViewModel.canHandleBack.collectAsState() + BackHandler(enabled = canHandleBack) { + chatViewModel.handleBackPressed() } - - // Add the callback - this will be automatically removed when the activity is destroyed - onBackPressedDispatcher.addCallback(this, backCallback) ChatScreen(viewModel = chatViewModel) } diff --git a/app/src/main/java/com/bitchat/android/ui/ChatState.kt b/app/src/main/java/com/bitchat/android/ui/ChatState.kt index f4504cc2..e0790894 100644 --- a/app/src/main/java/com/bitchat/android/ui/ChatState.kt +++ b/app/src/main/java/com/bitchat/android/ui/ChatState.kt @@ -170,6 +170,28 @@ class ChatState( initialValue = false ) + // True while some in-app navigation state is open for Back to unwind. + // Mirrors the branches of ChatViewModel.handleBackPressed: the back + // handler is enabled from this, the press is consumed by that, and the + // two drifting apart is what makes Back feel broken. + val canHandleBack: StateFlow = combine( + _showAppInfo, + _showPasswordPrompt, + _selectedPrivateChatPeer, + _privateChatSheetPeer, + _currentChannel + ) { showAppInfo, showPasswordPrompt, privateChatPeer, privateChatSheetPeer, channel -> + showAppInfo || + showPasswordPrompt || + privateChatPeer != null || + privateChatSheetPeer != null || + channel != null + }.stateIn( + scope = scope, + started = WhileSubscribed(5_000), + initialValue = false + ) + // Getters for internal state access fun getMessagesValue() = _messages.value fun getConnectedPeersValue() = _connectedPeers.value diff --git a/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt b/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt index bf9e2f7c..76e784bc 100644 --- a/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt +++ b/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt @@ -377,6 +377,7 @@ class ChatViewModel( val passwordPromptChannel: StateFlow = state.passwordPromptChannel val hasUnreadChannels = state.hasUnreadChannels val hasUnreadPrivateMessages = state.hasUnreadPrivateMessages + val canHandleBack = state.canHandleBack val showCommandSuggestions: StateFlow = state.showCommandSuggestions val commandSuggestions: StateFlow> = state.commandSuggestions val showMentionSuggestions: StateFlow = state.showMentionSuggestions diff --git a/app/src/test/kotlin/com/bitchat/android/ui/ChatStateBackNavigationTest.kt b/app/src/test/kotlin/com/bitchat/android/ui/ChatStateBackNavigationTest.kt new file mode 100644 index 00000000..b0bacbd8 --- /dev/null +++ b/app/src/test/kotlin/com/bitchat/android/ui/ChatStateBackNavigationTest.kt @@ -0,0 +1,93 @@ +package com.bitchat.android.ui + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.Job +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import org.junit.After +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test + +/** + * `canHandleBack` tells the chat screen's back handler whether there is any + * in-app navigation state left to unwind. It has to agree with the branches of + * [ChatViewModel.handleBackPressed], because the handler is enabled from one + * and the press is consumed by the other. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class ChatStateBackNavigationTest { + + private lateinit var scope: TestScope + private lateinit var state: ChatState + private lateinit var subscription: Job + + @Before + fun setUp() { + scope = TestScope(UnconfinedTestDispatcher()) + state = ChatState(scope) + // canHandleBack is shared WhileSubscribed, so it only tracks its + // sources while something collects it. The composable does that in + // production; the test has to do it explicitly. + subscription = scope.launch { state.canHandleBack.collect { } } + } + + @After + fun tearDown() { + subscription.cancel() + } + + @Test + fun `is false on the bare chat screen`() { + assertFalse(state.canHandleBack.value) + } + + @Test + fun `is true while the app info dialog is open`() { + state.setShowAppInfo(true) + + assertTrue(state.canHandleBack.value) + } + + @Test + fun `is true while the password prompt is open`() { + state.setShowPasswordPrompt(true) + + assertTrue(state.canHandleBack.value) + } + + @Test + fun `is true while a private chat is selected`() { + state.setSelectedPrivateChatPeer("peer-a") + + assertTrue(state.canHandleBack.value) + } + + @Test + fun `is true while the private chat sheet is open`() { + state.setPrivateChatSheetPeer("peer-a") + + assertTrue(state.canHandleBack.value) + } + + @Test + fun `is true while a channel is open`() { + state.setCurrentChannel("#bitchat") + + assertTrue(state.canHandleBack.value) + } + + @Test + fun `returns to false once the last overlay closes`() { + state.setCurrentChannel("#bitchat") + state.setShowAppInfo(true) + + state.setShowAppInfo(false) + assertTrue("the channel is still open", state.canHandleBack.value) + + state.setCurrentChannel(null) + assertFalse("nothing is left to unwind", state.canHandleBack.value) + } +} From 68361d949f4bcc6eae06005b7e321c8cc0d4577c Mon Sep 17 00:00:00 2001 From: Moe Hamade <69801237+moehamade@users.noreply.github.com> Date: Wed, 26 Aug 2026 00:11:54 +0300 Subject: [PATCH 2/3] fix(ui): forward Back when canHandleBack is momentarily stale BackHandler's enabled flag trails the state it mirrors by a coroutine dispatch and a recomposition. A second Back press inside that window finds the handler still enabled while handleBackPressed() already has nothing to unwind, and ignoring its result consumed the press instead of letting the system act on it. The callback the handler replaced did forward it. Falls back to finish() on an unhandled press, which is what the dispatcher reached before once no enabled callback consumed it. Reported by Codex review on #912. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MdKgoKZbL5K1wsn1WsES26 --- app/src/main/java/com/bitchat/android/MainActivity.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/bitchat/android/MainActivity.kt b/app/src/main/java/com/bitchat/android/MainActivity.kt index 8f0b84c1..aa336810 100644 --- a/app/src/main/java/com/bitchat/android/MainActivity.kt +++ b/app/src/main/java/com/bitchat/android/MainActivity.kt @@ -318,7 +318,11 @@ class MainActivity : OrientationAwareActivity() { // through to the system, which exits the app. val canHandleBack by chatViewModel.canHandleBack.collectAsState() BackHandler(enabled = canHandleBack) { - chatViewModel.handleBackPressed() + // enabled reaches this handler a dispatch and a recomposition + // after the state changes, so a second press can arrive while + // it is still true but there is no longer anything to unwind. + // Forward that press instead of swallowing it. + if (!chatViewModel.handleBackPressed()) finish() } ChatScreen(viewModel = chatViewModel) } From 11097a629a19637e8df3aa5aec40e7e1ad6777f4 Mon Sep 17 00:00:00 2001 From: Moe Hamade <69801237+moehamade@users.noreply.github.com> Date: Wed, 26 Aug 2026 00:28:55 +0300 Subject: [PATCH 3/3] style: cut the commentary around the back handler down to what the code cannot say The block carried seven comment lines over four of code, against 0.17 for the file as a whole. Most of it restated the identifiers: enabled = canHandleBack does not need a sentence explaining that Back is intercepted while there is state to unwind. What is left is the part that is not derivable. That enabled is a variable rather than true is a predictive-back decision, and finish() being reachable at all only makes sense once you know the flag trails the state it mirrors. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MdKgoKZbL5K1wsn1WsES26 --- app/src/main/java/com/bitchat/android/MainActivity.kt | 10 +++------- app/src/main/java/com/bitchat/android/ui/ChatState.kt | 6 ++---- .../bitchat/android/ui/ChatStateBackNavigationTest.kt | 11 ++++------- 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/com/bitchat/android/MainActivity.kt b/app/src/main/java/com/bitchat/android/MainActivity.kt index aa336810..3d752291 100644 --- a/app/src/main/java/com/bitchat/android/MainActivity.kt +++ b/app/src/main/java/com/bitchat/android/MainActivity.kt @@ -313,15 +313,11 @@ class MainActivity : OrientationAwareActivity() { } OnboardingState.CHECKING, OnboardingState.INITIALIZING, OnboardingState.COMPLETE -> { - // Intercept Back only while the chat has navigation state to - // unwind. Staying disabled otherwise lets the dispatcher fall - // through to the system, which exits the app. val canHandleBack by chatViewModel.canHandleBack.collectAsState() + // Disabled rather than always-on so predictive back can preview + // the exit instead of the app claiming every gesture. BackHandler(enabled = canHandleBack) { - // enabled reaches this handler a dispatch and a recomposition - // after the state changes, so a second press can arrive while - // it is still true but there is no longer anything to unwind. - // Forward that press instead of swallowing it. + // enabled trails the state by a dispatch and a recomposition. if (!chatViewModel.handleBackPressed()) finish() } ChatScreen(viewModel = chatViewModel) diff --git a/app/src/main/java/com/bitchat/android/ui/ChatState.kt b/app/src/main/java/com/bitchat/android/ui/ChatState.kt index e0790894..16210894 100644 --- a/app/src/main/java/com/bitchat/android/ui/ChatState.kt +++ b/app/src/main/java/com/bitchat/android/ui/ChatState.kt @@ -170,10 +170,8 @@ class ChatState( initialValue = false ) - // True while some in-app navigation state is open for Back to unwind. - // Mirrors the branches of ChatViewModel.handleBackPressed: the back - // handler is enabled from this, the press is consumed by that, and the - // two drifting apart is what makes Back feel broken. + // Mirrors the branches of ChatViewModel.handleBackPressed. The back handler + // is enabled from this and the press consumed by that, so they must agree. val canHandleBack: StateFlow = combine( _showAppInfo, _showPasswordPrompt, diff --git a/app/src/test/kotlin/com/bitchat/android/ui/ChatStateBackNavigationTest.kt b/app/src/test/kotlin/com/bitchat/android/ui/ChatStateBackNavigationTest.kt index b0bacbd8..5c5c41de 100644 --- a/app/src/test/kotlin/com/bitchat/android/ui/ChatStateBackNavigationTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/ui/ChatStateBackNavigationTest.kt @@ -12,10 +12,9 @@ import org.junit.Before import org.junit.Test /** - * `canHandleBack` tells the chat screen's back handler whether there is any - * in-app navigation state left to unwind. It has to agree with the branches of - * [ChatViewModel.handleBackPressed], because the handler is enabled from one - * and the press is consumed by the other. + * `canHandleBack` must agree with the branches of + * [ChatViewModel.handleBackPressed]: the back handler is enabled from one and + * the press consumed by the other. */ @OptIn(ExperimentalCoroutinesApi::class) class ChatStateBackNavigationTest { @@ -28,9 +27,7 @@ class ChatStateBackNavigationTest { fun setUp() { scope = TestScope(UnconfinedTestDispatcher()) state = ChatState(scope) - // canHandleBack is shared WhileSubscribed, so it only tracks its - // sources while something collects it. The composable does that in - // production; the test has to do it explicitly. + // Shared WhileSubscribed, so it only tracks its sources while collected. subscription = scope.launch { state.canHandleBack.collect { } } }