From 2befae885492c9a03b7887d9d52ae602f2e06d4c Mon Sep 17 00:00:00 2001 From: Karenkov Igor Date: Mon, 27 Oct 2025 18:51:00 +0700 Subject: [PATCH 01/16] Refactored screen cleanup and lifecycle disposal namings for clarity. Updated documentation. --- config/detekt/detekt.yml | 2 +- .../com/github/terrakok/modo/ComposeRender.kt | 85 ++++++++++++------- .../modo/animation/ScreenTransitions.kt | 4 +- 3 files changed, 57 insertions(+), 34 deletions(-) diff --git a/config/detekt/detekt.yml b/config/detekt/detekt.yml index 774d9de6..7e298dd7 100644 --- a/config/detekt/detekt.yml +++ b/config/detekt/detekt.yml @@ -881,7 +881,7 @@ Compose: # -- You can optionally have a list of types to be treated as lambdas (e.g. typedefs or fun interfaces not picked up automatically) # treatAsLambda: MyLambdaType CompositionLocalAllowlist: - active: true + active: false # -- You can optionally define a list of CompositionLocals that are allowed here allowedCompositionLocals: LocalContainerScreen,LocalMultiScreenNavigation,LocalSaveableStateHolder,LocalBeforeScreenContentOnDispose,LocalAfterScreenContentOnDispose,LocalStackNavigation, CompositionLocalNaming: diff --git a/modo-compose/src/main/java/com/github/terrakok/modo/ComposeRender.kt b/modo-compose/src/main/java/com/github/terrakok/modo/ComposeRender.kt index 89de2722..9be0c625 100644 --- a/modo-compose/src/main/java/com/github/terrakok/modo/ComposeRender.kt +++ b/modo-compose/src/main/java/com/github/terrakok/modo/ComposeRender.kt @@ -19,8 +19,8 @@ import androidx.lifecycle.Lifecycle.Event.ON_START import androidx.lifecycle.Lifecycle.Event.ON_STOP import com.github.terrakok.modo.android.ModoScreenAndroidAdapter import com.github.terrakok.modo.animation.ScreenTransition -import com.github.terrakok.modo.animation.displayingScreensAfterScreenContent -import com.github.terrakok.modo.animation.displayingScreensBeforeScreenContent +import com.github.terrakok.modo.animation.preDisposeProtectedScreens +import com.github.terrakok.modo.animation.cleanupProtectedScreens import com.github.terrakok.modo.lifecycle.LifecycleDependency import com.github.terrakok.modo.model.ScreenModelStore import com.github.terrakok.modo.model.dependenciesSortedByRemovePriority @@ -35,12 +35,12 @@ val defaultRendererContent: (@Composable ComposeRendererScope<*>.(screenModifier val LocalSaveableStateHolder = staticCompositionLocalOf { null } -private val LocalBeforeScreenContentOnDispose = staticCompositionLocalOf<() -> Unit> { - error("No LocalBeforeScreenContentOnDispose provided!") +private val LocalClearScreens = staticCompositionLocalOf<() -> Unit> { + error("No LocalClearScreens provided!") } -private val LocalAfterScreenContentOnDispose = staticCompositionLocalOf<() -> Unit> { - error("No LocalAfterScreenContentOnDispose provided!") +private val LocalPreDispose = staticCompositionLocalOf<() -> Unit> { + error("No LocalPreDispose provided!") } /** @@ -65,39 +65,62 @@ fun Screen.SaveableContent( ) { LocalSaveableStateHolder.currentOrThrow.SaveableStateProvider(key = screenKey) { ModoScreenAndroidAdapter.get(this).ProvideAndroidIntegration(manualResumePause) { - BeforeScreenContent() + SetupScreenCleanup() Content(modifier) - AfterScreenContent() + SetupLifecycleDisposal() } } } /** - * This function responsible for correct cleaning [Screen]'s when it already has left composition and some clean up can be made. - * It's not always clean [Screen] when it lives composition, but adds extra logic of tracking displaying [Screen]'s and triggering - * function provided by [LocalBeforeScreenContentOnDispose] in order to try clean removed screens that are not visible for user. + * Sets up safe screen cleanup management for this screen. + * + * CRITICAL: Must be called BEFORE user content to ensure cleanup happens AFTER user content is gone. + * + * While this screen is in composition: + * - Protects the screen from being prematurely cleaned by adding it to [cleanupProtectedScreens] + * - Acts as a safety gate - [clearScreens] will skip this screen while it's tracked here + * + * When this screen leaves composition: + * - Removes protection by removing it from [cleanupProtectedScreens] + * - Triggers immediate cleanup of screen resources (ScreenModelStore, SavedState) via [LocalClearScreens] + * + * @see ComposeRenderer.clearScreens for the cleanup logic that respects this protection */ @Composable -private inline fun Screen.BeforeScreenContent() { - val onDisposed = LocalBeforeScreenContentOnDispose.current +internal inline fun Screen.SetupScreenCleanup() { + val onDisposed = LocalClearScreens.current DisposableEffect(this) { - displayingScreensBeforeScreenContent[this@BeforeScreenContent] = Unit + cleanupProtectedScreens[this@SetupScreenCleanup] = Unit onDispose { - displayingScreensBeforeScreenContent -= this@BeforeScreenContent -// Log.d("LifecycleDebug", "BeforeScreenContent $screenKey onDispose") + cleanupProtectedScreens -= this@SetupScreenCleanup +// Log.d("LifecycleDebug", "SetupScreenCleanup $screenKey onDispose") onDisposed.invoke() } } } +/** + * Sets up protection for this screen during disposal phase to prevent premature lifecycle cleanup. + * + * While this screen is in composition: + * - Protects the screen from being prematurely disposed by adding it to [preDisposeProtectedScreens] + * - Acts as a safety gate - [ComposeRenderer.onPreDispose] will skip this screen while it's tracked here + * + * When this screen leaves composition: + * - Removes protection by removing it from [preDisposeProtectedScreens] + * - Triggers pre-disposal lifecycle callback via [LocalPreDispose] + * + * @see ComposeRenderer.onPreDispose for the pre-disposal logic that respects this protection + */ @Composable -private inline fun Screen.AfterScreenContent() { - val onPreDispose = LocalAfterScreenContentOnDispose.current +private inline fun Screen.SetupLifecycleDisposal() { + val onPreDispose = LocalPreDispose.current DisposableEffect(this) { - displayingScreensAfterScreenContent[this@AfterScreenContent] = Unit + preDisposeProtectedScreens[this@SetupLifecycleDisposal] = Unit onDispose { - displayingScreensAfterScreenContent -= this@AfterScreenContent -// Log.d("LifecycleDebug", "AfterScreenContent $screenKey onDispose") + preDisposeProtectedScreens -= this@SetupLifecycleDisposal +// Log.d("LifecycleDebug", "SetupLifecycleDisposal $screenKey onDispose") onPreDispose() } } @@ -152,7 +175,7 @@ internal class ComposeRenderer( ) { val stateHolder: SaveableStateHolder = LocalSaveableStateHolder.currentOrThrow - val beforeScreenContentOnDispose = remember { + val clearScreens = remember { { clearScreens(stateHolder) } @@ -160,7 +183,7 @@ internal class ComposeRenderer( // pre dispose means that we can send ON_DISPOSE if screen is removing, // to let Screen.Content to handle ON_DISPOSE by using functions like DisposableEffect - val afterScreenContentOnDispose = remember { + val preDispose = remember { { onPreDispose() } @@ -168,8 +191,8 @@ internal class ComposeRenderer( CompositionLocalProvider( LocalContainerScreen provides containerScreen, - LocalBeforeScreenContentOnDispose provides beforeScreenContentOnDispose, - LocalAfterScreenContentOnDispose provides afterScreenContentOnDispose, + LocalClearScreens provides clearScreens, + LocalPreDispose provides preDispose, *provideCompositionLocal ) { ComposeRendererScope(lastState, state, screen).content(modifier) @@ -191,7 +214,7 @@ internal class ComposeRenderer( } // There can be several transition of different screens on the screen, // so it is important properly clear screens that are not visible for user. - val safeToRemove = removedScreens.filter { it !in displayingScreensBeforeScreenContent } + val safeToRemove = removedScreens.filter { it !in cleanupProtectedScreens } safeToRemove.clearStates(stateHolder) if (removedScreens.isNotEmpty()) { safeToRemove.forEach { @@ -201,7 +224,7 @@ internal class ComposeRenderer( } /** - * Called onPreDispose for removed screens, that are not presented in [displayingScreensAfterScreenContent] (not displayed on screen). + * Called onPreDispose for removed screens, that are not presented in [preDisposeProtectedScreens] (not displayed on screen). * @param clearAll - forces to call onPreDispose on all children screen states that renderer holds (removed and "displayed") */ private fun onPreDispose(clearAll: Boolean = false) { @@ -214,14 +237,14 @@ internal class ComposeRenderer( } // There can be several transition of different screens on the screen, // so it is important properly clear screens that are not visible for user. - val safeToRemove = removedScreens.filter { it !in displayingScreensAfterScreenContent } + val safeToRemove = removedScreens.filter { it !in preDisposeProtectedScreens } safeToRemove.onPreDispose() } private fun Screen.clearState(stateHolder: SaveableStateHolder) { // It's important to do this check for debug purpose, because we must guaranty that Screen is cleaned only if it is not displaying anymore. // But it seems like it is not working with movable content, so this one is going to be triggered. - if (this in displayingScreensBeforeScreenContent) { + if (this in cleanupProtectedScreens) { ModoDevOptions.onIllegalClearState.validationFailed( IllegalStateException( "Trying to remove clean state of the screen $this, why this screen still is visible for User." @@ -236,11 +259,11 @@ internal class ComposeRenderer( // need for correct handling lifecycle private fun Screen.onPreDispose() { -// Log.d("LifecycleDebug", "afterScreenContentOnDispose $screenKey") +// Log.d("LifecycleDebug", "onPreDispose $screenKey") dependenciesSortedByRemovePriority() .filterIsInstance() .forEach { it.onPreDispose() } - // send afterScreenContentOnDispose to nested screens + // send onPreDispose to nested screens ((this as? ContainerScreen<*, *>)?.renderer as? ComposeRenderer<*>)?.onPreDispose(clearAll = true) } diff --git a/modo-compose/src/main/java/com/github/terrakok/modo/animation/ScreenTransitions.kt b/modo-compose/src/main/java/com/github/terrakok/modo/animation/ScreenTransitions.kt index 43671404..ee6984c0 100644 --- a/modo-compose/src/main/java/com/github/terrakok/modo/animation/ScreenTransitions.kt +++ b/modo-compose/src/main/java/com/github/terrakok/modo/animation/ScreenTransitions.kt @@ -19,8 +19,8 @@ import com.github.terrakok.modo.SaveableContent import com.github.terrakok.modo.Screen import com.github.terrakok.modo.model.lifecycleDependency -val displayingScreensBeforeScreenContent = mutableStateMapOf() -val displayingScreensAfterScreenContent = mutableStateMapOf() +val cleanupProtectedScreens = mutableStateMapOf() +val preDisposeProtectedScreens = mutableStateMapOf() typealias ScreenTransitionContent = @Composable AnimatedVisibilityScope.(Screen) -> Unit From e1c7951fb33528fa774f5b91a9451b865a37adf7 Mon Sep 17 00:00:00 2001 From: Karenkov Igor Date: Mon, 27 Oct 2025 18:57:06 +0700 Subject: [PATCH 02/16] Updated `remember` in `ComposeRender` to include `stateHolder` as a key for recomposition --- .../src/main/java/com/github/terrakok/modo/ComposeRender.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modo-compose/src/main/java/com/github/terrakok/modo/ComposeRender.kt b/modo-compose/src/main/java/com/github/terrakok/modo/ComposeRender.kt index 9be0c625..014927f3 100644 --- a/modo-compose/src/main/java/com/github/terrakok/modo/ComposeRender.kt +++ b/modo-compose/src/main/java/com/github/terrakok/modo/ComposeRender.kt @@ -175,7 +175,7 @@ internal class ComposeRenderer( ) { val stateHolder: SaveableStateHolder = LocalSaveableStateHolder.currentOrThrow - val clearScreens = remember { + val clearScreens = remember(stateHolder) { { clearScreens(stateHolder) } From 12d113c9aa819d9f5b8880fe1a42c4072796f940 Mon Sep 17 00:00:00 2001 From: Karenkov Igor Date: Mon, 27 Oct 2025 19:25:09 +0700 Subject: [PATCH 03/16] Improved logging of lifecycle: added classname to the start --- .../modo/sample/screens/base/ButtonsScreenContent.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sample/src/main/java/com/github/terrakok/modo/sample/screens/base/ButtonsScreenContent.kt b/sample/src/main/java/com/github/terrakok/modo/sample/screens/base/ButtonsScreenContent.kt index 56034c26..f9719d08 100644 --- a/sample/src/main/java/com/github/terrakok/modo/sample/screens/base/ButtonsScreenContent.kt +++ b/sample/src/main/java/com/github/terrakok/modo/sample/screens/base/ButtonsScreenContent.kt @@ -105,18 +105,18 @@ internal fun Screen.SampleScreenContent( @OptIn(ExperimentalModoApi::class) @Composable -fun Screen.LogLifecycle() { +fun Screen.LogLifecycle(prefix: String = this::class.simpleName.orEmpty()) { val lifecycleOwner = LocalLifecycleOwner.current // You will not be able to observe updates of lifecycleOwner when this content is not in the composition DisposableEffect(lifecycleOwner) { - logcat(tag = "LifecycleDebug") { "$screenKey DisposableEffect" } + logcat(tag = "LifecycleDebug") { "$prefix $screenKey DisposableEffect".trim() } val observer = LifecycleEventObserver { _, event -> - logcat(tag = "LifecycleDebug") { "$screenKey DisposableEffect $event" } + logcat(tag = "LifecycleDebug") { "$prefix $screenKey DisposableEffect $event".trim() } } lifecycleOwner.lifecycle.addObserver(observer) onDispose { - logcat(tag = "LifecycleDebug") { "$screenKey DisposableEffect onDispose" } + logcat(tag = "LifecycleDebug") { "$prefix $screenKey DisposableEffect onDispose" } lifecycleOwner.lifecycle.removeObserver(observer) } } From 4d5f3eaf91b9c987952bb718f76cb5d885ae24ce Mon Sep 17 00:00:00 2001 From: Karenkov Igor Date: Mon, 27 Oct 2025 19:25:30 +0700 Subject: [PATCH 04/16] Fixed preview of buttons screen --- .../modo/sample/screens/base/LifecycleEventsHistory.kt | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/sample/src/main/java/com/github/terrakok/modo/sample/screens/base/LifecycleEventsHistory.kt b/sample/src/main/java/com/github/terrakok/modo/sample/screens/base/LifecycleEventsHistory.kt index 0f283a68..129e0b2c 100644 --- a/sample/src/main/java/com/github/terrakok/modo/sample/screens/base/LifecycleEventsHistory.kt +++ b/sample/src/main/java/com/github/terrakok/modo/sample/screens/base/LifecycleEventsHistory.kt @@ -21,6 +21,7 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.input.pointer.pointerInput import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.platform.LocalInspectionMode import androidx.compose.ui.unit.TextUnit import androidx.compose.ui.unit.sp import androidx.lifecycle.Lifecycle @@ -40,13 +41,11 @@ fun LifecycleEventsHistory( modifier: Modifier = Modifier, key: String? = null, enabled: Boolean = SampleAppConfig.displayLifecycleEvents, - lifecycleEventsHistory: SnapshotStateList = - viewModel(key = key) { - LifecycleEventsViewModel(createSavedStateHandle()) - }.lifecycleEventsHistory, + lifecycleEventsHistory: SnapshotStateList? = null, fontSize: TextUnit = 16.sp, ) { - if (enabled) { + if (enabled && !LocalInspectionMode.current) { + val lifecycleEventsHistory = lifecycleEventsHistory ?: viewModel(key = key).lifecycleEventsHistory val lifecycleOwner = LocalLifecycleOwner.current val context = LocalContext.current From ee96bfacbb28255744089b48fc9a91f20be41d75 Mon Sep 17 00:00:00 2001 From: Karenkov Igor Date: Tue, 28 Oct 2025 17:27:05 +0700 Subject: [PATCH 05/16] Added back button to screens --- .../modo/sample/components/BackButton.kt | 36 +++++++++++++++++++ .../screens/base/ButtonsScreenContent.kt | 26 ++++++++++---- 2 files changed, 56 insertions(+), 6 deletions(-) create mode 100644 sample/src/main/java/com/github/terrakok/modo/sample/components/BackButton.kt diff --git a/sample/src/main/java/com/github/terrakok/modo/sample/components/BackButton.kt b/sample/src/main/java/com/github/terrakok/modo/sample/components/BackButton.kt new file mode 100644 index 00000000..7ec797eb --- /dev/null +++ b/sample/src/main/java/com/github/terrakok/modo/sample/components/BackButton.kt @@ -0,0 +1,36 @@ +package com.github.terrakok.modo.sample.components + +import androidx.compose.material.Icon +import androidx.compose.material.IconButton +import androidx.compose.material.MaterialTheme +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.automirrored.filled.ArrowBack +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.graphics.vector.rememberVectorPainter +import androidx.compose.ui.tooling.preview.Preview + +@Composable +fun BackButton( + onClick: () -> Unit, + modifier: Modifier = Modifier, + contentDescription: String = "Back to previous screen", +) { + IconButton( + modifier = modifier, + onClick = onClick + ) { + Icon( + painter = rememberVectorPainter(image = Icons.AutoMirrored.Filled.ArrowBack), + contentDescription = contentDescription + ) + } +} + +@Preview +@Composable +private fun Preview() { + MaterialTheme { + BackButton(onClick = {}, contentDescription = "cancel") + } +} \ No newline at end of file diff --git a/sample/src/main/java/com/github/terrakok/modo/sample/screens/base/ButtonsScreenContent.kt b/sample/src/main/java/com/github/terrakok/modo/sample/screens/base/ButtonsScreenContent.kt index f9719d08..8b4fc9b8 100644 --- a/sample/src/main/java/com/github/terrakok/modo/sample/screens/base/ButtonsScreenContent.kt +++ b/sample/src/main/java/com/github/terrakok/modo/sample/screens/base/ButtonsScreenContent.kt @@ -3,6 +3,7 @@ package com.github.terrakok.modo.sample.screens.base import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.ColumnScope +import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.WindowInsets import androidx.compose.foundation.layout.fillMaxSize @@ -20,7 +21,9 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.saveable.rememberSaveable +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.LocalInspectionMode import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp @@ -30,11 +33,15 @@ import com.github.terrakok.modo.ExperimentalModoApi import com.github.terrakok.modo.Screen import com.github.terrakok.modo.ScreenKey import com.github.terrakok.modo.sample.SampleAppConfig +import com.github.terrakok.modo.sample.components.BackButton import com.github.terrakok.modo.sample.randomBackground import com.github.terrakok.modo.sample.screens.ButtonsState import com.github.terrakok.modo.sample.screens.GroupedButtonsList import com.github.terrakok.modo.sample.screens.GroupedButtonsState import com.github.terrakok.modo.sample.screens.ModoButtonSpec +import com.github.terrakok.modo.stack.LocalStackNavigation +import com.github.terrakok.modo.stack.back +import com.github.terrakok.modo.stack.backToRoot import kotlinx.coroutines.delay import kotlinx.coroutines.isActive import logcat.logcat @@ -139,13 +146,20 @@ internal fun SampleScreenContent( Box( modifier = modifier .randomBackground() - .windowInsetsPadding(WindowInsets.systemBars) - .padding(8.dp), + .windowInsetsPadding(WindowInsets.systemBars), ) { - Column { - Text( - text = counter.toString() - ) + Column(Modifier.padding(8.dp)) { + Row( + verticalAlignment = Alignment.CenterVertically + ) { + val stackNavigation = if (LocalInspectionMode.current) null else LocalStackNavigation.current + BackButton( + onClick = { stackNavigation?.back() }, + ) + Text( + text = counter.toString() + ) + } Text( text = "$screenName $screenIndex", style = MaterialTheme.typography.h5, From d5c95b24684b922208a6e5c1c0e5d95cb376c8e5 Mon Sep 17 00:00:00 2001 From: Karenkov Igor Date: Tue, 28 Oct 2025 17:30:33 +0700 Subject: [PATCH 06/16] Improved logging --- .idea/codeStyles/Project.xml | 1 + .../java/com/github/terrakok/modo/ComposeRender.kt | 12 ++++++++---- .../java/com/github/terrakok/modo/ModoDevOptions.kt | 3 +++ .../modo/android/ModoScreenAndroidAdapter.kt | 10 +++++++--- .../terrakok/modo/animation/ScreenTransitions.kt | 4 ++-- .../com/github/terrakok/modo/util/LoggingUtils.kt | 8 ++++++++ .../terrakok/modo/sample/ModoSampleApplication.kt | 9 ++++++++- .../modo/sample/screens/dialogs/DialogsPlayground.kt | 1 - .../io/github/ikarenkov/workshop/WorkshopApp.kt | 7 +++++++ 9 files changed, 44 insertions(+), 11 deletions(-) create mode 100644 modo-compose/src/main/java/com/github/terrakok/modo/util/LoggingUtils.kt diff --git a/.idea/codeStyles/Project.xml b/.idea/codeStyles/Project.xml index 192dee4c..5d238a15 100644 --- a/.idea/codeStyles/Project.xml +++ b/.idea/codeStyles/Project.xml @@ -55,6 +55,7 @@