getAdapterPosition() returning -1 when using GroupieAdapter.update/updateAsync with equality checks
Skyyo opened this issue · 3 comments
Describe the bug
I believe there is something wrong with how the getAdapterPosition()
is getting the position, when hasSameContentAs()
is used in items.
val groupieAdapter = GroupieAdapter()
groupieAdapter.setOnItemClickListener { item, _ ->
val positionClicked = groupieAdapter.getAdapterPosition(item)
Toast.makeText(this@MainActivity, "position $positionClicked", Toast.LENGTH_SHORT)
.show()
}
Initially this returns expected positions. But if we call the .update
or .updateAsync
with the same dataset, and item has diff util related checks(hasSameContentAs to be precise), the getAdapterPosition
will return -1.
class TextItem(private val title: String) : BindableItem<ItemTextBinding>() {
override fun isSameAs(other: Item<*>): Boolean = other is TextItem
override fun hasSameContentAs(other: Item<*>): Boolean =
other is TextItem && other.title == title
override fun getLayout() = R.layout.item_text
override fun initializeViewBinding(view: View) = ItemTextBinding.bind(view)
override fun bind(viewBinding: ItemTextBinding, position: Int) {
viewBinding.tvTitle.text = title
}
}
To Reproduce
Reproducible sample can be found here
Steps:
- tap on any visible recyclerView item, toast with it's position will appear.
- tap on the button, to submit the list to recyclerView and trigger the issue.
- tap on any visible recyclerView item again, you'll find that toast message shows
-1
Expected behavior
I'd expect getAdapterPosition()
to return real item position.
Library version
version 2.9.0
Additional context
- MainActivity.kt. If there is anything that I'm doing wrong - will be glad to know about it.
So I made this change like over a month ago b746769
I think it would fix it, but only at nesting level 1
And it's kinda hacky so I honestly don't know if it is the right solution 🤔 which is why it was never released as a 2.9.1
The "problem" is that rebinding doesn't happen when the item is the same. But if it did, then it'd trigger a notifyItemChanged()
, which causes animation flicker, which isn't what you want.
I actually haven't figured out a good solution for this at all. I am thinking about it every now and then, but... 😖
I was thinking about the approach, that is being used when we want to update a single view from the item using payload keys
, and manually updating icons/texts. That would prevent the flickering but give access to rebinding
Didn't try it, since we can pass the lambdas to the items or even dispatch updates in a different manner.
Giving the fact that workaround is very easy to do (might not be as pretty(subjective)/optimised, but good enough). - I believe this issue isn't that big of a deal, and can be closed/suspended?
Maybe a short info about this behaviour can be added to the readme
, but then it would make sense to state all known issues ( if there is anything except this one ).