From e16ca2b2d23b77880b47a963d07ea3948decb0da Mon Sep 17 00:00:00 2001 From: Alex Hart Date: Thu, 25 Sep 2025 10:39:02 -0300 Subject: [PATCH] Several navhost behavioural updates to ensure the right pane is displayed at the right time. --- .../thoughtcrime/securesms/MainActivity.kt | 28 ++++- .../securesms/main/MainActivityComponents.kt | 75 +++++++++++- .../main/MainNavigationDetailLocation.kt | 13 +++ .../securesms/main/MainNavigationViewModel.kt | 109 +++++++++--------- 4 files changed, 167 insertions(+), 58 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/MainActivity.kt b/app/src/main/java/org/thoughtcrime/securesms/MainActivity.kt index 5fec9349b8..359ac3c1e3 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/MainActivity.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/MainActivity.kt @@ -126,6 +126,8 @@ import org.thoughtcrime.securesms.main.callNavGraphBuilder import org.thoughtcrime.securesms.main.chatNavGraphBuilder import org.thoughtcrime.securesms.main.navigateToDetailLocation import org.thoughtcrime.securesms.main.rememberDetailNavHostController +import org.thoughtcrime.securesms.main.rememberFocusRequester +import org.thoughtcrime.securesms.main.rememberMainNavigationDetailLocation import org.thoughtcrime.securesms.main.storiesNavGraphBuilder import org.thoughtcrime.securesms.mediasend.camerax.CameraXUtil import org.thoughtcrime.securesms.mediasend.v2.MediaSelectionActivity @@ -320,6 +322,7 @@ class MainActivity : PassphraseRequiredActivity(), VoiceNoteMediaControllerOwner val isBackHandlerEnabled = mainToolbarState.destination != MainNavigationListLocation.CHATS BackHandler(enabled = isBackHandlerEnabled) { + mainNavigationViewModel.setFocusedPane(ThreePaneScaffoldRole.Secondary) mainNavigationViewModel.goTo(MainNavigationListLocation.CHATS) } @@ -353,17 +356,33 @@ class MainActivity : PassphraseRequiredActivity(), VoiceNoteMediaControllerOwner ) val mutableInteractionSource = remember { MutableInteractionSource() } - val mainNavigationDetailLocation by mainNavigationViewModel.detailLocation.collectAsStateWithLifecycle(mainNavigationViewModel.earlyNavigationDetailLocationRequested ?: MainNavigationDetailLocation.Empty) + val mainNavigationDetailLocation by rememberMainNavigationDetailLocation(mainNavigationViewModel) - val chatsNavHostController = rememberDetailNavHostController { + val chatsNavHostController = rememberDetailNavHostController( + onRequestFocus = rememberFocusRequester( + mainNavigationViewModel = mainNavigationViewModel, + currentListLocation = mainNavigationState.currentListLocation, + isTargetListLocation = { it in listOf(MainNavigationListLocation.CHATS, MainNavigationListLocation.ARCHIVE) } + ) + ) { chatNavGraphBuilder() } - val callsNavHostController = rememberDetailNavHostController { + val callsNavHostController = rememberDetailNavHostController( + onRequestFocus = rememberFocusRequester( + mainNavigationViewModel = mainNavigationViewModel, + currentListLocation = mainNavigationState.currentListLocation + ) { it == MainNavigationListLocation.CALLS } + ) { callNavGraphBuilder(it) } - val storiesNavHostController = rememberDetailNavHostController { + val storiesNavHostController = rememberDetailNavHostController( + onRequestFocus = rememberFocusRequester( + mainNavigationViewModel = mainNavigationViewModel, + currentListLocation = mainNavigationState.currentListLocation + ) { it == MainNavigationListLocation.STORIES } + ) { storiesNavGraphBuilder() } @@ -377,6 +396,7 @@ class MainActivity : PassphraseRequiredActivity(), VoiceNoteMediaControllerOwner MainNavigationListLocation.STORIES -> storiesNavHostController }.navigateToDetailLocation(mainNavigationDetailLocation) } + is MainNavigationDetailLocation.Chats -> chatsNavHostController.navigateToDetailLocation(mainNavigationDetailLocation) is MainNavigationDetailLocation.Calls -> callsNavHostController.navigateToDetailLocation(mainNavigationDetailLocation) is MainNavigationDetailLocation.Stories -> storiesNavHostController.navigateToDetailLocation(mainNavigationDetailLocation) diff --git a/app/src/main/java/org/thoughtcrime/securesms/main/MainActivityComponents.kt b/app/src/main/java/org/thoughtcrime/securesms/main/MainActivityComponents.kt index b49d055724..6fac8860f4 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/main/MainActivityComponents.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/main/MainActivityComponents.kt @@ -13,8 +13,14 @@ import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.padding import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.adaptive.layout.ThreePaneScaffoldRole import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.State +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip @@ -24,6 +30,7 @@ import androidx.lifecycle.viewmodel.compose.LocalViewModelStoreOwner import androidx.navigation.NavGraphBuilder import androidx.navigation.NavHostController import androidx.navigation.compose.NavHost +import androidx.navigation.compose.currentBackStackEntryAsState import androidx.navigation.compose.rememberNavController import androidx.navigation.createGraph import org.thoughtcrime.securesms.R @@ -44,8 +51,64 @@ fun EmptyDetailScreen() { } } +/** + * Emits [MainNavigationDetailLocation] whenever a change occurs, and persists the latest value. + * + * In order to ensure proper behaviour when moving from the inner to outer screen, and to ensure we don't accidentally end up + * back on an unexpected Empty screen, we utilize a LaunchedEffect that subscribes to our detailLocation Flow instead of directly + * utilizing collectAsStateWithLifecycle. Then the latest value is remembered as a saveable using the default [MainNavigationDetailLocation.Saver] + */ @Composable -fun rememberDetailNavHostController(builder: NavGraphBuilder.(NavHostController) -> Unit): NavHostController { +fun rememberMainNavigationDetailLocation( + mainNavigationViewModel: MainNavigationViewModel +): State { + val state = rememberSaveable( + stateSaver = MainNavigationDetailLocation.Saver() + ) { + mutableStateOf(mainNavigationViewModel.earlyNavigationDetailLocationRequested ?: MainNavigationDetailLocation.Empty) + } + + LaunchedEffect(Unit) { + mainNavigationViewModel.detailLocation.collect { + if (state.value == it) { + mainNavigationViewModel.setFocusedPane( + if (it == MainNavigationDetailLocation.Empty) { + ThreePaneScaffoldRole.Secondary + } else { + ThreePaneScaffoldRole.Primary + } + ) + } + + state.value = it + } + } + + return state +} + +@Composable +fun rememberFocusRequester( + mainNavigationViewModel: MainNavigationViewModel, + currentListLocation: MainNavigationListLocation, + isTargetListLocation: (MainNavigationListLocation) -> Boolean +): (ThreePaneScaffoldRole) -> Unit { + return remember(currentListLocation, isTargetListLocation, mainNavigationViewModel) { + if (isTargetListLocation(currentListLocation)) { + { + mainNavigationViewModel.setFocusedPane(it) + } + } else { + {} + } + } +} + +@Composable +fun rememberDetailNavHostController( + onRequestFocus: (ThreePaneScaffoldRole) -> Unit, + builder: NavGraphBuilder.(NavHostController) -> Unit +): NavHostController { val navHostController = rememberNavController() val viewModelStore = LocalViewModelStoreOwner.current!!.viewModelStore @@ -57,10 +120,18 @@ fun rememberDetailNavHostController(builder: NavGraphBuilder.(NavHostController) navHostController.setViewModelStore(viewModelStore) navHostController.setGraph(graph, null) - graph } + val entry by navHostController.currentBackStackEntryAsState() + LaunchedEffect(entry) { + if (entry != null && entry?.destination?.route != MainNavigationDetailLocation.Empty::class.qualifiedName) { + onRequestFocus(ThreePaneScaffoldRole.Primary) + } else { + onRequestFocus(ThreePaneScaffoldRole.Secondary) + } + } + return navHostController } diff --git a/app/src/main/java/org/thoughtcrime/securesms/main/MainNavigationDetailLocation.kt b/app/src/main/java/org/thoughtcrime/securesms/main/MainNavigationDetailLocation.kt index a8d8f2f398..1656f5e0e5 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/main/MainNavigationDetailLocation.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/main/MainNavigationDetailLocation.kt @@ -6,10 +6,12 @@ package org.thoughtcrime.securesms.main import android.os.Parcelable +import androidx.compose.runtime.saveable.SaverScope import kotlinx.parcelize.IgnoredOnParcel import kotlinx.parcelize.Parcelize import kotlinx.serialization.Serializable import kotlinx.serialization.Transient +import kotlinx.serialization.json.Json import org.thoughtcrime.securesms.conversation.ConversationArgs import org.thoughtcrime.securesms.recipients.RecipientId import org.thoughtcrime.securesms.service.webrtc.links.CallLinkRoomId @@ -17,9 +19,20 @@ import org.thoughtcrime.securesms.service.webrtc.links.CallLinkRoomId /** * Describes which content to display in the detail view. */ +@Serializable @Parcelize sealed class MainNavigationDetailLocation : Parcelable { + class Saver : androidx.compose.runtime.saveable.Saver { + override fun SaverScope.save(value: MainNavigationDetailLocation): String? { + return Json.encodeToString(value) + } + + override fun restore(value: String): MainNavigationDetailLocation? { + return Json.decodeFromString(value) + } + } + /** * Flag utilized internally to determine whether the given route is displayed at the root * of a task stack (or on top of Empty) diff --git a/app/src/main/java/org/thoughtcrime/securesms/main/MainNavigationViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/main/MainNavigationViewModel.kt index 98d15b0696..3cff7143aa 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/main/MainNavigationViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/main/MainNavigationViewModel.kt @@ -7,6 +7,7 @@ package org.thoughtcrime.securesms.main import androidx.compose.material3.adaptive.ExperimentalMaterial3AdaptiveApi import androidx.compose.material3.adaptive.layout.ThreePaneScaffoldRole +import androidx.compose.material3.adaptive.navigation.BackNavigationBehavior import androidx.compose.material3.adaptive.navigation.ThreePaneScaffoldNavigator import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope @@ -72,6 +73,15 @@ class MainNavigationViewModel( var earlyNavigationDetailLocationRequested: MainNavigationDetailLocation? = null private set + private var earlyFocusedPaneRequested: ThreePaneScaffoldRole? = null + + /** + * Which pane we display to the user at a given time should be driven solely by user intention. There are cases + * where the user can change configurations (such as opening a foldable) and we will restore state and errantly + * take them back into a PRIMARY pane. This boolean helps avoid these cases. + */ + private var lockPaneToSecondary = false + init { performStoreUpdate(MainNavigationRepository.getNumberOfUnreadMessages()) { unreadChats, state -> state.copy(chatsCount = unreadChats.toInt()) @@ -97,7 +107,7 @@ class MainNavigationViewModel( fun wrapNavigator(composeScope: CoroutineScope, threePaneScaffoldNavigator: ThreePaneScaffoldNavigator, goToLegacyDetailLocation: (MainNavigationDetailLocation) -> Unit): ThreePaneScaffoldNavigator { this.goToLegacyDetailLocation = goToLegacyDetailLocation this.navigatorScope = composeScope - this.navigator = threePaneScaffoldNavigator + this.navigator = Nav(threePaneScaffoldNavigator) earlyNavigationListLocationRequested?.let { goTo(it) @@ -105,22 +115,50 @@ class MainNavigationViewModel( earlyNavigationListLocationRequested = null + earlyFocusedPaneRequested?.let { + setFocusedPane(it) + } + + earlyFocusedPaneRequested = null + earlyNavigationDetailLocationRequested?.let { goTo(it) } - return threePaneScaffoldNavigator + return this.navigator!! } fun clearEarlyDetailLocation() { earlyNavigationDetailLocationRequested = null } + fun setFocusedPane(role: ThreePaneScaffoldRole) { + val roleToGoTo = if (lockPaneToSecondary) { + ThreePaneScaffoldRole.Secondary + } else { + role + } + + if (navigator == null) { + earlyFocusedPaneRequested = roleToGoTo + return + } + + navigatorScope?.launch { + navigator?.navigateTo(roleToGoTo) + } + } + /** * Navigates to the requested location. If the navigator is not present, this functionally sets our * "default" location to that specified, and we will route the user there when the navigator is set. + * + * This does not update what panel is currently focused, so that we can perform actions (such as first + * render) *before* swapping panes. This helps to prevent flashing / duplicate loads. */ override fun goTo(location: MainNavigationDetailLocation) { + lockPaneToSecondary = false + if (!WindowSizeClass.isLargeScreenSupportEnabled()) { goToLegacyDetailLocation?.invoke(location) return @@ -134,67 +172,19 @@ class MainNavigationViewModel( viewModelScope.launch { internalDetailLocation.emit(location) } - - val focusedPane = when (location) { - is MainNavigationDetailLocation.Empty -> { - ThreePaneScaffoldRole.Secondary - } - - is MainNavigationDetailLocation.Chats.Conversation -> { - ThreePaneScaffoldRole.Primary - } - - is MainNavigationDetailLocation.Calls -> { - ThreePaneScaffoldRole.Primary - } - } - - navigatorScope?.launch { - val currentPane: ThreePaneScaffoldRole = navigator?.currentDestination?.pane ?: return@launch - - if (currentPane == focusedPane) { - return@launch - } - - if (currentPane == ThreePaneScaffoldRole.Secondary) { - navigator?.navigateTo(focusedPane) - } else { - navigator?.navigateBack() - if (navigator?.currentDestination == null) { - navigator?.navigateTo(ThreePaneScaffoldRole.Secondary) - } - } - } } override fun goTo(location: MainNavigationListLocation) { + lockPaneToSecondary = true + if (navigator == null) { earlyNavigationListLocationRequested = location return } - when (location) { - MainNavigationListLocation.CHATS -> Unit - MainNavigationListLocation.ARCHIVE -> Unit - MainNavigationListLocation.CALLS -> Unit - MainNavigationListLocation.STORIES -> Unit - } - internalMainNavigationState.update { it.copy(currentListLocation = location) } - - navigatorScope?.launch { - val currentPane = navigator?.currentDestination?.pane ?: return@launch - if (currentPane == ThreePaneScaffoldRole.Secondary) { - return@launch - } else { - navigator?.navigateBack() - if (navigator?.currentDestination == null) { - navigator?.navigateTo(ThreePaneScaffoldRole.Secondary) - } - } - } } fun goToCameraFirstStoryCapture() { @@ -257,6 +247,7 @@ class MainNavigationViewModel( if (currentTab == destination) { internalTabClickEvents.emit(destination) } else { + setFocusedPane(ThreePaneScaffoldRole.Secondary) goTo(destination) } } @@ -273,4 +264,18 @@ class MainNavigationViewModel( enum class NavigationEvent { STORY_CAMERA_FIRST } + + /** + * Ensures that when the user navigates back from the PRIMARY to SECONDARY pane, we lock our pane until they choose another primary + * piece of content via [goTo]. + */ + private inner class Nav(private val delegate: ThreePaneScaffoldNavigator) : ThreePaneScaffoldNavigator by delegate { + override suspend fun seekBack(backNavigationBehavior: BackNavigationBehavior, fraction: Float) { + delegate.seekBack(backNavigationBehavior, fraction) + + if (fraction == 0f) { + lockPaneToSecondary = true + } + } + } }