all: use notifyItemChanged everywhere

Apply the notifyItemChanged fix everywhere by making it an explicit part of the RecyclerView framework. This way, implementing future selection and rewrite behavior will be much easier, as the payload argument is available in every adapter implementation.
2022-06-02 10:42:33 -06:00 · 2022-06-02 10:42:33 -06:00 · e73d10070b
commit e73d10070b
parent 10afae0bfc
11 changed files with 123 additions and 69 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -9,9 +9,11 @@
 - Fixed crash when seeking to the end of a track as the track changed to a track with a lower duration 
 - Fixed regression where GadgetBridge media controls would no longer work
 - Fixed issue where the album/artist/genre would not be correctly restored
+- Fixed issue where items would not highlight properly in the detail UI

 #### Dev/Meta
 - Switched from `LiveData` to `StateFlow`
+- Use `notifyItemChanged` instead of directly mutating `ViewHolder` instances.

 ## v2.3.0

--- 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,7 @@ import org.oxycblt.auxio.util.textSafe
 class AlbumDetailAdapter(listener: Listener) :
    DetailAdapter<AlbumDetailAdapter.Listener>(listener, DIFFER) {
    private var currentSong: Song? = null
-    private var currentHighlightedSongPos: Int? = null
+    private var currentSongPos: Int? = null

    override fun getCreatorFromItem(item: Item) =
        super.getCreatorFromItem(item)
@ -65,13 +65,19 @@ class AlbumDetailAdapter(listener: Listener) :
                else -> null
            }

-    override fun onBind(viewHolder: RecyclerView.ViewHolder, item: Item, listener: Listener) {
-        super.onBind(viewHolder, item, listener)
-
-        when (item) {
-            is Album -> (viewHolder as AlbumDetailViewHolder).bind(item, listener)
-            is DiscHeader -> (viewHolder as DiscHeaderViewHolder).bind(item, Unit)
-            is Song -> (viewHolder as AlbumSongViewHolder).bind(item, listener)
+    override fun onBind(
+        viewHolder: RecyclerView.ViewHolder,
+        item: Item,
+        listener: Listener,
+        payload: List<Any>
+    ) {
+        super.onBind(viewHolder, item, listener, payload)
+        if (payload.isEmpty()) {
+            when (item) {
+                is Album -> (viewHolder as AlbumDetailViewHolder).bind(item, listener)
+                is DiscHeader -> (viewHolder as DiscHeaderViewHolder).bind(item, Unit)
+                is Song -> (viewHolder as AlbumSongViewHolder).bind(item, listener)
+            }
        }
    }

@ -83,8 +89,8 @@ class AlbumDetailAdapter(listener: Listener) :
    fun highlightSong(song: Song?) {
        if (song == currentSong) return
        currentSong = song
-        currentHighlightedSongPos?.let { notifyItemChanged(it, PAYLOAD_HIGHLIGHT_CHANGED) }
-        currentHighlightedSongPos = highlightItem(song)
+        currentSongPos?.let { pos -> notifyItemChanged(pos, PAYLOAD_HIGHLIGHT_CHANGED) }
+        currentSongPos = highlightItem(song)
    }

    companion object {
--- 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,7 +45,7 @@ import org.oxycblt.auxio.util.textSafe
 class ArtistDetailAdapter(listener: Listener) :
    DetailAdapter<DetailAdapter.Listener>(listener, DIFFER) {
    private var currentAlbum: Album? = null
-    private var currentHighlightedAlbumPos: Int? = null
+    private var currentAlbumPos: Int? = null

    private var currentSong: Song? = null
    private var currentHighlightedSongPos: Int? = null
@ -68,29 +68,35 @@ class ArtistDetailAdapter(listener: Listener) :
                else -> null
            }

-    override fun onBind(viewHolder: RecyclerView.ViewHolder, item: Item, listener: Listener) {
-        super.onBind(viewHolder, item, listener)
-        when (item) {
-            is Artist -> (viewHolder as ArtistDetailViewHolder).bind(item, listener)
-            is Album -> (viewHolder as ArtistAlbumViewHolder).bind(item, listener)
-            is Song -> (viewHolder as ArtistSongViewHolder).bind(item, listener)
-            else -> {}
+    override fun onBind(
+        viewHolder: RecyclerView.ViewHolder,
+        item: Item,
+        listener: Listener,
+        payload: List<Any>
+    ) {
+        super.onBind(viewHolder, item, listener, payload)
+        if (payload.isEmpty()) {
+            when (item) {
+                is Artist -> (viewHolder as ArtistDetailViewHolder).bind(item, listener)
+                is Album -> (viewHolder as ArtistAlbumViewHolder).bind(item, listener)
+                is Song -> (viewHolder as ArtistSongViewHolder).bind(item, listener)
+                else -> {}
+            }
        }
    }

    override fun onHighlightViewHolder(viewHolder: Highlightable, item: Item) {
        viewHolder.setHighlighted(
            (item is Album && item.id == currentAlbum?.id) ||
-                (item is Song && item.id == currentSong?.id)
-        )
+                (item is Song && item.id == currentSong?.id))
    }

    /** Update the current [album] that this adapter should highlight */
    fun highlightAlbum(album: Album?) {
        if (album == currentAlbum) return
        currentAlbum = album
-        currentHighlightedAlbumPos?.let { notifyItemChanged(it, PAYLOAD_HIGHLIGHT_CHANGED) }
-        currentHighlightedAlbumPos = highlightItem(album)
+        currentAlbumPos?.let { pos -> notifyItemChanged(pos, PAYLOAD_HIGHLIGHT_CHANGED) }
+        currentAlbumPos = highlightItem(album)
    }

    /** Update the [song] that this adapter should highlight */
--- a/app/src/main/java/org/oxycblt/auxio/detail/recycler/DetailAdapter.kt
+++ b/app/src/main/java/org/oxycblt/auxio/detail/recycler/DetailAdapter.kt
@ -71,10 +71,17 @@ abstract class DetailAdapter<L : DetailAdapter.Listener>(
            else -> null
        }

-    override fun onBind(viewHolder: RecyclerView.ViewHolder, item: Item, listener: L) {
-        when (item) {
-            is Header -> (viewHolder as NewHeaderViewHolder).bind(item, Unit)
-            is SortHeader -> (viewHolder as SortHeaderViewHolder).bind(item, listener)
+    override fun onBind(
+        viewHolder: RecyclerView.ViewHolder,
+        item: Item,
+        listener: L,
+        payload: List<Any>
+    ) {
+        if (payload.isEmpty()) {
+            when (item) {
+                is Header -> (viewHolder as NewHeaderViewHolder).bind(item, Unit)
+                is SortHeader -> (viewHolder as SortHeaderViewHolder).bind(item, listener)
+            }
        }

        if (viewHolder is Highlightable) {
--- 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,7 @@ import org.oxycblt.auxio.util.textSafe
 class GenreDetailAdapter(listener: Listener) :
    DetailAdapter<DetailAdapter.Listener>(listener, DIFFER) {
    private var currentSong: Song? = null
-    private var currentHighlightedSongPos: Int? = null
+    private var currentSongPos: Int? = null

    override fun getCreatorFromItem(item: Item) =
        super.getCreatorFromItem(item)
@ -61,12 +61,19 @@ class GenreDetailAdapter(listener: Listener) :
                else -> null
            }

-    override fun onBind(viewHolder: RecyclerView.ViewHolder, item: Item, listener: Listener) {
-        super.onBind(viewHolder, item, listener)
-        when (item) {
-            is Genre -> (viewHolder as GenreDetailViewHolder).bind(item, listener)
-            is Song -> (viewHolder as GenreSongViewHolder).bind(item, listener)
-            else -> {}
+    override fun onBind(
+        viewHolder: RecyclerView.ViewHolder,
+        item: Item,
+        listener: Listener,
+        payload: List<Any>
+    ) {
+        super.onBind(viewHolder, item, listener, payload)
+        if (payload.isEmpty()) {
+            when (item) {
+                is Genre -> (viewHolder as GenreDetailViewHolder).bind(item, listener)
+                is Song -> (viewHolder as GenreSongViewHolder).bind(item, listener)
+                else -> {}
+            }
        }
    }

@ -78,8 +85,8 @@ class GenreDetailAdapter(listener: Listener) :
    fun highlightSong(song: Song?) {
        if (song == currentSong) return
        currentSong = song
-        currentHighlightedSongPos?.let { notifyItemChanged(it, PAYLOAD_HIGHLIGHT_CHANGED) }
-        currentHighlightedSongPos = highlightItem(song)
+        currentSongPos?.let { pos -> notifyItemChanged(pos, PAYLOAD_HIGHLIGHT_CHANGED) }
+        currentSongPos = highlightItem(song)
    }

    companion object {
--- a/app/src/main/java/org/oxycblt/auxio/home/tabs/TabAdapter.kt
+++ b/app/src/main/java/org/oxycblt/auxio/home/tabs/TabAdapter.kt
@ -53,6 +53,7 @@ class TabAdapter(listener: Listener) :

        fun setTab(at: Int, tab: Tab) {
            tabs[at] = tab
+            adapter.notifyItemChanged(at, PAYLOAD_TAB_CHANGED)
        }

        fun moveItems(from: Int, to: Int) {
@ -63,18 +64,17 @@ class TabAdapter(listener: Listener) :
            adapter.notifyItemMoved(from, to)
        }
    }
+
+    companion object {
+        val PAYLOAD_TAB_CHANGED = Any()
+    }
 }

 class TabViewHolder private constructor(private val binding: ItemTabBinding) :
    BindingViewHolder<Tab, TabAdapter.Listener>(binding.root) {
    @SuppressLint("ClickableViewAccessibility")
    override fun bind(item: Tab, listener: TabAdapter.Listener) {
-        binding.root.apply {
-            setOnClickListener {
-                binding.tabIcon.isChecked = !binding.tabIcon.isChecked
-                listener.onVisibilityToggled(item.mode)
-            }
-        }
+        binding.root.apply { setOnClickListener { listener.onVisibilityToggled(item.mode) } }

        binding.tabIcon.apply {
            setText(item.mode.string)
--- a/app/src/main/java/org/oxycblt/auxio/search/SearchAdapter.kt
+++ b/app/src/main/java/org/oxycblt/auxio/search/SearchAdapter.kt
@ -60,7 +60,8 @@ class SearchAdapter(listener: MenuItemListener) : MultiAdapter<MenuItemListener>
    override fun onBind(
        viewHolder: RecyclerView.ViewHolder,
        item: Item,
-        listener: MenuItemListener
+        listener: MenuItemListener,
+        payload: List<Any>
    ) {
        when (item) {
            is Song -> (viewHolder as SongViewHolder).bind(item, listener)
--- a/app/src/main/java/org/oxycblt/auxio/ui/RecyclerFramework.kt
+++ b/app/src/main/java/org/oxycblt/auxio/ui/RecyclerFramework.kt
@ -37,13 +37,23 @@ abstract class MonoAdapter<T, L, VH : BindingViewHolder<T, L>>(private val liste
    /** The creator instance that all viewholders will be derived from. */
    protected abstract val creator: BindingViewHolder.Creator<VH>

+    /**
+     * An optional override to further modify the given [viewHolder]. The normal operation is to
+     * bind the viewholder, with nothing more.
+     */
+    open fun onBind(viewHolder: VH, item: T, listener: L, payload: List<Any>) {
+        viewHolder.bind(item, listener)
+    }
+
    override fun getItemCount(): Int = data.getItemCount()

    override fun onCreateViewHolder(parent: ViewGroup, viewType: Int) =
        creator.create(parent.context)

-    override fun onBindViewHolder(viewHolder: VH, position: Int) {
-        viewHolder.bind(data.getItem(position), listener)
+    override fun onBindViewHolder(holder: VH, position: Int) = throw UnsupportedOperationException()
+
+    override fun onBindViewHolder(viewHolder: VH, position: Int, payload: List<Any>) {
+        onBind(viewHolder, data.getItem(position), listener, payload)
    }
 }

@ -53,7 +63,7 @@ private typealias AnyCreator = BindingViewHolder.Creator<out RecyclerView.ViewHo
 * An adapter for many viewholders tied to many types of data. Deriving this is more complicated
 * than [MonoAdapter], as less overrides can be provided "for free".
 * @author OxygenCobalt
- * 
+ *
 * TODO: Force impls to handle payload situations.
 */
 abstract class MultiAdapter<L>(private val listener: L) :
@ -78,7 +88,12 @@ abstract class MultiAdapter<L>(private val listener: L) :
     * Bind the given viewholder to an item. Casting must be done on the consumer's end due to
     * bounds on [BindingViewHolder].
     */
-    protected abstract fun onBind(viewHolder: RecyclerView.ViewHolder, item: Item, listener: L)
+    protected abstract fun onBind(
+        viewHolder: RecyclerView.ViewHolder,
+        item: Item,
+        listener: L,
+        payload: List<Any>
+    )

    override fun getItemCount(): Int = data.getItemCount()

@ -94,8 +109,15 @@ abstract class MultiAdapter<L>(private val listener: L) :
            }
            .create(parent.context)

-    override fun onBindViewHolder(holder: RecyclerView.ViewHolder, position: Int) {
-        onBind(holder, data.getItem(position), listener)
+    override fun onBindViewHolder(holder: RecyclerView.ViewHolder, position: Int) =
+        throw UnsupportedOperationException()
+
+    override fun onBindViewHolder(
+        viewHolder: RecyclerView.ViewHolder,
+        position: Int,
+        payload: List<Any>
+    ) {
+        onBind(viewHolder, data.getItem(position), listener, payload)
    }
 }

--- a/app/src/main/java/org/oxycblt/auxio/ui/accent/AccentAdapter.kt
+++ b/app/src/main/java/org/oxycblt/auxio/ui/accent/AccentAdapter.kt
@ -19,7 +19,6 @@ package org.oxycblt.auxio.ui.accent

 import android.content.Context
 import androidx.appcompat.widget.TooltipCompat
-import androidx.recyclerview.widget.RecyclerView
 import org.oxycblt.auxio.R
 import org.oxycblt.auxio.databinding.ItemAccentBinding
 import org.oxycblt.auxio.ui.BackingData
@ -38,28 +37,28 @@ class AccentAdapter(listener: Listener) :
    MonoAdapter<Accent, AccentAdapter.Listener, AccentViewHolder>(listener) {
    var selectedAccent: Accent? = null
        private set
-    private var selectedViewHolder: AccentViewHolder? = null

    override val data = AccentData()
    override val creator = AccentViewHolder.CREATOR

-    override fun onBindViewHolder(viewHolder: AccentViewHolder, position: Int) {
-        super.onBindViewHolder(viewHolder, position)
-
-        if (data.getItem(position) == selectedAccent) {
-            selectedViewHolder?.setSelected(false)
-            selectedViewHolder = viewHolder
-            viewHolder.setSelected(true)
+    override fun onBind(
+        viewHolder: AccentViewHolder,
+        item: Accent,
+        listener: Listener,
+        payload: List<Any>
+    ) {
+        if (payload.isEmpty()) {
+            super.onBind(viewHolder, item, listener, payload)
        }
+
+        viewHolder.setSelected(item == selectedAccent)
    }

-    fun setSelectedAccent(accent: Accent, recycler: RecyclerView) {
+    fun setSelectedAccent(accent: Accent) {
        if (accent == selectedAccent) return
+        selectedAccent?.let { old -> notifyItemChanged(old.index, PAYLOAD_SELECTION_CHANGED) }
        selectedAccent = accent
-        selectedViewHolder?.setSelected(false)
-        selectedViewHolder =
-            recycler.findViewHolderForAdapterPosition(accent.index) as AccentViewHolder?
-        selectedViewHolder?.setSelected(true)
+        notifyItemChanged(accent.index, PAYLOAD_SELECTION_CHANGED)
    }

    interface Listener {
@ -70,6 +69,10 @@ class AccentAdapter(listener: Listener) :
        override fun getItem(position: Int) = Accent.from(position)
        override fun getItemCount() = Accent.MAX
    }
+
+    companion object {
+        val PAYLOAD_SELECTION_CHANGED = Any()
+    }
 }

 class AccentViewHolder private constructor(private val binding: ItemAccentBinding) :
--- a/app/src/main/java/org/oxycblt/auxio/ui/accent/AccentCustomizeDialog.kt
+++ b/app/src/main/java/org/oxycblt/auxio/ui/accent/AccentCustomizeDialog.kt
@ -55,17 +55,13 @@ class AccentCustomizeDialog :
    }

    override fun onBindingCreated(binding: DialogAccentBinding, savedInstanceState: Bundle?) {
-        // --- UI SETUP ---
-
        binding.accentRecycler.adapter = accentAdapter
-
        accentAdapter.setSelectedAccent(
            if (savedInstanceState != null) {
                Accent.from(savedInstanceState.getInt(KEY_PENDING_ACCENT))
            } else {
                settingsManager.accent
-            },
-            binding.accentRecycler)
+            })
    }

    override fun onSaveInstanceState(outState: Bundle) {
@ -78,7 +74,7 @@ class AccentCustomizeDialog :
    }

    override fun onAccentSelected(accent: Accent) {
-        accentAdapter.setSelectedAccent(accent, requireBinding().accentRecycler)
+        accentAdapter.setSelectedAccent(accent)
    }

    companion object {
--- a/app/src/main/java/org/oxycblt/auxio/util/FrameworkUtil.kt
+++ b/app/src/main/java/org/oxycblt/auxio/util/FrameworkUtil.kt
@ -169,7 +169,11 @@ fun Fragment.launch(
    viewLifecycleOwner.lifecycleScope.launch { viewLifecycleOwner.repeatOnLifecycle(state, block) }
 }

-/** Combines the called flow with the given flow and then collects them both into [block]. */
+/**
+ * Combines the called flow with the given flow and then collects them both into [block].
+ * This is a bit of a dumb hack with [combine], as when we have to combine flows, we often
+ * just want to call the same block with both functions, and not do any transformations.
+ */
 suspend fun <T1, T2> Flow<T1>.collectWith(other: Flow<T2>, block: suspend (T1, T2) -> Unit) {
    combine(this, other) { a, b -> a to b }.collect { block(it.first, it.second) }
 }