material-components / material-components-android

Modular and customizable Material Design UI components for Android
Apache License 2.0
16.38k stars 3.08k forks source link

[TopAppBar] Color effect when scrolling is lost in case list item of RecyclerView is updated #2825

Closed patrickfrei closed 2 years ago

patrickfrei commented 2 years ago

Description: In my app, I'm using AppBarLayout, MaterialToolbar and TabLayout similar to the Material Component Catalog example TopAppBarCompressEffectFragment.java and cat_topappbar_compress_effect_fragment.xml.

My app has also implemented the same effects to automatically change the colors of these elements when the user is scrolling the UI up and down. In my case, the scrollable part is a ViewPager2 plus a RecyclerView for each fragment / tab in order to make the UI horizontally and vertically swipable. See source code: https://github.com/patrickfrei/test_coding

When I'm scrolling the UI up and down, I've noticed that as soon as a list item from the RecyclerView is refreshing its content, the color of AppBarLayout, MaterialToolbar and TabLayout is reset to their default. If you then scroll up and down the list a little bit, the color of the elements is reinstated. If a list item of the RecyclerView has to be refreshed e.g. every one second and the user is at the same time scrolling up and down the list, the aforementioned element colors will always be reset and set again what causes the UI to flicker. See here for an example where list item nr. 4 is updated every three seconds for illustrative purposes: https://github.com/patrickfrei/test_coding/blob/main/issue.mp4

Expected behavior: The colors of AppBarLayout, MaterialToolbar and TabLayout shouldn't be reset to their default when the user is scrolling and a list item of the RecyclerView is updated. The color effect should remain stable. Is there any workaround for that problem?

Source code:

<?xml version="1.0" encoding="utf-8"?>
<androidx.coordinatorlayout.widget.CoordinatorLayout xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:app="http://schemas.android.com/apk/res-auto"
    xmlns:tools="http://schemas.android.com/tools"
    android:id="@+id/main_content"
    android:layout_width="match_parent"
    android:layout_height="match_parent"
    android:fitsSystemWindows="true"
    tools:context=".MainActivity">

    <com.google.android.material.appbar.AppBarLayout
        android:id="@+id/appbar"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        android:fitsSystemWindows="true">

        <com.google.android.material.appbar.MaterialToolbar
            android:id="@+id/toolbar"
            android:layout_width="match_parent"
            android:layout_height="wrap_content"
            app:layout_scrollFlags="scroll|enterAlways|snap"
            app:layout_scrollEffect="compress"
            app:title="MyApp" />

        <com.google.android.material.tabs.TabLayout
            android:id="@+id/tabs"
            android:layout_width="match_parent"
            android:layout_height="wrap_content"
            app:layout_scrollFlags="scroll|enterAlways"
            android:background="@android:color/transparent"
            app:tabMode="scrollable" />

    </com.google.android.material.appbar.AppBarLayout>

    <androidx.viewpager2.widget.ViewPager2
        android:id="@+id/container"
        android:layout_width="match_parent"
        android:layout_height="match_parent"
        app:layout_behavior="@string/appbar_scrolling_view_behavior" />

</androidx.coordinatorlayout.widget.CoordinatorLayout>

Minimal sample app repro:

Android API version: API level 33 (but also in lower API levels)

Material Library version: 1.7.0-alpha03

Device: Pixel 5 emulator

dsn5ft commented 2 years ago

Hi @patrickfrei,

This should be a duplicate of https://github.com/material-components/material-components-android/issues/163 and https://github.com/material-components/material-components-android/issues/2591.

Basically you can set app:liftOnScrollTargetViewId="@+id/your_recycler_view_id" on your AppBarLayout to ensure that it uses the RecyclerView to determine the lifted state, instead of the ViewPager which can have difficulty correctly passing up the nested scrolling information.

There's also a note about this in the AppBarLayout docs here.

Closing the issue for now since it seems like the exact same issue, but let us know if that works and if not feel free to reopen.

patrickfrei commented 2 years ago

Hi @dsn5ft,

Many thanks for your reply!

I've just read the AppBarLayout docs and added app:liftOnScrollTargetViewId="@+id/tab1ListView" to my sample app as you have suggested. It's working perfectly for the first tab. But the second tab has now no color effect anymore as the RecyclerView's name of the second tab is tab2ListView (I have to use unique ids for all RecyclerView's). So, I've tried to add multiple ids to app:liftOnScrollTargetViewId by using the pipe symbol but that doesn't work...

Is there also a workaround for such a case with multiple RecyclerView's with different ids?

dsn5ft commented 2 years ago

Np! So instead of setting the id in XML, can you try calling AppBarLayout#setLiftOnScrollTargetViewId whenever the tab is changed? That way AppBarLayout can find the correct RecyclerView.

patrickfrei commented 2 years ago

Well, yes that's it... the question is how I can dynamically get the id of the RecyclerView that is below the currently select tab. Maybe, I can determine it by using registerOnPageChangeCallback

dsn5ft commented 2 years ago

Do you need to get the id of the RecyclerView dynamically? Instead can you have a static mapping of tab -> RecyclerView id?

patrickfrei commented 2 years ago

No, I don't really need to get the id dynamically. It was just for convenience because having a static mapping table always causes additional maintenance, but in my case there are not many tab ids to maintain. So, I've now implemented a static mapping table and I'm calling AppBarLayout#setLiftOnScrollTargetViewId whenever the tab has changed.

It's working perfectly fine 😃 Many thanks for your help!

dsn5ft commented 2 years ago

Great! No problem. The only other way I can think of doing it more dynamically is searching through the ViewPager's child views, checking for instanceof RecyclerView, and then doing appBarLayout.setLiftOnScrollTargetViewId(childView.getId()). It should be relatively easy if the RecyclerView is always a direct child of the ViewPager. If not then you might have to search down the hierarchy more recursively.

patrickfrei commented 2 years ago

Hi @dsn5ft,

Sorry to bother you again, but I've made some further tests and the UI is still slightly flickering, but only when switching from one tab to another. I've updated the code and the video to show you the effect:

The flickering just occurs when two adjacent tabs are in a different scroll state: The color effect is applied on tab 1 because the user is somewhere in the middle of the list and the color effect is NOT applied on tab2 because the user is at the top of the list. I think that the issue occurs due to the list being updated twice (once in the background due to adapter.notifyDataSetChanged() and then probably once again from the Material Design code that handles the color effect).

It's not a big issue but do you see any chance to get rid of the flickering in this particular case?

dsn5ft commented 2 years ago

Hmm I'm not really sure, we'd probably need to debug with some breakpoints to figure out exactly what scroll-related events are being fired to cause the lifted state to change.

patrickfrei commented 2 years ago

Hi @dsn5ft , I've made some further tests because debugging the Material Design / ViewPagerAdapter coding exceeds my knowledge: 1) I've used registerOnPageChangeCallback (ViewPagerAdapter) instead of addOnTabSelectedListener (TabLayout). This change didn't have an effect, i.e. the layout was still flickering. 2) Then, I've removed the background loading logic of the list, basically adapter.notifyDataSetChanged(). It's clearly that part that causes the flickering in combination with the Material Design code that handles the color effect, because as soon as I've removed the according code (assuming that the list is static) and loaded the list directly in the foreground, the flickering stopped (horizontally & vertically).

patrickfrei commented 2 years ago

Hi @dsn5ft,

I don't know if you're still looking into that issue, but in the meantime, I've further debugged my (test) app and stick to my explanations that the flickering AppBar / TabLayout is caused by adapter.notifyDataSetChanged(). Lifecycle: In the first step, appBarLayout.setLiftOnScrollTargetViewId is set and the tab is changed to one with a colored background. In the second step, adapter.notifyDataSetChanged() causes the list to be refreshed. As the TabLayout is transparent and the list background is white, I suppose that the white background of the list is quickly shown. In my opinion, this causes the UI to flicker.

Interestingly, I've found out that setting appBarLayout.forceLayout(); right after calling appBarLayout.setLiftOnScrollTargetViewId(recyclerviewId); is stopping that issue in 99% of the cases.

Latest code: https://github.com/patrickfrei/test_coding

If you're interested in my approach or think that there is a better one, please let me know!

dsn5ft commented 2 years ago

Thanks for continuing to debug, providing all of this detail, and for filing https://github.com/material-components/material-components-android/issues/2856 separately. We'll post updates there if we have any.