Fixed parent lifecycle propagation logic

ikarenkov · ikarenkov · Nov 16, 2024 · Nov 6, 2024 · Nov 11, 2024 · Nov 16, 2024
commit 09278c713cd55c1a4189402cdb9b90cc16f39297
diff --git a/build-logic/convention/src/main/kotlin/com/github/terrakok/DetektPlugin.kt b/build-logic/convention/src/main/kotlin/com/github/terrakok/DetektPlugin.kt
@@ -26,49 +26,57 @@ class DetektPlugin : Plugin<Project> {
 
 fun Project.setupDetektTask() {
     tasks.register<Detekt>("detektAll") {
-        reports {
-            sarif.required = true
-        }
-        // The directories where detekt looks for source files.
-        // Defaults to `files("src/main/java", "src/test/java", "src/main/kotlin", "src/test/kotlin")`.
-        setSource(projectDir)
+        configureDetekt(this)
+    }
+    tasks.register<Detekt>("detektFormat") {
+        configureDetekt(this)
+        autoCorrect = true
+    }
+}
 
-        include("**/*.kt")
-        include("**/*.kts")
-        exclude("**/resources/")
-        exclude("**/build/")
-        exclude("Writerside/codeSnippets/")
+private fun Project.configureDetekt(detekt: Detekt) {
+    detekt.reports {
+        sarif.required = true
+    }
+    // The directories where detekt looks for source files.
+    // Defaults to `files("src/main/java", "src/test/java", "src/main/kotlin", "src/test/kotlin")`.
+    detekt.setSource(project.projectDir)
+
+    detekt.include("**/*.kt")
+    detekt.include("**/*.kts")
+    detekt.exclude("**/resources/")
+    detekt.exclude("**/build/")
+    detekt.exclude("Writerside/codeSnippets/")
 
-        // Builds the AST in parallel. Rules are always executed in parallel.
-        // Can lead to speedups in larger projects. `false` by default.
-        parallel = true
+    // Builds the AST in parallel. Rules are always executed in parallel.
+    // Can lead to speedups in larger projects. `false` by default.
+    detekt.parallel = true
 
-        // Define the detekt configuration(s) you want to use.
-        // Defaults to the default detekt configuration.
-        config.setFrom("config/detekt/detekt.yml")
+    // Define the detekt configuration(s) you want to use.
+    // Defaults to the default detekt configuration.
+    detekt.config.setFrom("config/detekt/detekt.yml")
 
-        // Applies the config files on top of detekt's default config file. `false` by default.
-        buildUponDefaultConfig = false
+    // Applies the config files on top of detekt's default config file. `false` by default.
+    detekt.buildUponDefaultConfig = false
 
-        // Turns on all the rules. `false` by default.
-        allRules = false
+    // Turns on all the rules. `false` by default.
+    detekt.allRules = false
 
-        // Specifying a baseline file. All findings stored in this file in subsequent runs of detekt.
+    // Specifying a baseline file. All findings stored in this file in subsequent runs of detekt.
 //    baseline = file("path/to/baseline.xml")
 
-        // Disables all default detekt rulesets and will only run detekt with custom rules
-        // defined in plugins passed in with `detektPlugins` configuration. `false` by default.
-        disableDefaultRuleSets = false
+    // Disables all default detekt rulesets and will only run detekt with custom rules
+    // defined in plugins passed in with `detektPlugins` configuration. `false` by default.
+    detekt.disableDefaultRuleSets = false
 
-        // Adds debug output during task execution. `false` by default.
-        debug = false
+    // Adds debug output during task execution. `false` by default.
+    detekt.debug = false
 
-        // If set to `true` the build does not fail when the
-        // maxIssues count was reached. Defaults to `false`.
-        ignoreFailures = true
+    // If set to `true` the build does not fail when the
+    // maxIssues count was reached. Defaults to `false`.
+    detekt.ignoreFailures = true
 
-        // Specify the base path for file paths in the formatted reports.
-        // If not set, all file paths reported will be absolute file path.
-        basePath = rootProject.projectDir.absolutePath
-    }
+    // Specify the base path for file paths in the formatted reports.
+    // If not set, all file paths reported will be absolute file path.
+    detekt.basePath = project.rootProject.projectDir.absolutePath
 }
diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
@@ -1,7 +1,7 @@
 [versions]
 composeWheelPicker = "1.0.0-beta05"
 leakcanaryAndroid = "2.14"
-modo = "0.10.0"
+modo = "0.10.1"
 androidGradlePlugin = "8.4.0"
 detektComposeVersion = "0.3.20"
 detektVersion = "1.23.6"

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
@@ -50,7 +50,7 @@ private val LocalAfterScreenContentOnDispose = staticCompositionLocalOf<() -> Un
  * 3. Handles lifecycle of [Screen] by adding [DisposableEffect] before and after content, in order to notify [ComposeRenderer]
  *    when [Screen.Content] is about to leave composition and when it has left composition.
  * @param modifier is a modifier that will be passed into [Screen.Content]
- * @param manualResumePause define whenever we are going to manually call [LifecycleDependency.onResume] and [LifecycleDependency.onPause]
+ * @param manualResumePause define whenever we are going to manually call [LifecycleDependency.showTransitionFinished] and [LifecycleDependency.hideTransitionStarted]
  * to emmit [ON_RESUME] and [ON_PAUSE]. Otherwise, [ON_RESUME] will be called straight after [ON_START] and [ON_PAUSE] will be called straight
  * before [ON_STOP].
  *

diff --git a/modo-compose/src/main/java/com/github/terrakok/modo/android/ModoScreenAndroidAdapter.kt b/modo-compose/src/main/java/com/github/terrakok/modo/android/ModoScreenAndroidAdapter.kt
@@ -50,15 +50,15 @@ import com.github.terrakok.modo.util.getApplication
 import java.util.concurrent.atomic.AtomicReference
 
 /**
- * Adapter for Screem, to support android-related features using Modo, such as:
- * 1. ViewModel support
- * 2. Lifecycle support
- * 3. SavedState support
+ * Adapter for Screen that provides android-related features support using Modo, such as:
+ * 1. ViewModel
+ * 2. Lifecycle
+ * 3. SavedState
  *
  * It the single instance of [ModoScreenAndroidAdapter] per Screen.
  */
 class ModoScreenAndroidAdapter private constructor(
-//    just for debug purpose.
+    // For debugging purposes
     internal val screen: Screen
 ) :
     LifecycleOwner,
@@ -101,10 +101,11 @@ class ModoScreenAndroidAdapter private constructor(
     private val application: Application? get() = atomicContext.get()?.applicationContext?.getApplication()
 
     /**
-     * Indicates that lifecycle should be moved to RESUMED state as soon as parent will be RESUMED.
+     * Holding transition state of the screen to be able to handle lifecycle events from parent properly.
+     * Check out [needPropagateLifecycleEventFromParent] for more details.
      */
     @Volatile
-    private var isResumePostponed: Boolean = false
+    private var screenTransitionState: ScreenTransitionState = ScreenTransitionState.HIDDEN
 
     init {
         controller.performAttach()
@@ -136,18 +137,18 @@ class ModoScreenAndroidAdapter private constructor(
         safeHandleLifecycleEvent(ON_DESTROY)
     }
 
-    override fun onPause() {
+    override fun hideTransitionStarted() {
+        screenTransitionState = ScreenTransitionState.HIDING
         safeHandleLifecycleEvent(ON_PAUSE)
     }
 
-    override fun onResume() {
+    override fun showTransitionFinished() {
+        screenTransitionState = ScreenTransitionState.SHOWN
         val parentState = atomicParentLifecycleOwner.get()?.lifecycle?.currentState
-        if (parentState == null || parentState != Lifecycle.State.RESUMED) {
-            // We need to do so, because child is ready to move to resumed state, when parent is not.
-            // It can happen when we display this screen as a first screen inside container, that is animating.
-            // So we need to wait until parent will be resumed and execute ON_RESUME event after it.
-            isResumePostponed = true
-        } else {
+        // It's crucial to check parent state, because our state can't be greater than a parent state.
+        // If this condition is not met, resuming will be done when parent will be resumed and screenTransitionState == ScreenTransitionState.SHOWN.
+        // It can happen when we display this screen as a first screen inside container, that is animating.
+        if (parentState != null && parentState == Lifecycle.State.RESUMED) {
             safeHandleLifecycleEvent(ON_RESUME)
         }
     }
@@ -253,15 +254,11 @@ class ModoScreenAndroidAdapter private constructor(
                     if (
                         needPropagateLifecycleEventFromParent(
                             event,
-                            isResumePostponed = isResumePostponed,
+                            screenTransitionState = screenTransitionState,
                             isActivityFinishing = activity?.isFinishing,
                             isChangingConfigurations = activity?.isChangingConfigurations
                         )
                     ) {
-                        // reset postpone resume flag because we moved state to resumed
-                        if (event == ON_RESUME) {
-                            isResumePostponed = false
-                        }
                         safeHandleLifecycleEvent(event)
                     }
                 }
@@ -285,10 +282,31 @@ class ModoScreenAndroidAdapter private constructor(
         val skippEvent = needSkipEvent(lifecycle.currentState, event)
         if (!skippEvent) {
 //            Log.d("ModoScreenAndroidAdapter", "${screen.screenKey} handleLifecycleEvent $event")
+            screenTransitionState = when {
+                // Whenever we receive ON_RESUME event, we need to move screen to SHOWN state to indicate that there is no transition of this screen.
+                event == Lifecycle.Event.ON_RESUME -> ScreenTransitionState.SHOWN
+                // Whe need to check transition state to distinguish between
+                // 1. finishing screen hiding transition. In this case, we need to move screen to hidden state.
+                // 2. hiding screen, because of lifecycle event. In this case we don't need to change animation state.
+                event == Lifecycle.Event.ON_STOP && screenTransitionState == ScreenTransitionState.HIDING -> ScreenTransitionState.HIDDEN
+                // Pause by itself doesn't mean that screen is hidden, it can be visible, but not active. F.e. when system dialog is shown.
+                else -> screenTransitionState
+            }
             lifecycle.handleLifecycleEvent(event)
         }
     }
 
+    /**
+     * Enum that represents
+     */
+    private enum class ScreenTransitionState {
+        HIDING,
+        HIDDEN,
+        SHOWN
+        // There is no SHOWING state, because we cannot distinguish it by using lifecycle events and
+        // [hideTransitionStarted] and [showTransitionFinished] methods.
+    }
+
     companion object {
 
         private val moveLifecycleStateUpEvents = setOf(
@@ -315,9 +333,9 @@ class ModoScreenAndroidAdapter private constructor(
             ) { ModoScreenAndroidAdapter(screen) }
 
         @JvmStatic
-        fun needPropagateLifecycleEventFromParent(
+        private fun needPropagateLifecycleEventFromParent(
             event: Lifecycle.Event,
-            isResumePostponed: Boolean,
+            screenTransitionState: ScreenTransitionState,
             isActivityFinishing: Boolean?,
             isChangingConfigurations: Boolean?
         ) =
@@ -336,12 +354,13 @@ class ModoScreenAndroidAdapter private constructor(
                 event == ON_DESTROY && (isActivityFinishing == false || isChangingConfigurations == true) -> {
                     false
                 }
-                isResumePostponed && event == ON_RESUME -> {
+                screenTransitionState == ScreenTransitionState.SHOWN && event == ON_RESUME -> {
                     true
                 }
                 else -> {
-                    // Parent can only move lifecycle state down. Because parent cant be already resumed, but child is not, because of running animation.
-                    event !in moveLifecycleStateUpEvents
+                    // Except previous condition, parent can only move lifecycle state down.
+                    // Because parent cant be already resumed, but child is not, because of running animation.
+                    event in moveLifecycleStateDownEvents
                 }
             }
 

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
@@ -56,6 +56,7 @@ fun ComposeRendererScope<*>.ScreenTransition(
         modifier = modifier,
         contentKey = { it.screenKey },
     ) { screen ->
+        content(screen)
         DisposableEffect(transition.currentState, transition.targetState) {
 //            Log.d(
 //                "LifecycleDebug",
@@ -66,15 +67,14 @@ fun ComposeRendererScope<*>.ScreenTransition(
             if (screen == transition.currentState && screen != transition.targetState) {
                 // Start of animation that hides this screen, so we should pause lifecycle
 //                Log.d("LifecycleDebug", "${screen.screenKey}: ON_PAUSE!")
-                screen.lifecycleDependency()?.onPause()
+                screen.lifecycleDependency()?.hideTransitionStarted()
             }
             if (transition.currentState == transition.targetState && screen == transition.currentState) {
                 // Finish of animation that shows this screen, so we should resume lifecycle
 //                Log.d("LifecycleDebug", "${screen.screenKey}: ON_RESUME!")
-                screen.lifecycleDependency()?.onResume()
+                screen.lifecycleDependency()?.showTransitionFinished()
             }
             onDispose { }
         }
-        content(screen)
     }
 }
diff --git a/modo-compose/src/main/java/com/github/terrakok/modo/lifecycle/LifecycleDependency.kt b/modo-compose/src/main/java/com/github/terrakok/modo/lifecycle/LifecycleDependency.kt
@@ -12,13 +12,13 @@ interface LifecycleDependency {
      * Should be called when associated screen is ready to be moved to ON_RESUME state.
      * F.e. when screen appearance animation is finished and screen is fully visible.
      */
-    fun onResume()
+    fun showTransitionFinished()
 
     /**
      * Should be called when associated screen is ready to be moved to ON_PAUSE state.
      * F.e. when screen hide animation is started and screen is not fully visible.
      */
-    fun onPause()
+    fun hideTransitionStarted()
 
     fun onPreDispose()
 

diff --git a/modo-compose/src/main/java/com/github/terrakok/modo/stack/StackScreen.kt b/modo-compose/src/main/java/com/github/terrakok/modo/stack/StackScreen.kt
@@ -174,7 +174,7 @@ abstract class StackScreen(
                 }
             }
             DialogScreen.DialogConfig.Custom -> {
-                DecorateCustomDialog(dialog, modifier) {
+                DecorateCustomDialog(dialog, modifier) { modifier ->
                     Content(dialog, modifier, content)
                 }
             }

diff --git a/sample/src/main/java/com/github/terrakok/modo/sample/Animations.kt b/sample/src/main/java/com/github/terrakok/modo/sample/Animations.kt
@@ -9,6 +9,7 @@ import androidx.compose.animation.slideOutHorizontally
 import androidx.compose.animation.togetherWith
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.unit.IntOffset
 import com.github.terrakok.modo.ComposeRendererScope
 import com.github.terrakok.modo.DialogScreen
 import com.github.terrakok.modo.ExperimentalModoApi
@@ -30,21 +31,23 @@ fun ComposeRendererScope<StackState>.SlideTransition(
             val transitionType = calculateStackTransitionType(oldState, newState)
             when {
                 transitionType == StackTransitionType.Replace -> {
-                    scaleIn(initialScale = 2f) + fadeIn() togetherWith fadeOut()
+                    val animationSpec = tween<Float>(durationMillis = SampleAppConfig.animationDurationMs)
+                    scaleIn(initialScale = 2f, animationSpec = animationSpec) + fadeIn(animationSpec) togetherWith
+                        fadeOut(animationSpec)
                 }
-                oldState?.stack?.last() is DialogScreen -> {
-                    fadeIn() togetherWith fadeOut()
-                }
-                oldState?.stack?.last() !is DialogScreen && newState?.stack?.last() is DialogScreen -> {
-                    fadeIn() togetherWith fadeOut()
+                oldState?.stack?.last() is DialogScreen ||
+                    oldState?.stack?.last() !is DialogScreen && newState?.stack?.last() is DialogScreen -> {
+                    val animationSpec = tween<Float>(durationMillis = SampleAppConfig.animationDurationMs)
+                    fadeIn(animationSpec) togetherWith fadeOut(animationSpec)
                 }
                 else -> {
                     val (initialOffset, targetOffset) = when (transitionType) {
                         StackTransitionType.Pop -> ({ size: Int -> -size }) to ({ size: Int -> size })
                         else -> ({ size: Int -> size }) to ({ size: Int -> -size })
                     }
-                    slideInHorizontally(initialOffsetX = initialOffset, animationSpec = tween(durationMillis = 1000)) togetherWith
-                        slideOutHorizontally(targetOffsetX = targetOffset, animationSpec = tween(durationMillis = 1000))
+                    val animationSpec = tween<IntOffset>(durationMillis = SampleAppConfig.animationDurationMs)
+                    slideInHorizontally(initialOffsetX = initialOffset, animationSpec = animationSpec) togetherWith
+                        slideOutHorizontally(targetOffsetX = targetOffset, animationSpec = animationSpec)
                 }
             }
         }

diff --git a/sample/src/main/java/com/github/terrakok/modo/sample/SampleAppConfig.kt b/sample/src/main/java/com/github/terrakok/modo/sample/SampleAppConfig.kt
@@ -2,4 +2,6 @@ package com.github.terrakok.modo.sample
 
 object SampleAppConfig {
     const val counterEnabled: Boolean = true
+    const val displayLifecycleEvents: Boolean = false
+    const val animationDurationMs = 1000
 }
diff --git a/sample/src/main/java/com/github/terrakok/modo/sample/screens/ButtonsList.kt b/sample/src/main/java/com/github/terrakok/modo/sample/screens/ButtonsList.kt
@@ -22,7 +22,7 @@ import androidx.compose.ui.text.style.TextAlign
 import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.unit.dp
 
-fun ButtonsState(buttons: List<ModoButtonSpec>) = GroupedButtonsState(
+fun ButtonsState(buttons: List<ModoButtonSpec>): GroupedButtonsState = GroupedButtonsState(
     listOf(GroupedButtonsState.Group(null, buttons))
 )
 

diff --git a/sample/src/main/java/com/github/terrakok/modo/sample/screens/MainScreen.kt b/sample/src/main/java/com/github/terrakok/modo/sample/screens/MainScreen.kt
@@ -109,7 +109,7 @@ internal fun Screen.MainScreenContent(
     )
 }
 
-@Suppress("LongMethod")
+@Suppress("LongMethod", "MagicNumber")
 @Composable
 private fun rememberButtons(
     screenKey: ScreenKey,

diff --git a/sample/src/main/java/com/github/terrakok/modo/sample/screens/PauseButtonState.kt b/sample/src/main/java/com/github/terrakok/modo/sample/screens/PauseButtonState.kt
@@ -0,0 +1,14 @@
+package com.github.terrakok.modo.sample.screens
+
+import android.content.Context
+import android.content.Intent
+
+fun PauseButtonSpec(context: Context) = ModoButtonSpec("Pause") {
+    val shareIntent = Intent().apply {
+        action = Intent.ACTION_SEND
+        putExtra(Intent.EXTRA_TEXT, "This is a test message.")
+        type = "text/plain"
+    }
+    // Launch the share dialog
+    context.startActivity(Intent.createChooser(shareIntent, "Share via"))
+}