Skip to content

Update SettingsViewModel.kt #1897

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+25 −0
Open

Update SettingsViewModel.kt #1897

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9efe95d
Update SettingsViewModel.kt
yannigoyal Apr 23, 2025
7d6c98e
Update SettingsViewModel.kt
yannigoyal Apr 23, 2025
0ae715a
Update SettingsViewModel.kt
yannigoyal Apr 23, 2025
7c9c983
Update SettingsViewModel.kt
yannigoyal Apr 24, 2025
8bacb1c
Update SettingsViewModel.kt
yannigoyal Jul 11, 2025
bc1007d
indent
yannigoyal Jul 11, 2025
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
25 changes: 25 additions & 0 deletions ...rc/main/kotlin/com/google/samples/apps/nowinandroid/feature/settings/SettingsViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,23 @@

package com.google.samples.apps.nowinandroid.feature.settings

import android.app.UiModeManager
import android.content.Context
import android.os.Build
import android.os.Build.VERSION_CODES
import androidx.appcompat.app.AppCompatDelegate
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import com.google.samples.apps.nowinandroid.core.data.repository.UserDataRepository
import com.google.samples.apps.nowinandroid.core.model.data.DarkThemeConfig
import com.google.samples.apps.nowinandroid.core.model.data.DarkThemeConfig.DARK
import com.google.samples.apps.nowinandroid.core.model.data.DarkThemeConfig.FOLLOW_SYSTEM
import com.google.samples.apps.nowinandroid.core.model.data.DarkThemeConfig.LIGHT
import com.google.samples.apps.nowinandroid.core.model.data.ThemeBrand
import com.google.samples.apps.nowinandroid.feature.settings.SettingsUiState.Loading
import com.google.samples.apps.nowinandroid.feature.settings.SettingsUiState.Success
import dagger.hilt.android.lifecycle.HiltViewModel
import dagger.hilt.android.qualifiers.ApplicationContext
import kotlinx.coroutines.flow.SharingStarted.Companion.WhileSubscribed
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.map
Expand All @@ -35,6 +44,7 @@ import kotlin.time.Duration.Companion.seconds
@HiltViewModel
class SettingsViewModel @Inject constructor(
private val userDataRepository: UserDataRepository,
@ApplicationContext private val context: Context,
Copy link
Contributor

Choose a reason for hiding this comment

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

Exposing Context instances in ViewModels is generally a bad practice as it might lead to memory leak (here since we use @ApplicationContext it does not really leak anything), and facilitate accessing Context resources that might change through configuration changes that would not be reflected in the ViewModel scope.

Could we instead request a UiModeManager and @Provide it in a Hilt module? (or even extract this logic of applying a DarkThemeConfig altogether in a dedicated class?)

) : ViewModel() {
val settingsUiState: StateFlow<SettingsUiState> =
userDataRepository.userData
Expand Down Expand Up @@ -62,6 +72,21 @@ class SettingsViewModel @Inject constructor(
fun updateDarkThemeConfig(darkThemeConfig: DarkThemeConfig) {
viewModelScope.launch {
userDataRepository.setDarkThemeConfig(darkThemeConfig)
val splashMode = when (darkThemeConfig) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This variable name is quite misleading. Could we change it to nightMode?

FOLLOW_SYSTEM -> AppCompatDelegate.MODE_NIGHT_FOLLOW_SYSTEM
LIGHT -> AppCompatDelegate.MODE_NIGHT_NO
DARK -> AppCompatDelegate.MODE_NIGHT_YES
}
AppCompatDelegate.setDefaultNightMode(splashMode)
if (Build.VERSION.SDK_INT >= VERSION_CODES.S) {
val uiModeMode = when (darkThemeConfig) {
Copy link
Contributor

Choose a reason for hiding this comment

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

uiModeMode should be renamed, why not simply uiMode or nightMode?

FOLLOW_SYSTEM -> UiModeManager.MODE_NIGHT_AUTO
LIGHT -> UiModeManager.MODE_NIGHT_NO
DARK -> UiModeManager.MODE_NIGHT_YES
}
(context.getSystemService(Context.UI_MODE_SERVICE) as? UiModeManager)
?.setApplicationNightMode(uiModeMode)
Comment on lines +87 to +88
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
(context.getSystemService(Context.UI_MODE_SERVICE) as? UiModeManager)
?.setApplicationNightMode(uiModeMode)
context.getSystemService<UiModeManager>()?.setApplicationNightMode(uiModeMode)

}
}
}

Expand Down
Loading