Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 698275c3 authored by Catherine Liang's avatar Catherine Liang
Browse files

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
parent ee17d9a1

src/com/android/customization/picker/color/data/repository/ColorPickerRepositoryImpl.kt

+2 −4
Original line number Diff line number Diff line
@@ -51,11 +51,9 @@ class ColorPickerRepositoryImpl( 
    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) ->

src/com/android/customization/picker/color/ui/binder/ColorPickerBinder.kt

+16 −20
Original line number Diff line number Diff line
@@ -93,29 +93,21 @@ object ColorPickerBinder { 


                launch {

                    viewModel.colorOptions.collect { colorOptions ->

                        colorOptionAdapter.setItems(colorOptions)

                        // 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.post {

                                (colorOptionContainerView.layoutManager as LinearLayoutManager)

                                    .onRestoreInstanceState(layoutManagerSavedState)

                                layoutManagerSavedState = null

                            }

                            } 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 as LinearLayoutManager)

                                    .scrollToPositionWithOffset(indexToFocus, 0)

                            }

                            colorOptionContainerView.layoutManager = linearLayoutManager

                        }

                    }

                }
@@ -123,9 +115,13 @@ object ColorPickerBinder { 
        }

        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()

                )

            }