rubensousa / DpadRecyclerView

A RecyclerView built for Android TV with Compose in mind and as a replacement for Leanback's BaseGridView.
https://rubensousa.github.io/DpadRecyclerView/
Apache License 2.0
136 stars 17 forks source link

Grid being flipped by screen scrolling orientation with DpadSpanSizeLookup. #156

Closed NewtonCesarRoncari closed 1 year ago

NewtonCesarRoncari commented 1 year ago

Hi, I'm trying to change the DpadSpanSizeLookup by ViewType, submitting a list with 2 different items, one representing a rail and the other representing a grid item, but the screen rendering becomes incorrect. Performing a top-to-bottom scroll, the grid appears correctly, but when scrolling from bottom to top, the grid becomes inverted

https://github.com/rubensousa/DpadRecyclerView/assets/31870695/a22dbcaa-1e52-43d1-9779-1b5da116e631

rubensousa commented 1 year ago

Hi @NewtonCesarRoncari thanks for the bug report. I will have a look at this next week since I won't have time. For now, as a temporary workaround, you should be able to fix this by ensuring an even number of items in the grid (a number divisible by the span count)

NewtonCesarRoncari commented 1 year ago

Great! @rubensousa Thanks for the reply, if you need more info or details, feel free to ask.

rubensousa commented 1 year ago

@NewtonCesarRoncari fix should be here: https://github.com/rubensousa/DpadRecyclerView/pull/158

Can you please pull that to your fork and play around with it a bit? I will release a new version after your feedback

NewtonCesarRoncari commented 1 year ago

Hi @rubensousa, Great, I'll do some tests and give you a feedback.

NewtonCesarRoncari commented 1 year ago

Hi, @rubensousa The getItemViewType method of the adapter is trying to run for positions that do not have rails. In the case of clicking up on the first rail or down on the last rail, it causes an ArrayIndexOutOfBoundsException.

In my example, I am using ListAdapter instead of a MutableListAdapter.

    override fun getItemViewType(position: Int): Int {
        return currentList[position].type.ordinal    // Trhow ArrayIndexOutOfBoundsException
    }
com...sousa.dpadrecyclerview.sample  E  java.lang.ArrayIndexOutOfBoundsException: length=26; index=-1
at java.util.ArrayList.get(ArrayList.java:439)
at java.util.Collections$UnmodifiableList.get(Collections.java:1356)
at com.rubensousa.dpadrecyclerview.sample.ui.screen.list.HorizontalListAdapter.getItemViewType(HorizontalListAdapter.kt:66)
at androidx.recyclerview.widget.NestedAdapterWrapper.getItemViewType(NestedAdapterWrapper.java:146)
at androidx.recyclerview.widget.ConcatAdapterController.getItemViewType(ConcatAdapterController.java:319)
at androidx.recyclerview.widget.ConcatAdapter.getItemViewType(ConcatAdapter.java:178)
at com.rubensousa.dpadrecyclerview.sample.ui.screen.list.ListFragment$setupRecyclerView$1$3.getSpanSize(ListFragment.kt:139)
at com.rubensousa.dpadrecyclerview.layoutmanager.focus.SpanFocusFinder.getNextSpanEnd(SpanFocusFinder.kt:201)
at com.rubensousa.dpadrecyclerview.layoutmanager.focus.SpanFocusFinder.fitsNextInCurrentSpanGroup(SpanFocusFinder.kt:189)
at com.rubensousa.dpadrecyclerview.layoutmanager.focus.SpanFocusFinder.moveToStartOfNextSpanGroup(SpanFocusFinder.kt:164)
at com.rubensousa.dpadrecyclerview.layoutmanager.focus.SpanFocusFinder.findNextSpanPosition(SpanFocusFinder.kt:77)

as a temporary workaround i did this:

     override fun getItemViewType(position: Int): Int {
        val mPosition = position.coerceIn(0, currentList.lastIndex)
        return currentList[mPosition].type.ordinal
    }
rubensousa commented 1 year ago

Thank you for the report once again. This should be easy to address. So everything else was fine, correct?

NewtonCesarRoncari commented 1 year ago

Yes, the rest is working perfectly 👍

rubensousa commented 1 year ago

@NewtonCesarRoncari fix is now available in 1.1.0-alpha03