detail: do not cache viewholder pos for highlight

Fix an issue with highlighting in the detail UIs taht stemmed from the
ViewHolder position being cached.

Resorting could have changed the ViewHolder position, so caching it
will result in ViewHolders being incorrectly updated.
This commit is contained in:
OxygenCobalt 2022-06-04 13:51:59 -06:00
parent b2ad54eef3
commit 7673fa4a40
No known key found for this signature in database
GPG key ID: 37DBE3621FE9AD47
5 changed files with 26 additions and 25 deletions

View file

@ -45,7 +45,6 @@ import org.oxycblt.auxio.util.textSafe
class AlbumDetailAdapter(listener: Listener) : class AlbumDetailAdapter(listener: Listener) :
DetailAdapter<AlbumDetailAdapter.Listener>(listener, DIFFER) { DetailAdapter<AlbumDetailAdapter.Listener>(listener, DIFFER) {
private var currentSong: Song? = null private var currentSong: Song? = null
private var currentSongPos: Int? = null
override fun getCreatorFromItem(item: Item) = override fun getCreatorFromItem(item: Item) =
super.getCreatorFromItem(item) super.getCreatorFromItem(item)
@ -81,14 +80,13 @@ class AlbumDetailAdapter(listener: Listener) :
} }
} }
override fun shouldHighlightViewHolder(item: Item) = item.id == currentSong?.id override fun shouldHighlightViewHolder(item: Item) = item is Song && item.id == currentSong?.id
/** Update the [song] that this adapter should highlight */ /** Update the [song] that this adapter should highlight */
fun highlightSong(song: Song?) { fun highlightSong(song: Song?) {
if (song == currentSong) return if (song == currentSong) return
highlightImpl(currentSong, song)
currentSong = song currentSong = song
currentSongPos?.let { pos -> notifyItemChanged(pos, PAYLOAD_HIGHLIGHT_CHANGED) }
currentSongPos = highlightItem(song)
} }
companion object { companion object {

View file

@ -45,10 +45,7 @@ import org.oxycblt.auxio.util.textSafe
class ArtistDetailAdapter(listener: Listener) : class ArtistDetailAdapter(listener: Listener) :
DetailAdapter<DetailAdapter.Listener>(listener, DIFFER) { DetailAdapter<DetailAdapter.Listener>(listener, DIFFER) {
private var currentAlbum: Album? = null private var currentAlbum: Album? = null
private var currentAlbumPos: Int? = null
private var currentSong: Song? = null private var currentSong: Song? = null
private var currentHighlightedSongPos: Int? = null
override fun getCreatorFromItem(item: Item) = override fun getCreatorFromItem(item: Item) =
super.getCreatorFromItem(item) super.getCreatorFromItem(item)
@ -92,17 +89,15 @@ class ArtistDetailAdapter(listener: Listener) :
/** Update the current [album] that this adapter should highlight */ /** Update the current [album] that this adapter should highlight */
fun highlightAlbum(album: Album?) { fun highlightAlbum(album: Album?) {
if (album == currentAlbum) return if (album == currentAlbum) return
highlightImpl(currentAlbum, album)
currentAlbum = album currentAlbum = album
currentAlbumPos?.let { pos -> notifyItemChanged(pos, PAYLOAD_HIGHLIGHT_CHANGED) }
currentAlbumPos = highlightItem(album)
} }
/** Update the [song] that this adapter should highlight */ /** Update the [song] that this adapter should highlight */
fun highlightSong(song: Song?) { fun highlightSong(song: Song?) {
if (song == currentSong) return if (song == currentSong) return
highlightImpl(currentSong, song)
currentSong = song currentSong = song
currentHighlightedSongPos?.let { notifyItemChanged(it, PAYLOAD_HIGHLIGHT_CHANGED) }
currentHighlightedSongPos = highlightItem(song)
} }
companion object { companion object {

View file

@ -35,6 +35,7 @@ import org.oxycblt.auxio.ui.NewHeaderViewHolder
import org.oxycblt.auxio.ui.SimpleItemCallback import org.oxycblt.auxio.ui.SimpleItemCallback
import org.oxycblt.auxio.util.context import org.oxycblt.auxio.util.context
import org.oxycblt.auxio.util.inflater import org.oxycblt.auxio.util.inflater
import org.oxycblt.auxio.util.logW
import org.oxycblt.auxio.util.textSafe import org.oxycblt.auxio.util.textSafe
abstract class DetailAdapter<L : DetailAdapter.Listener>( abstract class DetailAdapter<L : DetailAdapter.Listener>(
@ -43,16 +44,26 @@ abstract class DetailAdapter<L : DetailAdapter.Listener>(
) : MultiAdapter<L>(listener) { ) : MultiAdapter<L>(listener) {
abstract fun shouldHighlightViewHolder(item: Item): Boolean abstract fun shouldHighlightViewHolder(item: Item): Boolean
protected inline fun <reified T : Item> highlightItem(newItem: T?): Int? { protected inline fun <reified T : Item> highlightImpl(oldItem: T?, newItem: T?) {
if (newItem == null) { if (oldItem != null) {
return null val pos = data.currentList.indexOfFirst { item -> item.id == oldItem.id && item is T }
if (pos > -1) {
notifyItemChanged(pos, PAYLOAD_HIGHLIGHT_CHANGED)
} else {
logW("oldItem was not in adapter data")
}
} }
// Use existing data instead of having to re-sort it. if (newItem != null) {
// We also have to account for the album count when searching for the ViewHolder. val pos = data.currentList.indexOfFirst { item -> item is T && item.id == newItem.id }
val pos = data.currentList.indexOfFirst { item -> item.id == newItem.id && item is T }
notifyItemChanged(pos, PAYLOAD_HIGHLIGHT_CHANGED) if (pos > -1) {
return pos notifyItemChanged(pos, PAYLOAD_HIGHLIGHT_CHANGED)
} else {
logW("newItem was not in adapter data")
}
}
} }
@Suppress("LeakingThis") override val data = AsyncBackingData(this, diffCallback) @Suppress("LeakingThis") override val data = AsyncBackingData(this, diffCallback)
@ -90,7 +101,7 @@ abstract class DetailAdapter<L : DetailAdapter.Listener>(
companion object { companion object {
// This payload value serves two purposes: // This payload value serves two purposes:
// 1. It disables animations from notifyItemChanged, which looks bad when highlighting // 1. It disables animations from notifyItemChanged, which looks bad when highlighting
// values. // ViewHolders.
// 2. It instructs adapters to avoid re-binding information, and instead simply // 2. It instructs adapters to avoid re-binding information, and instead simply
// change the highlight state. // change the highlight state.
val PAYLOAD_HIGHLIGHT_CHANGED = Any() val PAYLOAD_HIGHLIGHT_CHANGED = Any()

View file

@ -43,7 +43,6 @@ import org.oxycblt.auxio.util.textSafe
class GenreDetailAdapter(listener: Listener) : class GenreDetailAdapter(listener: Listener) :
DetailAdapter<DetailAdapter.Listener>(listener, DIFFER) { DetailAdapter<DetailAdapter.Listener>(listener, DIFFER) {
private var currentSong: Song? = null private var currentSong: Song? = null
private var currentSongPos: Int? = null
override fun getCreatorFromItem(item: Item) = override fun getCreatorFromItem(item: Item) =
super.getCreatorFromItem(item) super.getCreatorFromItem(item)
@ -77,14 +76,13 @@ class GenreDetailAdapter(listener: Listener) :
} }
} }
override fun shouldHighlightViewHolder(item: Item) = item.id == currentSong?.id override fun shouldHighlightViewHolder(item: Item) = item is Song && item.id == currentSong?.id
/** Update the [song] that this adapter should highlight */ /** Update the [song] that this adapter should highlight */
fun highlightSong(song: Song?) { fun highlightSong(song: Song?) {
if (song == currentSong) return if (song == currentSong) return
highlightImpl(currentSong, song)
currentSong = song currentSong = song
currentSongPos?.let { pos -> notifyItemChanged(pos, PAYLOAD_HIGHLIGHT_CHANGED) }
currentSongPos = highlightItem(song)
} }
companion object { companion object {

View file

@ -25,7 +25,6 @@ import coil.request.Disposable
import coil.request.ImageRequest import coil.request.ImageRequest
import coil.size.Size import coil.size.Size
import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.music.Song
import org.oxycblt.auxio.util.logD
/** /**
* A utility to provide bitmaps in a manner less prone to race conditions. * A utility to provide bitmaps in a manner less prone to race conditions.