mirror of
https://github.com/permissionlesstech/bitchat-android.git
synced 2026-09-19 04:59:59 +00:00
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MdKgoKZbL5K1wsn1WsES26
This commit is contained in:
parent
5156f7de89
commit
78991a289e
@ -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)
|
||||
}
|
||||
|
||||
|
||||
@ -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<Boolean> = 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
|
||||
|
||||
@ -377,6 +377,7 @@ class ChatViewModel(
|
||||
val passwordPromptChannel: StateFlow<String?> = state.passwordPromptChannel
|
||||
val hasUnreadChannels = state.hasUnreadChannels
|
||||
val hasUnreadPrivateMessages = state.hasUnreadPrivateMessages
|
||||
val canHandleBack = state.canHandleBack
|
||||
val showCommandSuggestions: StateFlow<Boolean> = state.showCommandSuggestions
|
||||
val commandSuggestions: StateFlow<List<CommandSuggestion>> = state.commandSuggestions
|
||||
val showMentionSuggestions: StateFlow<Boolean> = state.showMentionSuggestions
|
||||
|
||||
@ -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)
|
||||
}
|
||||
}
|
||||
Loading…
x
Reference in New Issue
Block a user