Skip to content
Open
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
Expand Up @@ -16,9 +16,13 @@

package com.example.jetnews.ui.home

import androidx.lifecycle.SavedStateHandle
import androidx.lifecycle.ViewModel
import androidx.lifecycle.ViewModelProvider
import androidx.lifecycle.createSavedStateHandle
import androidx.lifecycle.viewModelScope
import androidx.lifecycle.viewmodel.initializer
import androidx.lifecycle.viewmodel.viewModelFactory
import com.example.jetnews.R
import com.example.jetnews.data.Result
import com.example.jetnews.data.posts.PostsRepository
Expand All @@ -28,6 +32,7 @@ import com.example.jetnews.utils.ErrorMessage
import java.util.UUID
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.flow.update
Expand Down Expand Up @@ -75,7 +80,7 @@ sealed interface HomeUiState {
*/
private data class HomeViewModelState(
val postsFeed: PostsFeed? = null,
val selectedPostId: String? = null, // TODO back selectedPostId in a SavedStateHandle
val selectedPostId: String? = null,
val isArticleOpen: Boolean = false,
val favorites: Set<String> = emptySet(),
val isLoading: Boolean = false,
Expand Down Expand Up @@ -114,13 +119,23 @@ private data class HomeViewModelState(
/**
* ViewModel that handles the business logic of the Home screen
*/
class HomeViewModel(private val postsRepository: PostsRepository, preSelectedPostId: String?) : ViewModel() {
class HomeViewModel(
private val postsRepository: PostsRepository,
private val savedStateHandle: SavedStateHandle,
preSelectedPostId: String?,
) : ViewModel() {

// StateFlow backed by SavedStateHandle for selectedPostId persistence across process death
private val selectedPostIdState: StateFlow<String?> = savedStateHandle.getStateFlow(
key = SELECTED_POST_ID_KEY,
initialValue = preSelectedPostId,
)

private val viewModelState = MutableStateFlow(
HomeViewModelState(
isLoading = true,
selectedPostId = preSelectedPostId,
isArticleOpen = preSelectedPostId != null,
selectedPostId = selectedPostIdState.value,
isArticleOpen = selectedPostIdState.value != null,
),
)

Expand All @@ -142,6 +157,18 @@ class HomeViewModel(private val postsRepository: PostsRepository, preSelectedPos
viewModelState.update { it.copy(favorites = favorites) }
}
}

// Observe selectedPostId changes from SavedStateHandle
viewModelScope.launch {
selectedPostIdState.collect { selectedPostId ->
viewModelState.update {
it.copy(
selectedPostId = selectedPostId,
isArticleOpen = selectedPostId != null,
)
}
}
}
Comment on lines +162 to +171
Copy link
Contributor

Choose a reason for hiding this comment

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

high

This collector introduces a bug where the UI state for isArticleOpen is not correctly restored after a configuration change or process death. By deriving isArticleOpen solely from selectedPostId != null, it incorrectly forces the article to be displayed even if the user had previously closed it and returned to the feed.

To fix this, the isArticleOpen state must also be persisted in SavedStateHandle. This requires a more comprehensive change across the ViewModel. Here's a step-by-step guide:

  1. Define a new key for isArticleOpen in the companion object:

    private const val IS_ARTICLE_OPEN_KEY = "isArticleOpen"
  2. Create a StateFlow for isArticleOpen from SavedStateHandle, right below selectedPostIdState:

    private val isArticleOpenState: StateFlow<Boolean> = savedStateHandle.getStateFlow(
        key = IS_ARTICLE_OPEN_KEY,
        initialValue = preSelectedPostId != null
    )
  3. Update viewModelState initialization (around line 135) to use the new flow's value:

    private val viewModelState = MutableStateFlow(
        HomeViewModelState(
            isLoading = true,
            selectedPostId = selectedPostIdState.value,
            isArticleOpen = isArticleOpenState.value
        )
    )
  4. Update interactedWithArticleDetails() (around line 237) to set both states in SavedStateHandle:

    fun interactedWithArticleDetails(postId: String) {
        savedStateHandle[SELECTED_POST_ID_KEY] = postId
        savedStateHandle[IS_ARTICLE_OPEN_KEY] = true
    }
  5. Update interactedWithFeed() to also write to SavedStateHandle. Note: This function is not part of the PR changes, but it's a necessary part of the fix.

    fun interactedWithFeed() {
        savedStateHandle[IS_ARTICLE_OPEN_KEY] = false
    }
  6. Replace this collector with two separate collectors, one for each state flow, to correctly update viewModelState.

        // Observe selectedPostId changes from SavedStateHandle
        viewModelScope.launch {
            selectedPostIdState.collect { selectedPostId ->
                viewModelState.update { it.copy(selectedPostId = selectedPostId) }
            }
        }

        // Observe isArticleOpen changes from SavedStateHandle
        viewModelScope.launch {
            isArticleOpenState.collect { isArticleOpen ->
                viewModelState.update { it.copy(isArticleOpen = isArticleOpen) }
            }
        }

}

/**
Expand Down Expand Up @@ -208,12 +235,8 @@ class HomeViewModel(private val postsRepository: PostsRepository, preSelectedPos
* Notify that the user interacted with the article details
*/
fun interactedWithArticleDetails(postId: String) {
viewModelState.update {
it.copy(
selectedPostId = postId,
isArticleOpen = true,
)
}
// Save to SavedStateHandle for persistence across process death
savedStateHandle[SELECTED_POST_ID_KEY] = postId
}

/**
Expand All @@ -229,12 +252,19 @@ class HomeViewModel(private val postsRepository: PostsRepository, preSelectedPos
* Factory for HomeViewModel that takes PostsRepository as a dependency
*/
companion object {
fun provideFactory(postsRepository: PostsRepository, preSelectedPostId: String? = null): ViewModelProvider.Factory =
object : ViewModelProvider.Factory {
@Suppress("UNCHECKED_CAST")
override fun <T : ViewModel> create(modelClass: Class<T>): T {
return HomeViewModel(postsRepository, preSelectedPostId) as T
}
private const val SELECTED_POST_ID_KEY = "selectedPostId"

fun provideFactory(
postsRepository: PostsRepository,
preSelectedPostId: String? = null,
): ViewModelProvider.Factory = viewModelFactory {
initializer {
HomeViewModel(
postsRepository = postsRepository,
savedStateHandle = createSavedStateHandle(),
preSelectedPostId = preSelectedPostId,
)
}
}
}
}