Fix color option selection indicator
After changing wallpaper, no color option shows as selected. This is
because the optimistic update value that is overriding the actual
selected color option was not reset to null after the update is
complete. In addition to resetting the optimistic update value to null
at the right time, the shareIn function to convert flows to SharedFlows
is removed since it was preventing updates from flowing consistently
from the repository to the view model.
Flag: NA
Test: manually verified that the expected color option is selected after
a wallpaper change
Test: regression tested color picker features including pausing and
resuming app and changing light/dark theme, no regressions observed
Bug: 318050029
Change-Id: Ia26d0d5fd763a55d8421bacd9b9a8ebdb6085862
diff --git a/src/com/android/customization/picker/color/domain/interactor/ColorPickerInteractor.kt b/src/com/android/customization/picker/color/domain/interactor/ColorPickerInteractor.kt
index d3b2eba..e7759ce 100644
--- a/src/com/android/customization/picker/color/domain/interactor/ColorPickerInteractor.kt
+++ b/src/com/android/customization/picker/color/domain/interactor/ColorPickerInteractor.kt
@@ -20,6 +20,8 @@
import com.android.customization.picker.color.shared.model.ColorOptionModel
import javax.inject.Provider
import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.asStateFlow
+import kotlinx.coroutines.flow.onEach
/** Single entry-point for all application state and business logic related to system color. */
class ColorPickerInteractor(
@@ -30,20 +32,28 @@
/**
* The newly selected color option for overwriting the current active option during an
- * optimistic update, the value is set to null when update fails
+ * optimistic update, the value is set to null when update completes
*/
- val activeColorOption = MutableStateFlow<ColorOptionModel?>(null)
+ private val _selectingColorOption = MutableStateFlow<ColorOptionModel?>(null)
+ val selectingColorOption = _selectingColorOption.asStateFlow()
/** List of wallpaper and preset color options on the device, categorized by Color Type */
- val colorOptions = repository.colorOptions
+ val colorOptions =
+ repository.colorOptions.onEach {
+ // Reset optimistic update value when colorOptions updates
+ _selectingColorOption.value = null
+ }
suspend fun select(colorOptionModel: ColorOptionModel) {
- activeColorOption.value = colorOptionModel
+ _selectingColorOption.value = colorOptionModel
try {
+ // Do not reset optimistic update selection on selection success because UI color is not
+ // actually updated until the picker restarts. Wait to do so when updated color options
+ // become available
repository.select(colorOptionModel)
snapshotRestorer.get().storeSnapshot(colorOptionModel)
} catch (e: Exception) {
- activeColorOption.value = null
+ _selectingColorOption.value = null
}
}
diff --git a/src/com/android/customization/picker/color/ui/viewmodel/ColorPickerViewModel.kt b/src/com/android/customization/picker/color/ui/viewmodel/ColorPickerViewModel.kt
index 32e9362..52df31a 100644
--- a/src/com/android/customization/picker/color/ui/viewmodel/ColorPickerViewModel.kt
+++ b/src/com/android/customization/picker/color/ui/viewmodel/ColorPickerViewModel.kt
@@ -31,11 +31,9 @@
import kotlin.math.min
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
-import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.map
-import kotlinx.coroutines.flow.shareIn
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch
@@ -95,122 +93,97 @@
/** The list of all color options mapped by their color type */
private val allColorOptions:
Flow<Map<ColorType, List<OptionItemViewModel<ColorOptionIconViewModel>>>> =
- interactor.colorOptions
- .map { colorOptions ->
- colorOptions
- .map { colorOptionEntry ->
- colorOptionEntry.key to
- colorOptionEntry.value.map { colorOptionModel ->
- val colorOption: ColorOptionImpl =
- colorOptionModel.colorOption as ColorOptionImpl
- val lightThemeColors =
- colorOption.previewInfo.resolveColors(/* darkTheme= */ false)
- val darkThemeColors =
- colorOption.previewInfo.resolveColors(/* darkTheme= */ true)
- val isSelectedFlow: StateFlow<Boolean> =
- interactor.activeColorOption
- .map {
- it?.colorOption?.isEquivalent(
- colorOptionModel.colorOption
- )
- ?: colorOptionModel.isSelected
- }
- .stateIn(viewModelScope)
- OptionItemViewModel<ColorOptionIconViewModel>(
- key =
- MutableStateFlow(colorOptionModel.key) as StateFlow<String>,
- payload =
- ColorOptionIconViewModel(
- lightThemeColor0 = lightThemeColors[0],
- lightThemeColor1 = lightThemeColors[1],
- lightThemeColor2 = lightThemeColors[2],
- lightThemeColor3 = lightThemeColors[3],
- darkThemeColor0 = darkThemeColors[0],
- darkThemeColor1 = darkThemeColors[1],
- darkThemeColor2 = darkThemeColors[2],
- darkThemeColor3 = darkThemeColors[3],
- ),
- text =
- Text.Loaded(
- colorOption.getContentDescription(context).toString()
- ),
- isTextUserVisible = false,
- isSelected = isSelectedFlow,
- onClicked =
- isSelectedFlow.map { isSelected ->
- if (isSelected) {
- null
- } else {
- {
- viewModelScope.launch {
- interactor.select(colorOptionModel)
- logger.logThemeColorApplied(
- colorOptionModel.colorOption
- .sourceForLogging,
- colorOptionModel.colorOption
- .styleForLogging,
- colorOptionModel.colorOption
- .seedColorForLogging,
- )
- }
+ interactor.colorOptions.map { colorOptions ->
+ colorOptions
+ .map { colorOptionEntry ->
+ colorOptionEntry.key to
+ colorOptionEntry.value.map { colorOptionModel ->
+ val colorOption: ColorOptionImpl =
+ colorOptionModel.colorOption as ColorOptionImpl
+ val lightThemeColors =
+ colorOption.previewInfo.resolveColors(/* darkTheme= */ false)
+ val darkThemeColors =
+ colorOption.previewInfo.resolveColors(/* darkTheme= */ true)
+ val isSelectedFlow: StateFlow<Boolean> =
+ interactor.selectingColorOption
+ .map {
+ it?.colorOption?.isEquivalent(colorOptionModel.colorOption)
+ ?: colorOptionModel.isSelected
+ }
+ .stateIn(viewModelScope)
+ OptionItemViewModel<ColorOptionIconViewModel>(
+ key = MutableStateFlow(colorOptionModel.key) as StateFlow<String>,
+ payload =
+ ColorOptionIconViewModel(
+ lightThemeColor0 = lightThemeColors[0],
+ lightThemeColor1 = lightThemeColors[1],
+ lightThemeColor2 = lightThemeColors[2],
+ lightThemeColor3 = lightThemeColors[3],
+ darkThemeColor0 = darkThemeColors[0],
+ darkThemeColor1 = darkThemeColors[1],
+ darkThemeColor2 = darkThemeColors[2],
+ darkThemeColor3 = darkThemeColors[3],
+ ),
+ text =
+ Text.Loaded(
+ colorOption.getContentDescription(context).toString()
+ ),
+ isTextUserVisible = false,
+ isSelected = isSelectedFlow,
+ onClicked =
+ isSelectedFlow.map { isSelected ->
+ if (isSelected) {
+ null
+ } else {
+ {
+ viewModelScope.launch {
+ interactor.select(colorOptionModel)
+ logger.logThemeColorApplied(
+ colorOptionModel.colorOption
+ .sourceForLogging,
+ colorOptionModel.colorOption
+ .styleForLogging,
+ colorOptionModel.colorOption
+ .seedColorForLogging,
+ )
}
}
- },
- )
- }
- }
- .toMap()
- }
- .shareIn(
- scope = viewModelScope,
- started = SharingStarted.WhileSubscribed(),
- replay = 1,
- )
+ }
+ },
+ )
+ }
+ }
+ .toMap()
+ }
/** The list of all available color options for the selected Color Type. */
val colorOptions: Flow<List<OptionItemViewModel<ColorOptionIconViewModel>>> =
combine(allColorOptions, selectedColorTypeTabId) {
- allColorOptions:
- Map<ColorType, List<OptionItemViewModel<ColorOptionIconViewModel>>>,
- selectedColorTypeIdOrNull ->
- val selectedColorTypeId = selectedColorTypeIdOrNull ?: ColorType.WALLPAPER_COLOR
- allColorOptions[selectedColorTypeId]!!
- }
- .shareIn(
- scope = viewModelScope,
- started = SharingStarted.Eagerly,
- replay = 1,
- )
+ allColorOptions: Map<ColorType, List<OptionItemViewModel<ColorOptionIconViewModel>>>,
+ selectedColorTypeIdOrNull ->
+ val selectedColorTypeId = selectedColorTypeIdOrNull ?: ColorType.WALLPAPER_COLOR
+ allColorOptions[selectedColorTypeId]!!
+ }
/** The list of color options for the color section */
val colorSectionOptions: Flow<List<OptionItemViewModel<ColorOptionIconViewModel>>> =
- allColorOptions
- .map { allColorOptions ->
- val wallpaperOptions = allColorOptions[ColorType.WALLPAPER_COLOR]
- val presetOptions = allColorOptions[ColorType.PRESET_COLOR]
- val subOptions =
- wallpaperOptions!!.subList(
- 0,
- min(COLOR_SECTION_OPTION_SIZE, wallpaperOptions.size)
+ allColorOptions.map { allColorOptions ->
+ val wallpaperOptions = allColorOptions[ColorType.WALLPAPER_COLOR]
+ val presetOptions = allColorOptions[ColorType.PRESET_COLOR]
+ val subOptions =
+ wallpaperOptions!!.subList(0, min(COLOR_SECTION_OPTION_SIZE, wallpaperOptions.size))
+ // Add additional options based on preset colors if size of wallpaper color options is
+ // less than COLOR_SECTION_OPTION_SIZE
+ val additionalSubOptions =
+ presetOptions!!.subList(
+ 0,
+ min(
+ max(0, COLOR_SECTION_OPTION_SIZE - wallpaperOptions.size),
+ presetOptions.size,
)
- // Add additional options based on preset colors if size of wallpaper color options
- // is
- // less than COLOR_SECTION_OPTION_SIZE
- val additionalSubOptions =
- presetOptions!!.subList(
- 0,
- min(
- max(0, COLOR_SECTION_OPTION_SIZE - wallpaperOptions.size),
- presetOptions.size,
- )
- )
- subOptions + additionalSubOptions
- }
- .shareIn(
- scope = viewModelScope,
- started = SharingStarted.WhileSubscribed(),
- replay = 1,
- )
+ )
+ subOptions + additionalSubOptions
+ }
class Factory(
private val context: Context,