duckduckgo / Android

DuckDuckGo Android App
https://play.google.com/store/apps/details?id=com.duckduckgo.mobile.android
Apache License 2.0
3.77k stars 892 forks source link

Tab manager scrolling fix #4918

Closed 0nko closed 1 month ago

0nko commented 1 month ago

Task/Issue URL: https://app.asana.com/0/488551667048375/1208062551083727/f

Description

This PR fixes the initial scroll position after a tab manager is opened. The position should now be centered to the current tab.

The problem was that the call to scroll to the selected tab was in the render() function, which would not always happen before the LayoutManager was initialized because the LiveData observers were called in different/inconsistent order. This was causing the tabs to sometimes show the first tab when initially opening the tab manager.

The PR also improves the scroll position when switching between the list mode and grid mode, which should now be pretty much centered around the same middle item.

Steps to test this PR

I was able to reproduce this only when I had a lot of tabs open. If you don’t have many, you can add this to the TabSwitcherViewModel:

    init {
        viewModelScope.launch(dispatcherProvider.io()) {
            if (tabRepository.flowTabs.first().size < 100) {
                repeat(100) {
                    tabRepository.add("cnn.com")
                }
            }
        }
    }

Initial position

Configuration change

Switching view mode

0nko commented 1 month ago

Thanks for the review, @joshliebe! I’ve addressed both issues:

On these devices it shifts the list down every time I switch between the view modes.

I changed the way the current scroll offset is calculated to be more precise and consistent and this shouldn’t happen anymore.

It works as expected on a Pixel 7 Pro and a Nexus 6P. However, the transition between the two modes is no longer smooth (It jumps before landing on its final state).

I hide the RecyclerView until the layout is in its final state, which should solve this issue.

Ready for another round.