[question] Use setHasFixedSize list will not be displayed

tcqq commented 4 years ago

@ibrahimyilmaz If the RecyclerView layout uses wrap_content instead of match_parent and setHasFixedSize = true, the list will not be displayed.

activity_country_indonesia.xml

<?xml version="1.0" encoding="utf-8"?>
<LinearLayout 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:layout_width="match_parent"
    android:layout_height="match_parent"
    android:orientation="vertical">

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

        <androidx.appcompat.widget.Toolbar
            android:id="@+id/toolbar"
            android:layout_width="match_parent"
            android:layout_height="wrap_content" />

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

    <androidx.recyclerview.widget.RecyclerView
        android:id="@+id/recycler_view"
        android:layout_width="match_parent"
        android:layout_height="match_parent" / "wrap_content"
        app:layoutManager="androidx.recyclerview.widget.LinearLayoutManager"
        tools:listitem="@layout/item_task_detail" />

</LinearLayout>

CountryIndonesiaActivity.kt

class CountryIndonesiaActivity : BaseActivity() {
    private val viewModel by viewModel<CountryIndonesiaViewModel>()
    private lateinit var binding: ActivityCountryIndonesiaBinding
    private val viewAdapter by lazy { createAdapter(::onItemClicked) }

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        binding = ActivityCountryIndonesiaBinding.inflate(layoutInflater)
        setContentView(binding.root)
        setupActionBarWithBackButton(binding.toolbar)
        initView()

        viewModel.loadData()
    }

    private fun initView() {
        with(binding.recyclerView) {
            adapter = viewAdapter
            setHasFixedSize(true)
        }
    }

    override fun observeChange() {
        observe(viewModel.items, ::onDataLoaded)
        observe(viewModel.toastMessage, ::showSnackbarMessage)
    }

    private fun onDataLoaded(items: List<BaseViewItem>) {
        viewAdapter.submitList(items)
    }

    private fun onItemClicked(viewItem: BaseViewItem, view: View) {
        when (viewItem) {
            is DailyItem -> {

            }
            is TextItem -> {
                DailyGraphActivity.startActivity(this)
            }
        }
    }

    companion object {
        @JvmStatic
        fun startActivity(context: Context?) =
            context?.startActivity(Intent(context, CountryIndonesiaActivity::class.java))
    }
}

This question can be through kotlin-mvvm-covid19 project test.

ibrahimyilmaz commented 4 years ago

Hi @tcqq Thanks for the issue that you created. But is it really related to adapter?

Kiel does only provide ListAdapter's builder function.

tcqq commented 4 years ago

Hi @tcqq Thanks for the issue that you created. But is it really related to adapter?

The previously used adapter FlexibleAdapter has not had this problem. But kiel's code is easier to use, I want to fix this issue.

I don't know where the problem is, can you help me see it? Thank you. @ibrahimyilmaz

tcqq commented 4 years ago

This problem cannot be reproduced through kiel's example. The problem is reproduced via kotlin-mvvm-covid19.

kotlin-mvvm-covid19 uses asynchronous network requests to get and display data, while kiel uses fixed data. This problem will occur when using asynchronous network request+kiel currently.

I want to solve this problem because the application will eventually use network requests to display data, setHasFixedSize is also important in list optimization, but the cause of this problem has not been found yet.

ibrahimyilmaz commented 4 years ago

Thanks for investigation.

Lets dig what the problem is:

Kiel provides a simple dsl to create RecyclerViewAdapter which we Android developers have to write. It does not do any magical things in background. The source code is open and you can check =)

setHasFixedSize true https://developer.android.com/reference/kotlin/androidx/recyclerview/widget/RecyclerView#sethasfixedsize When we read the documentation well, there are some important points we should pay attention


RecyclerView can perform several optimizations if it can know in advance 
that RecyclerView's size is not affected by the adapter contents. 
RecyclerView can still change its size based on other factors 
(e.g. its parent's size) but 
this size calculation cannot depend on the size of its children or 
contents of its adapter (except the number of items in the adapter).

If your use of RecyclerView falls into this category, set this to true. It will allow RecyclerView to avoid invalidating the whole layout when its adapter contents change.



when: 
`wrap content` and `setHasFixedSize` true -> 
Before network request:
 There is adapter in RecyclerView but its item size is 0.
After network request:
 There is adapter in RecyclerView but its item size is for example 10.

The problem: 
RecyclerView has calculated its size as 0 in the beginning and does not change the content.

But if we use match_parent or change the layout with ConstraintLayout with constraints. we can see the content.

Also if you create simple RecyclerView Adapter it should behave as it behaves now.

Also there are many magics in FlexibleAdapter. They may somehow touch the RecyclerView and invalidate its size. But it seems setHasFixedSize and wrap content work together like that.

I would recommend to change layout.xml.

If you agree and this is easy for you, can you do that?

tcqq commented 4 years ago

@ibrahimyilmaz Do you mean to change the wrap_content of all layouts to match_parent? If this is the case, it is possible that many scenarios of nested RecyclerView will not be supported.

I currently found this problem when creating a nested RecyclerView layout, hope it can be fixed

ibrahimyilmaz commented 4 years ago

Hi @tcqq

I mean that we need to help RecyclerView to calculate its size.

The easiest solution is wrap_content with weight 1 for the covid app.

If we talk about RecyclerView in RecyclerView case:

Let's say if you implement your NestedRecyclerViews in your project by yourself. How would you do the implementation?

The most important thing related to RecyclerView in RecyclerView, which we should focus/think when we do development, is this.

There is an example RecyclerView in RecyclerView in sample app. You may check this out.

tcqq commented 4 years ago

Let's say if you implement your NestedRecyclerViews in your project by yourself. How would you do the implementation?

List data can be displayed normally, I used this method to create nested RecyclerView before. @ibrahimyilmaz

<?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    android:layout_width="match_parent"
    android:layout_height="match_parent"
    android:orientation="vertical">

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

        <androidx.appcompat.widget.Toolbar
            android:id="@+id/toolbar"
            android:layout_width="match_parent"
            android:layout_height="?attr/actionBarSize" />

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

    <androidx.core.widget.NestedScrollView
        android:layout_width="match_parent"
        android:layout_height="match_parent">

        <LinearLayout
            android:layout_width="match_parent"
            android:layout_height="wrap_content"
            android:orientation="vertical">

            <com.google.android.material.card.MaterialCardView
                android:layout_width="match_parent"
                android:layout_height="match_parent"
                android:layout_marginStart="8dp"
                android:layout_marginTop="8dp"
                android:layout_marginEnd="8dp">

                <com.google.android.material.textfield.TextInputLayout
                    android:id="@+id/text_input_layout_region"
                    style="@style/TextInputLayoutExposedDropdownIcon"
                    android:layout_width="match_parent"
                    android:layout_height="wrap_content"
                    android:layout_marginStart="16dp"
                    android:layout_marginTop="16dp"
                    android:layout_marginEnd="16dp"
                    android:layout_marginBottom="20dp">

                    <com.google.android.material.textfield.TextInputEditText
                        android:id="@+id/edit_text_region"
                        android:layout_width="match_parent"
                        android:layout_height="wrap_content"
                        android:focusableInTouchMode="false"
                        android:hint="@string/country_or_region" />

                </com.google.android.material.textfield.TextInputLayout>

            </com.google.android.material.card.MaterialCardView>

            <com.google.android.material.card.MaterialCardView
                android:layout_width="match_parent"
                android:layout_height="match_parent"
                android:layout_margin="8dp">

                <androidx.recyclerview.widget.RecyclerView
                    android:id="@+id/recycler_view"
                    android:layout_width="match_parent"
                    android:layout_height="wrap_content"
                    tools:itemCount="1"
                    tools:listitem="@layout/item_payout_methods" />

            </com.google.android.material.card.MaterialCardView>

            <androidx.legacy.widget.Space
                android:layout_width="match_parent"
                android:layout_height="@dimen/mtrl_card_spacing" />

        </LinearLayout>

    </androidx.core.widget.NestedScrollView>

</LinearLayout>

        adapter = FlexibleAdapter(getItems(), this, true)
        recycler_view.layoutManager = SmoothScrollLinearLayoutManager(this)
        recycler_view.adapter = adapter
        recycler_view.setHasFixedSize(true)

ibrahimyilmaz commented 4 years ago

Ja good!! What does getItems() do? Before you create an adapter you know the size and passed it right. So that's why wrap_content works very well because knowing items before setting adapter to recyclerview helps the calculation of the size of RecyclerView.

Can you try to setAdapter of RecyclerView when you get data.:

adapter.submitList(items)
recyclerView.adapter = adapter

ibrahimyilmaz commented 4 years ago

Hi @tcqq Thanks for the issue that you created. But is it really related to adapter?

The previously used adapter FlexibleAdapter has not had this problem. But kiel's code is easier to use, I want to fix this issue.

I don't know where the problem is, can you help me see it? Thank you. @ibrahimyilmaz

I checked the history. There was VisitableRecyclerAdapter, I changed it to kiel. So there was no FlexibleAdapter in CovidApp.

All seems okay with this project.

Please write what you wanna actually achieve and elaborate it. Then I may help you. Otherwise none cant help you when you try to explain it in chaotic way.

Sincerely.

tcqq commented 4 years ago

Hi @tcqq Thanks for the issue that you created. But is it really related to adapter?

The previously used adapter FlexibleAdapter has not had this problem. But kiel's code is easier to use, I want to fix this issue. I don't know where the problem is, can you help me see it? Thank you. @ibrahimyilmaz

I checked the history. There was VisitableRecyclerAdapter, I changed it to kiel. So there was no FlexibleAdapter in CovidApp.

All seems okay with this project.

Please write what you wanna actually achieve and elaborate it. Then I may help you. Otherwise none cant help you when you try to explain it in chaotic way.

Sincerely.

@ibrahimyilmaz At present, the height of the RecyclerView of the covid project uses match_parent. Even if the height of the RecyclerView is set to wrap_content, if you use swipeRefresh, the height of the RecyclerView will still fill the layout. I delete swipeRefresh in the RecyclerView and change the height from match_parent to wrap_content to test this problem.

Replace activity_country_indonesia.xml and CountryIndonesiaActivity.kt in the covid project with the following code. Result: The list will not show any data.

Goal: I hope that the height of RecyclerView can be set to wrap_content instead of layout_weight, and the data can be displayed normally.

activity_country_indonesia.xml

<?xml version="1.0" encoding="utf-8"?>
<LinearLayout 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:layout_width="match_parent"
    android:layout_height="match_parent"
    android:background="@color/colorPrimary"
    android:orientation="vertical"
    tools:context=".ui.percountry.indonesia.CountryIndonesiaActivity">

    <androidx.appcompat.widget.Toolbar
        android:id="@+id/toolbar"
        android:layout_width="match_parent"
        android:layout_height="?actionBarSize"
        android:background="@color/colorPrimary" />

    <androidx.recyclerview.widget.RecyclerView
        android:id="@+id/recycler_view"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        app:layoutManager="androidx.recyclerview.widget.LinearLayoutManager" />

</LinearLayout>

If changing the height of RecyclerView from wrap_content to match_parent list will display all data.

CountryIndonesiaActivity.kt

class CountryIndonesiaActivity : BaseActivity() {
    private val viewModel by viewModel<CountryIndonesiaViewModel>()
    private lateinit var binding: ActivityCountryIndonesiaBinding
    private val viewAdapter by lazy { createAdapter(::onItemClicked) }

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        binding = ActivityCountryIndonesiaBinding.inflate(layoutInflater)
        setContentView(binding.root)
        setupActionBarWithBackButton(binding.toolbar)
        initView()

        viewModel.loadData()
    }

    private fun initView() {
        with(binding.recyclerView) {
            adapter = viewAdapter
            setHasFixedSize(true)
        }
/*        binding.swipeRefresh.setOnRefreshListener {
            viewModel.loadData()
        }*/
    }

    override fun observeChange() {
        observe(viewModel.items, ::onDataLoaded)
        observe(viewModel.toastMessage, ::showSnackbarMessage)
//        observe(viewModel.loading, ::loadingSwipeRefresh)
    }

/*    private fun loadingSwipeRefresh(loading: Boolean) {
        with(binding.swipeRefresh) {
            post {
                isRefreshing = loading
            }
        }
    }*/

    private fun onDataLoaded(items: List<BaseViewItem>) {
        viewAdapter.submitList(items)
    }

    private fun onItemClicked(viewItem: BaseViewItem, view: View) {
        when (viewItem) {
            is DailyItem -> {

            }
            is TextItem -> {
                DailyGraphActivity.startActivity(this)
            }
        }
    }

    companion object {
        @JvmStatic
        fun startActivity(context: Context?) =
            context?.startActivity(Intent(context, CountryIndonesiaActivity::class.java))
    }
}

ibrahimyilmaz commented 4 years ago

In your case: This will happen once,

private fun onDataLoaded(items: List<BaseViewItem>) {
        viewAdapter.submitList(items)
}

Can you try it with this?

private fun onDataLoaded(items: List<BaseViewItem>) {
        viewAdapter.submitList(items)
        with(binding.recyclerView) {
            adapter = viewAdapter
            setHasFixedSize(true)
        }
}

and don't forget to remove this in initView

with(binding.recyclerView) {
            adapter = viewAdapter
            setHasFixedSize(true)
}

tcqq commented 4 years ago

@ibrahimyilmaz Hi, thank you for your reply. Now the problem is solved, but do I need to reset the adapter every time before changing the list data? It would be best if it can be set once in initView to make it display normally.

tcqq commented 4 years ago

@ibrahimyilmaz I use this method to set the list data, no problem, but if I want to refresh the data, I try to call this method again and update the data, the list data will not be updated.

    private fun onDataLoaded(items: List<BaseViewItem>) {
        viewAdapter.submitList(items)
        with(recycler_view) {
            adapter = viewAdapter
            setHasFixedSize(true)
        }
    }

ibrahimyilmaz commented 4 years ago

Hi @tcqq I want to help you with your problem but still I cant follow your problem. You need to check how setHasFixedSize and ListAdapter works. Unfortunately I can't see the problem which you try to explain it to me.

It is clear that: when you provide wrap_content and adapter without data. The height will be calculated as 0 in LinearLayout and never changes. When we check we may find many problems in stackoverflow.

The second thing, Kiel is using ListAdapter. All data update logic is handled by Google's ListAdapter.

You are free to fork Kiel and play a bit with Adapter's code. If you find the reason of your problem which I can't unfortunately get, open a PR. I will be thankful for you about this.

This is also how open source works.

Sincerely

tcqq commented 4 years ago

@ibrahimyilmaz The current height of RecyclerView is set to wrap_content. I changed the method of initializing the list data and changing the list data to this method:

    private fun onDataLoaded(items: List<BaseViewItem>) {
        viewAdapter.submitList(items)
//        with(recycler_view) {
//            adapter = viewAdapter
//            setHasFixedSize(true)
//        }
    }

After setting the adapter under onCreate:

    with(recycler_view) {
        adapter = viewAdapter
        //setHasFixedSize(true)
    }

The list can update the data normally. But now can’t use setHasFixedSize(true), if you use it, you will be unable to display the data.

ibrahimyilmaz commented 4 years ago

Please fork kiel and create new Fragment in sample project which reproduces the issue that you face. I will take a look at this and help you.

But also have read Google's documentation and other resources? Kiel does not do any magic in background. It is a wrapper for recyclerview adapter such as ListAdapter, PagedListAdapter etc.

tcqq commented 4 years ago

Thank you for your answer, I will try to solve this problem when I have time.

tcqq commented 4 years ago

@rizmaulana Hi, can you help me see this problem, I haven't found a suitable solution for the time being.

ibrahimyilmaz commented 4 years ago

Please fork kiel and create new Fragment in sample project which reproduces the issue that you face. I will take a look at this and help you.

But also have read Google's documentation and other resources? Kiel does not do any magic in background. It is a wrapper for recyclerview adapter such as ListAdapter, PagedListAdapter etc.

Hi @tcqq As I said, the documentation of Google is very clear about setHasFixedSize. Also you can check this out. https://stackoverflow.com/a/39736376/115890


You can now use android:layout_height="wrap_content" 
on a RecyclerView, which, among other things, allows a 
CollapsingToolbarLayout to know it should not collapse 
when the RecyclerView is empty. This only works when 
you use setHasFixedSize(false) on the RecylcerView.

If you use setHasFixedSize(true) on the RecyclerView, 
this behavior to prevent the CollapsingToolbarLayout 
from collapsing does not work, even though the 
RecyclerView is indeed empty.

If setHasFixedSize was related to the size of items, it 
shouldn't have any effect when the RecyclerView has 
no items.

tcqq commented 4 years ago

@ibrahimyilmaz I understand, this issue is related to RecyclerView, thank you.

ibrahimyilmaz commented 4 years ago

Hi @tcqq But please create an example Fragment, then we can play with it. I can help you what you wanna do. Discovering new details for all of us is always good! If it is not related to Kiel please close the ticket and we can open follow up ticket for this specific example.

Sincerely Ibra

tcqq commented 4 years ago

@ibrahimyilmaz Ok i will create an example.

tcqq commented 4 years ago

This example can be tested by kiel, just modify DiffExampleFragment.kt and cancel the comment code with the red arrow.

ibrahimyilmaz / kiel

[question] Use setHasFixedSize list will not be displayed #71