Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.example.metasearch.feature.detail.person

import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableLongStateOf
Expand Down Expand Up @@ -62,6 +63,16 @@ class PersonDetailPresenter @AssistedInject constructor(
var showMergeConfirmDialog by rememberRetained { mutableStateOf(false) }
var showPhotoSelectDialog by rememberRetained { mutableStateOf(false) }

var isInitialized by remember { mutableStateOf(false) }

LaunchedEffect(person) {
if (person != null) {
isInitialized = true
} else if (isInitialized && !isLoading) {
navigator.pop()
}
}
Comment on lines +68 to +74
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

LaunchedEffect must also key on isLoading to prevent race condition.

The effect only triggers when person changes, not when isLoading changes. If the person becomes null while isLoading is true (during the merge at lines 129-132), the navigation check will fail. When isLoading later becomes false, the effect won't re-trigger, leaving the user on an empty screen—the exact issue this PR aims to fix.

🔎 Proposed fix
-        LaunchedEffect(person) {
+        LaunchedEffect(person, isLoading) {
             if (person != null) {
                 isInitialized = true
             } else if (isInitialized && !isLoading) {
                 navigator.pop()
             }
         }
🤖 Prompt for AI Agents
In
@feature/detail/src/main/java/com/example/metasearch/feature/detail/person/PersonDetailPresenter.kt
around lines 68 - 74, The LaunchedEffect in PersonDetailPresenter currently only
keys on person so it won't re-run when isLoading flips, causing a race where
person becomes null during loading and navigator.pop() never runs; update the
LaunchedEffect to key on both person and isLoading (e.g., LaunchedEffect(person,
isLoading)) and keep the existing logic that sets isInitialized when person !=
null and calls navigator.pop() when person == null && isInitialized &&
!isLoading so the effect will re-evaluate after the merge at the code altering
isLoading (the merge around lines 129-132) and correctly navigate away.


suspend fun savePersonInfo() {
val currentPerson = person ?: return

Expand Down