Reputation: 11
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
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
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