Fix color picker scroll position resets on select (1/2)
After a color option is selected in the color picker, the scroll
position jumps back to the start, rather than saving and restoring the
scroll position using save and restore instance state. The regression
was caused by a combination of 1) color picker restarting twice from
config change updates, 2) the view.post callback not being removed when
the view is destroyed, leading to two consecutive calls, and 3) recycler
view's restore instance state was called before the data was fully
binded. The code to update color options on selection was
also removed for now because it interferes with the config change
restart and saving instance state. This should be brought back once the
config change restart is disabled.
Bug: 298898592
Test: manually verified
Change-Id: Ic694fb3e73dffee1d5b8bcac8e417de886e7a0ae
diff --git a/src/com/android/customization/picker/color/data/repository/ColorPickerRepositoryImpl.kt b/src/com/android/customization/picker/color/data/repository/ColorPickerRepositoryImpl.kt
index ce5c0c4..6540ce0 100644
--- a/src/com/android/customization/picker/color/data/repository/ColorPickerRepositoryImpl.kt
+++ b/src/com/android/customization/picker/color/data/repository/ColorPickerRepositoryImpl.kt
@@ -51,11 +51,9 @@
private val _isApplyingSystemColor = MutableStateFlow(false)
override val isApplyingSystemColor = _isApplyingSystemColor.asStateFlow()
+ // TODO (b/299510645): update color options on selected option change after restart is disabled
override val colorOptions: Flow<Map<ColorType, List<ColorOptionModel>>> =
- combine(homeWallpaperColors, lockWallpaperColors, selectedColorOption) {
- homeColors,
- lockColors,
- _ ->
+ combine(homeWallpaperColors, lockWallpaperColors) { homeColors, lockColors ->
homeColors to lockColors
}
.map { (homeColors, lockColors) ->
diff --git a/src/com/android/customization/picker/color/ui/binder/ColorPickerBinder.kt b/src/com/android/customization/picker/color/ui/binder/ColorPickerBinder.kt
index cd9dd54..0f82f49 100644
--- a/src/com/android/customization/picker/color/ui/binder/ColorPickerBinder.kt
+++ b/src/com/android/customization/picker/color/ui/binder/ColorPickerBinder.kt
@@ -93,29 +93,21 @@
launch {
viewModel.colorOptions.collect { colorOptions ->
- colorOptionAdapter.setItems(colorOptions)
- // the same recycler view is used for different color types tabs
- // the scroll state of each tab should be independent of others
- if (layoutManagerSavedState != null) {
- colorOptionContainerView.post {
+ // only set or restore instance state on a recycler view once data binding
+ // is complete to ensure scroll position is reflected correctly
+ colorOptionAdapter.setItems(colorOptions) {
+ // the same recycler view is used for different color types tabs
+ // the scroll state of each tab should be independent of others
+ if (layoutManagerSavedState != null) {
(colorOptionContainerView.layoutManager as LinearLayoutManager)
.onRestoreInstanceState(layoutManagerSavedState)
layoutManagerSavedState = null
+ } else {
+ var indexToFocus = colorOptions.indexOfFirst { it.isSelected.value }
+ indexToFocus = if (indexToFocus < 0) 0 else indexToFocus
+ (colorOptionContainerView.layoutManager as LinearLayoutManager)
+ .scrollToPositionWithOffset(indexToFocus, 0)
}
- } else {
- var indexToFocus = colorOptions.indexOfFirst { it.isSelected.value }
- indexToFocus = if (indexToFocus < 0) 0 else indexToFocus
- val linearLayoutManager =
- object : LinearLayoutManager(view.context, HORIZONTAL, false) {
- override fun onLayoutCompleted(state: RecyclerView.State?) {
- super.onLayoutCompleted(state)
- // scrollToPosition seems to be inconsistently moving
- // selected
- // color to different positions
- scrollToPositionWithOffset(indexToFocus, 0)
- }
- }
- colorOptionContainerView.layoutManager = linearLayoutManager
}
}
}
@@ -123,9 +115,13 @@
}
return object : Binding {
override fun saveInstanceState(savedState: Bundle) {
+ // as a workaround for the picker restarting twice after a config change, if the
+ // picker restarts before the saved state was applied and set to null,
+ // re-use the same saved state
savedState.putParcelable(
LAYOUT_MANAGER_SAVED_STATE,
- colorOptionContainerView.layoutManager?.onSaveInstanceState()
+ layoutManagerSavedState
+ ?: colorOptionContainerView.layoutManager?.onSaveInstanceState()
)
}