From de7fae785bd6d693c85e63e2590d62e6cc55bc6a Mon Sep 17 00:00:00 2001 From: Chaohui Wang Date: Mon, 16 Jan 2023 14:55:30 +0800 Subject: Fix the selected option not saved for App List The available options will be refreshed each time the app list is re-entered, which caused the selected option is cleared before. Only reset the selected option if it is not available any more to fix this issue. Bug: 264228237 Test: Manually with Settings Test: Unit test Change-Id: I6d7b65d0a944bf1ab392500d17966c697971ea64 --- .../spaprivileged/model/app/AppListViewModel.kt | 6 +++--- .../spaprivileged/template/app/AppList.kt | 20 +++++++++++--------- .../spaprivileged/model/app/AppListViewModelTest.kt | 2 +- .../spaprivileged/template/app/AppListTest.kt | 4 ++-- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/SettingsLib/SpaPrivileged/src/com/android/settingslib/spaprivileged/model/app/AppListViewModel.kt b/packages/SettingsLib/SpaPrivileged/src/com/android/settingslib/spaprivileged/model/app/AppListViewModel.kt index df828f2c0fa2..6cd1c0d6a5ae 100644 --- a/packages/SettingsLib/SpaPrivileged/src/com/android/settingslib/spaprivileged/model/app/AppListViewModel.kt +++ b/packages/SettingsLib/SpaPrivileged/src/com/android/settingslib/spaprivileged/model/app/AppListViewModel.kt @@ -51,7 +51,7 @@ internal data class AppListData( } internal interface IAppListViewModel { - val option: StateFlowBridge + val optionFlow: MutableStateFlow val spinnerOptionsFlow: Flow> val appListDataFlow: Flow> } @@ -69,7 +69,7 @@ internal open class AppListViewModelImpl( val appListConfig = StateFlowBridge() val listModel = StateFlowBridge>() val showSystem = StateFlowBridge() - final override val option = StateFlowBridge() + final override val optionFlow = MutableStateFlow(null) val searchQuery = StateFlowBridge() private val appListRepository = appListRepositoryFactory(application) @@ -97,7 +97,7 @@ internal open class AppListViewModelImpl( listModel.getSpinnerOptions(recordList) } - override val appListDataFlow = option.flow.flatMapLatest(::filterAndSort) + override val appListDataFlow = optionFlow.filterNotNull().flatMapLatest(::filterAndSort) .combine(searchQuery.flow) { appListData, searchQuery -> appListData.filter { it.label.contains(other = searchQuery, ignoreCase = true) diff --git a/packages/SettingsLib/SpaPrivileged/src/com/android/settingslib/spaprivileged/template/app/AppList.kt b/packages/SettingsLib/SpaPrivileged/src/com/android/settingslib/spaprivileged/template/app/AppList.kt index 34c3ee0e2c0c..7199e3a319fe 100644 --- a/packages/SettingsLib/SpaPrivileged/src/com/android/settingslib/spaprivileged/template/app/AppList.kt +++ b/packages/SettingsLib/SpaPrivileged/src/com/android/settingslib/spaprivileged/template/app/AppList.kt @@ -24,11 +24,10 @@ import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.State import androidx.compose.runtime.collectAsState -import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember -import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.Dp @@ -37,7 +36,6 @@ import com.android.settingslib.spa.framework.compose.LogCompositions import com.android.settingslib.spa.framework.compose.TimeMeasurer.Companion.rememberTimeMeasurer import com.android.settingslib.spa.framework.compose.rememberLazyListStateAndHideKeyboardWhenStartScroll import com.android.settingslib.spa.framework.compose.toState -import com.android.settingslib.spa.framework.util.StateFlowBridge import com.android.settingslib.spa.widget.ui.CategoryTitle import com.android.settingslib.spa.widget.ui.PlaceholderTitle import com.android.settingslib.spa.widget.ui.Spinner @@ -52,6 +50,7 @@ import com.android.settingslib.spaprivileged.model.app.AppListViewModel import com.android.settingslib.spaprivileged.model.app.AppRecord import com.android.settingslib.spaprivileged.model.app.IAppListViewModel import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.MutableStateFlow private const val TAG = "AppList" private const val CONTENT_TYPE_HEADER = "header" @@ -88,7 +87,7 @@ internal fun AppListInput.AppListImpl( val viewModel = viewModelSupplier() Column(Modifier.fillMaxSize()) { val optionsState = viewModel.spinnerOptionsFlow.collectAsState(null, Dispatchers.IO) - SpinnerOptions(optionsState, viewModel.option) + SpinnerOptions(optionsState, viewModel.optionFlow) val appListData = viewModel.appListDataFlow.collectAsState(null, Dispatchers.IO) listModel.AppListWidget(appListData, header, bottomPadding, noItemMessage) } @@ -97,15 +96,18 @@ internal fun AppListInput.AppListImpl( @Composable private fun SpinnerOptions( optionsState: State?>, - optionBridge: StateFlowBridge, + optionFlow: MutableStateFlow, ) { val options = optionsState.value - val selectedOption = rememberSaveable(options) { - mutableStateOf(options?.let { it.firstOrNull()?.id ?: -1 }) + LaunchedEffect(options) { + if (options != null && !options.any { it.id == optionFlow.value }) { + // Reset to first option if the available options changed, and the current selected one + // does not in the new options. + optionFlow.value = options.let { it.firstOrNull()?.id ?: -1 } + } } - optionBridge.Sync(selectedOption) if (options != null) { - Spinner(options, selectedOption.value) { selectedOption.value = it } + Spinner(options, optionFlow.collectAsState().value) { optionFlow.value = it } } } diff --git a/packages/SettingsLib/SpaPrivileged/tests/src/com/android/settingslib/spaprivileged/model/app/AppListViewModelTest.kt b/packages/SettingsLib/SpaPrivileged/tests/src/com/android/settingslib/spaprivileged/model/app/AppListViewModelTest.kt index f51448744c56..9b3d0f520690 100644 --- a/packages/SettingsLib/SpaPrivileged/tests/src/com/android/settingslib/spaprivileged/model/app/AppListViewModelTest.kt +++ b/packages/SettingsLib/SpaPrivileged/tests/src/com/android/settingslib/spaprivileged/model/app/AppListViewModelTest.kt @@ -56,7 +56,7 @@ class AppListViewModelTest { viewModel.appListConfig.setIfAbsent(CONFIG) viewModel.listModel.setIfAbsent(listModel) viewModel.showSystem.setIfAbsent(false) - viewModel.option.setIfAbsent(0) + viewModel.optionFlow.value = 0 viewModel.searchQuery.setIfAbsent("") viewModel.reloadApps() return viewModel diff --git a/packages/SettingsLib/SpaPrivileged/tests/src/com/android/settingslib/spaprivileged/template/app/AppListTest.kt b/packages/SettingsLib/SpaPrivileged/tests/src/com/android/settingslib/spaprivileged/template/app/AppListTest.kt index 2a1f7a4ad908..d5c7c191d3ff 100644 --- a/packages/SettingsLib/SpaPrivileged/tests/src/com/android/settingslib/spaprivileged/template/app/AppListTest.kt +++ b/packages/SettingsLib/SpaPrivileged/tests/src/com/android/settingslib/spaprivileged/template/app/AppListTest.kt @@ -29,7 +29,6 @@ import androidx.compose.ui.unit.dp import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import com.android.settingslib.spa.framework.compose.toState -import com.android.settingslib.spa.framework.util.StateFlowBridge import com.android.settingslib.spa.widget.ui.SpinnerOption import com.android.settingslib.spaprivileged.R import com.android.settingslib.spaprivileged.model.app.AppEntry @@ -38,6 +37,7 @@ import com.android.settingslib.spaprivileged.model.app.AppListData import com.android.settingslib.spaprivileged.model.app.IAppListViewModel import com.android.settingslib.spaprivileged.tests.testutils.TestAppListModel import com.android.settingslib.spaprivileged.tests.testutils.TestAppRecord +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flowOf import org.junit.Rule import org.junit.Test @@ -125,7 +125,7 @@ class AppListTest { bottomPadding = 0.dp, ).AppListImpl { object : IAppListViewModel { - override val option: StateFlowBridge = StateFlowBridge() + override val optionFlow: MutableStateFlow = MutableStateFlow(null) override val spinnerOptionsFlow = flowOf(options.mapIndexed { index, option -> SpinnerOption(id = index, text = option) }) -- cgit v1.2.3-59-g8ed1b