search

JakeWharton / u2020

A sample Android app which showcases advanced usage of Dagger among other open source libraries.
https://www.youtube.com/watch?v=0XHx9jtxIxU
Apache License 2.0
5.68k stars 930 forks source link

Fix possibly outdated adapter position #205

Closed NightlyNexus closed 9 years ago

NightlyNexus commented 9 years ago

https://developer.android.com/reference/android/support/v7/widget/RecyclerView.Adapter.html#onBindViewHolder(VH,%20int)

The setHasStableIds(true); is unrelated but nice to have.

JakeWharton commented 9 years ago

Good find.

NightlyNexus commented 9 years ago

I'm pretty sure I was wrong here. The position, of course, shouldn't be cached, but the data is always bound to that ViewHolder. Sorry about this. :p Probably just revert via the GitHub button.

JakeWharton commented 9 years ago

I don't think it really matters either way, right? I prefer this as the adapter remains the source of truth for the data instead of caching potentially stale data.

NightlyNexus commented 9 years ago

Right, it doesn't matter either way; but, the data won't be stale, just the position of the data might have been changed.

JakeWharton commented 9 years ago

It's on the main thread though, and you can only update an adapter on the main thread.

NightlyNexus commented 9 years ago

I meant the position of the data might have been changed, and that's why the position should not be cached into a callback like a click listener.

hmm, I'll try to clarify my original thought:

This PR was unnecessary because the data bound to the ViewHolder stays with the ViewHolder. It is my understanding that, if we do something like notifyItemMoved(fromPosition, toPosition) to reflect a change in the corresponding data's position, the ViewHolder's corresponding position may be changed without a full rebind. The data is still correct, of course.