Incorrect sticky header position calculation when changing data

[chenf7]

Hi, I'm using StickyHeaderLinearLayoutManager for my EpoxyRecyclerView and in my controller I override the isStickyHeader() function:

        override fun isStickyHeader(position: Int): Boolean {
            return adapter.getModelAtPosition(position) is MyItemHeader
        }

I noticed that when switching between 2 data sets (2 different filters on same data) the second sticky header is off by 1 and a normal list item is set as the sticky header. Set 1 has 4 rows and set 2 has 174 rows (including item rows header rows and the first row which is a title row that shows how many items are in the list). Set1: [TitleItem(id: t1, value:2), MyItemHeader(id: h1, value: h1), Item(id: i1, value: val1), Item(id: i3, value: val3)] Set2: [TitleItem(id: t1, value:170), MyItemHeader(id: h1, value: h1), Item(id: i1, value: val1), Item(id: i2, value: val2), Item(id: i3, value: val3), Item(id: i4, value: val4), MyItemHeader(id: h2, value: h2), Item(id: i5, value: val5), ...]

So when switching from Set1 to Set2, id2 is added between id1 and id3, and other rows are added at the end.

When debugging it I see that upon change HeaderPositionsAdapterDataObserver.onItemRangeInserted() is called twice with those parameters: positionStart: 4, itemCount: 169 positionStart: 3 , itemCount: 1 So after the first call position 6 gets set as a header (headerPositions.add(6)) correctly, but then the second call moves it by one position because that call is interpreted as 1 item is added at position 3.

I think that the problem is that when onItemRangeInserted() is called, the data set in the adapter is already updated with the new data (in AsyncEpoxyDiffer.readOnlyList), as it has 174 items. So on that insert the headers are set with the positions correct for the end of the process (no change on that is needed). Now the second onItemRangeInserted() call for the insert of item i2 causes a shift in header ids. It assumes that the differ list was changed again, but it wasn't, so no shift of positions should happen

[elihart]

Thanks for the detailed report. It is very possible that a bug around this exists. However, the code was contributed by @AkshayChordiya and I'm not in a good position to try to fix this. If anyone can contribute a test to reproduce this and the corresponding fix we can maybe get it merged

[chenf7]

@elihart I do not have permission to contribute to this project. Following is a modified StickyHeaderController (test class) that will reproduce the bug. It's a test class from the sample project and I just modified a couple of lines.

The list will first load with only 1 header and a few items. Clicking on one of the items will add all the rest of the headers/items + another item between the first items. Then, scrolling after the second header will expose the bug, as the second header won't be sticky, but the item after it will wrongly become sticky.

package com.airbnb.epoxy.kotlinsample

import android.content.Context
import android.widget.Toast
import com.airbnb.epoxy.EpoxyController
import com.airbnb.epoxy.kotlinsample.models.StickyItemEpoxyHolder
import com.airbnb.epoxy.kotlinsample.models.itemEpoxyHolder
import com.airbnb.epoxy.kotlinsample.models.stickyItemEpoxyHolder
import com.airbnb.epoxy.stickyheader.StickyHeaderCallbacks

/**
 * Showcases [EpoxyController] with sticky header support
 */
class StickyHeaderController(
    private val context: Context
) : EpoxyController(), StickyHeaderCallbacks {

    var addItem = false

    override fun buildModels() {
        for (i in 0 until (if(addItem) 100 else 4)) {
            when {
                i % 5 == 0 -> stickyItemEpoxyHolder {
                    id("sticky-header $i")
                    title("Sticky header $i")
                    listener {
                        Toast.makeText(context, "clicked", Toast.LENGTH_LONG).show()
                    }
                }
                else -> {
                    if (i == 2 && addItem) {
                        itemEpoxyHolder {
                            id("view holder ADDED $i")
                            title("Added item. Click to remove")
                            listener {
                                Toast.makeText(context, "clicked", Toast.LENGTH_LONG)
                                    .show()
                                addItem = false
                                requestModelBuild()
                            }
                        }
                    }
                    itemEpoxyHolder {
                        id("view holder $i")
                        title("this is a View Holder item")
                        listener {
                            Toast.makeText(context, "clicked", Toast.LENGTH_LONG)
                                .show()
                            addItem = true
                            requestModelBuild()
                        }
                    }
                }
            }
        }
    }

    // Feel feel to use any logic here to determine if the [position] is sticky view or not
    override fun isStickyHeader(position: Int) = adapter.getModelAtPosition(position) is StickyItemEpoxyHolder
}

[elihart]

@chenf7 If you want to contribute a PR you can fork the project and open a PR against this repo.

[stephen-mojo]

@chenf7 Did you find a solution to this issue?

I have a UI with a toggle switch above a RecyclerView. When I press the toggle switch, different data is loaded into the view. If I change the data a couple times, my second sticky header is off by one.

[chenf7]

@stephen-mojo I didn't solve the issue itself. For now I've settled on using a new epoxy controller each time the data changes (changes that keep items order don't require a new controller, e.g. selection state).

[stephen-mojo]

@chenf7 Thank you for the idea. Works great. Something feels dirty about creating a new EpoxyController each time, but we gotta do what we gotta do.