Umair Nazim
Umair Nazim

Reputation: 11

Toast message appears multiple times when navigating back in Jetpack Compose with MutableStateFlow

I'm working on a Jetpack Compose project where I use MutableStateFlow to display a Toast message when saving an article in the DetailScreen. However, when I navigate back to the previous screen using the back button, the Toast message appears multiple times.

Here's a simplified version of my code:

@Composable
fun DetailScreen(
    article: CommonArticle?,
    navController: NavController,
    viewModel: DetailViewModel
) {
    val context = LocalContext.current
    val result = viewModel.result.collectAsStateWithLifecycle()
    Log.d("Test", result.value.toString()+" result ")

    if (article != null) {
        DetailScreenContent(
            article = article,
            navController = navController,
            context = context,
            viewModel = viewModel,
            result = result
        )
    } else {
        ErrorView()
    }
}

@Composable
private fun DetailScreenContent(
    article: CommonArticle,
    navController: NavController,
    context: Context,
    viewModel: DetailViewModel,
    result: State<Long>
) {

    Box(
        modifier = Modifier
            .fillMaxSize()
    ) {
        AsyncImage(
            model = ImageRequest.Builder(context).data(article.image ?: "").build(),
            contentDescription = null,
            modifier = Modifier
                .fillMaxWidth()
                .height(Dimens.ArticleImageHeight),
            contentScale = ContentScale.Crop
        )

        BackNavigation(
            modifier = Modifier.height(30.dp),
            onBackClick = {
                navController.navigateUp()
            }
        )

        Column {
            Spacer(
                modifier = Modifier
                    .height(Dimens.ArticleImageHeight - 50.dp)
            )

            Card(
                modifier = Modifier
                    .fillMaxSize(),
                shape = RoundedCornerShape(topStart = 30.dp, topEnd = 30.dp),
                elevation = CardDefaults.cardElevation(10.dp),
                colors = CardDefaults.cardColors(containerColor = Color.White)
            ) {
                Spacer(modifier = Modifier.height(5.dp))

                DetailOptions(
                    onBrowsingClick = {
                        navController.navigate(Route.WebViewScreen.route)
                    },
                    onBookmarkClick = {
                        viewModel.saveArticle(article)
                    },
                    onShareClick = {
                        Intent(Intent.ACTION_SEND).also {
                            it.putExtra(Intent.EXTRA_TEXT, article.url)
                            it.type = "text/plain"
                            if (it.resolveActivity(context.packageManager) != null) {
                                context.startActivity(it)
                            }
                        }
                    }
                )

                Card(
                    elevation = CardDefaults.cardElevation(3.dp),
                    colors = CardDefaults.cardColors(containerColor = Color.Black),
                    modifier = Modifier.padding(all = 10.dp)
                ) {
                    Text(
                        text = article?.getArticleSource()?.name ?: "",
                        color = Color.White,
                        modifier = Modifier.padding(8.dp)
                    )
                }

                Spacer(
                    modifier = Modifier
                        .height(Dimens.MediumPadding1)
                )

                Text(
                    text = article.title ?: "",
                    style = MaterialTheme.typography.titleLarge,
                    modifier = Modifier.padding(all = 10.dp)
                )

                Spacer(modifier = Modifier.height(Dimens.SmallPadding1))

                Text(
                    text = article.description ?: "",
                    textAlign = TextAlign.Justify,
                    style = MaterialTheme.typography.bodyMedium,
                    modifier = Modifier
                        .padding(all = 10.dp)
                        .fillMaxWidth(),
                    overflow = TextOverflow.Ellipsis
                )
            }//: Card
        }

        if (result.value != -1L) {
            Toast.makeText(context, "Article Saved", Toast.LENGTH_SHORT).show()
        }
    }//: Box
}
@HiltViewModel
class DetailViewModel @Inject constructor(
    private val newsUseCases: NewsUseCases
): ViewModel() {

    private val _result = MutableStateFlow(-1L)
    val result = _result.asStateFlow()

    fun saveArticle(article: CommonArticle) {
        when(article) {
            is Article -> {
                saveArticleInDB(article)
            }
        }
    }

    private fun saveArticleInDB(article: Article) {
        viewModelScope.launch(Dispatchers.IO) {
            val result = newsUseCases.upsertArticle(article)
            withContext(Dispatchers.Main) {
                _result.value = result
            }
        }
    }

}

I tried adding logs to see when the value is being collected, and it seems to trigger multiple times.

type here

Upvotes: 1

Views: 172

Answers (2)

BenjyTec
BenjyTec

Reputation: 10887

The code snippet

if (result.value != -1L) {
    Toast.makeText(context, "Article Saved", Toast.LENGTH_SHORT).show()
}

will be executed at each single recomposition. During navigation, it can happen that the target Composable is recomposed multiple times due to the default animations applied by the NavHost.

Thus we need to make sure that this code is only executed once at the first recomposition, and only then again when result.value changes. We can achieve this by using a LaunchedEffect:

LaunchedEffect(result.value) {
    if (result.value != -1L) {
        Toast.makeText(context, "Article Saved", Toast.LENGTH_SHORT).show()
    }
}

Please also note that it is good practice not to pass around State<> objects to child Composables. It might be tempting to do so, as you then can directly modify the state.value in the child Composable, but that violates the unidirectional data flow pattern.
So instead, change your code like this:

@Composable
private fun DetailScreenContent(
    article: CommonArticle,
    navController: NavController,
    context: Context,
    viewModel: DetailViewModel,
    result: Long
) {
    //..
    LaunchedEffect(result) {
        if (result != -1L) {
            Toast.makeText(context, "Article Saved", Toast.LENGTH_SHORT).show()
        }
    }
}

And call your DetailScreenContent Composable accordingly:

DetailScreenContent(
    article = article,
    navController = navController,
    context = context,
    viewModel = viewModel,
    result = result.value
)

Upvotes: 0

responsecode444
responsecode444

Reputation: 11

if (result.value != -1L) {
        Toast.makeText(context, "Article Saved", Toast.LENGTH_SHORT).show()
    }

This should be used like this instead:

val resultValue = remember(result) { result.value }
if (resultValue != -1L) {
        Toast.makeText(context, "Article Saved", Toast.LENGTH_SHORT).show()
    }

Upvotes: 0

Related Questions