From 7673fa4a409ab70e5f1b9b1dfceb3309b38cd3b2 Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Sat, 4 Jun 2022 13:51:59 -0600 Subject: [PATCH] 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. --- .../detail/recycler/AlbumDetailAdapter.kt | 6 ++-- .../detail/recycler/ArtistDetailAdapter.kt | 9 ++---- .../auxio/detail/recycler/DetailAdapter.kt | 29 +++++++++++++------ .../detail/recycler/GenreDetailAdapter.kt | 6 ++-- .../org/oxycblt/auxio/image/BitmapProvider.kt | 1 - 5 files changed, 26 insertions(+), 25 deletions(-) diff --git a/app/src/main/java/org/oxycblt/auxio/detail/recycler/AlbumDetailAdapter.kt b/app/src/main/java/org/oxycblt/auxio/detail/recycler/AlbumDetailAdapter.kt index 6e3a2973b..ed4c2318c 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/recycler/AlbumDetailAdapter.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/recycler/AlbumDetailAdapter.kt @@ -45,7 +45,6 @@ import org.oxycblt.auxio.util.textSafe class AlbumDetailAdapter(listener: Listener) : DetailAdapter(listener, DIFFER) { private var currentSong: Song? = null - private var currentSongPos: Int? = null override fun getCreatorFromItem(item: 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 */ fun highlightSong(song: Song?) { if (song == currentSong) return + highlightImpl(currentSong, song) currentSong = song - currentSongPos?.let { pos -> notifyItemChanged(pos, PAYLOAD_HIGHLIGHT_CHANGED) } - currentSongPos = highlightItem(song) } companion object { diff --git a/app/src/main/java/org/oxycblt/auxio/detail/recycler/ArtistDetailAdapter.kt b/app/src/main/java/org/oxycblt/auxio/detail/recycler/ArtistDetailAdapter.kt index d7204ae14..7adb4b512 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/recycler/ArtistDetailAdapter.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/recycler/ArtistDetailAdapter.kt @@ -45,10 +45,7 @@ import org.oxycblt.auxio.util.textSafe class ArtistDetailAdapter(listener: Listener) : DetailAdapter(listener, DIFFER) { private var currentAlbum: Album? = null - private var currentAlbumPos: Int? = null - private var currentSong: Song? = null - private var currentHighlightedSongPos: Int? = null override fun getCreatorFromItem(item: Item) = super.getCreatorFromItem(item) @@ -92,17 +89,15 @@ class ArtistDetailAdapter(listener: Listener) : /** Update the current [album] that this adapter should highlight */ fun highlightAlbum(album: Album?) { if (album == currentAlbum) return + highlightImpl(currentAlbum, album) currentAlbum = album - currentAlbumPos?.let { pos -> notifyItemChanged(pos, PAYLOAD_HIGHLIGHT_CHANGED) } - currentAlbumPos = highlightItem(album) } /** Update the [song] that this adapter should highlight */ fun highlightSong(song: Song?) { if (song == currentSong) return + highlightImpl(currentSong, song) currentSong = song - currentHighlightedSongPos?.let { notifyItemChanged(it, PAYLOAD_HIGHLIGHT_CHANGED) } - currentHighlightedSongPos = highlightItem(song) } companion object { diff --git a/app/src/main/java/org/oxycblt/auxio/detail/recycler/DetailAdapter.kt b/app/src/main/java/org/oxycblt/auxio/detail/recycler/DetailAdapter.kt index 26207d293..c3e1314ab 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/recycler/DetailAdapter.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/recycler/DetailAdapter.kt @@ -35,6 +35,7 @@ import org.oxycblt.auxio.ui.NewHeaderViewHolder import org.oxycblt.auxio.ui.SimpleItemCallback import org.oxycblt.auxio.util.context import org.oxycblt.auxio.util.inflater +import org.oxycblt.auxio.util.logW import org.oxycblt.auxio.util.textSafe abstract class DetailAdapter( @@ -43,16 +44,26 @@ abstract class DetailAdapter( ) : MultiAdapter(listener) { abstract fun shouldHighlightViewHolder(item: Item): Boolean - protected inline fun highlightItem(newItem: T?): Int? { - if (newItem == null) { - return null + protected inline fun highlightImpl(oldItem: T?, newItem: T?) { + if (oldItem != 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. - // We also have to account for the album count when searching for the ViewHolder. - val pos = data.currentList.indexOfFirst { item -> item.id == newItem.id && item is T } - notifyItemChanged(pos, PAYLOAD_HIGHLIGHT_CHANGED) - return pos + if (newItem != null) { + val pos = data.currentList.indexOfFirst { item -> item is T && item.id == newItem.id } + + if (pos > -1) { + notifyItemChanged(pos, PAYLOAD_HIGHLIGHT_CHANGED) + } else { + logW("newItem was not in adapter data") + } + } } @Suppress("LeakingThis") override val data = AsyncBackingData(this, diffCallback) @@ -90,7 +101,7 @@ abstract class DetailAdapter( companion object { // This payload value serves two purposes: // 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 // change the highlight state. val PAYLOAD_HIGHLIGHT_CHANGED = Any() diff --git a/app/src/main/java/org/oxycblt/auxio/detail/recycler/GenreDetailAdapter.kt b/app/src/main/java/org/oxycblt/auxio/detail/recycler/GenreDetailAdapter.kt index bca83ef49..90729f36c 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/recycler/GenreDetailAdapter.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/recycler/GenreDetailAdapter.kt @@ -43,7 +43,6 @@ import org.oxycblt.auxio.util.textSafe class GenreDetailAdapter(listener: Listener) : DetailAdapter(listener, DIFFER) { private var currentSong: Song? = null - private var currentSongPos: Int? = null override fun getCreatorFromItem(item: 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 */ fun highlightSong(song: Song?) { if (song == currentSong) return + highlightImpl(currentSong, song) currentSong = song - currentSongPos?.let { pos -> notifyItemChanged(pos, PAYLOAD_HIGHLIGHT_CHANGED) } - currentSongPos = highlightItem(song) } companion object { diff --git a/app/src/main/java/org/oxycblt/auxio/image/BitmapProvider.kt b/app/src/main/java/org/oxycblt/auxio/image/BitmapProvider.kt index 66ad95c5b..ebc0c18e9 100644 --- a/app/src/main/java/org/oxycblt/auxio/image/BitmapProvider.kt +++ b/app/src/main/java/org/oxycblt/auxio/image/BitmapProvider.kt @@ -25,7 +25,6 @@ import coil.request.Disposable import coil.request.ImageRequest import coil.size.Size 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.