From e73d10070bb55e0728c520643c679c3f114376ec Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Thu, 2 Jun 2022 10:42:33 -0600 Subject: [PATCH] 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. --- CHANGELOG.md | 2 ++ .../detail/recycler/AlbumDetailAdapter.kt | 26 ++++++++------ .../detail/recycler/ArtistDetailAdapter.kt | 30 +++++++++------- .../auxio/detail/recycler/DetailAdapter.kt | 15 +++++--- .../detail/recycler/GenreDetailAdapter.kt | 25 +++++++++----- .../org/oxycblt/auxio/home/tabs/TabAdapter.kt | 12 +++---- .../org/oxycblt/auxio/search/SearchAdapter.kt | 3 +- .../org/oxycblt/auxio/ui/RecyclerFramework.kt | 34 +++++++++++++++---- .../oxycblt/auxio/ui/accent/AccentAdapter.kt | 31 +++++++++-------- .../auxio/ui/accent/AccentCustomizeDialog.kt | 8 ++--- .../org/oxycblt/auxio/util/FrameworkUtil.kt | 6 +++- 11 files changed, 123 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 027cedebc..ee6730fd4 100644 --- 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 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 abdd3a7ee..83c9750bb 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,7 @@ import org.oxycblt.auxio.util.textSafe class AlbumDetailAdapter(listener: Listener) : DetailAdapter(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 + ) { + 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 { 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 fc5ff177a..9709a6894 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,7 +45,7 @@ import org.oxycblt.auxio.util.textSafe class ArtistDetailAdapter(listener: Listener) : DetailAdapter(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 + ) { + 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 */ 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 915b15b07..7a18abd15 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 @@ -71,10 +71,17 @@ abstract class DetailAdapter( 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 + ) { + 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) { 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 8b1b7d093..c4bc29163 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,7 @@ import org.oxycblt.auxio.util.textSafe class GenreDetailAdapter(listener: Listener) : DetailAdapter(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 + ) { + 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 { diff --git a/app/src/main/java/org/oxycblt/auxio/home/tabs/TabAdapter.kt b/app/src/main/java/org/oxycblt/auxio/home/tabs/TabAdapter.kt index e07658d1f..403f6bc00 100644 --- 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(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) diff --git a/app/src/main/java/org/oxycblt/auxio/search/SearchAdapter.kt b/app/src/main/java/org/oxycblt/auxio/search/SearchAdapter.kt index 934963bb6..ae4398782 100644 --- 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 override fun onBind( viewHolder: RecyclerView.ViewHolder, item: Item, - listener: MenuItemListener + listener: MenuItemListener, + payload: List ) { when (item) { is Song -> (viewHolder as SongViewHolder).bind(item, listener) diff --git a/app/src/main/java/org/oxycblt/auxio/ui/RecyclerFramework.kt b/app/src/main/java/org/oxycblt/auxio/ui/RecyclerFramework.kt index b63c2a44a..dc49a3320 100644 --- 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>(private val liste /** The creator instance that all viewholders will be derived from. */ protected abstract val creator: BindingViewHolder.Creator + /** + * 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) { + 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) { + onBind(viewHolder, data.getItem(position), listener, payload) } } @@ -53,7 +63,7 @@ private typealias AnyCreator = BindingViewHolder.Creator(private val listener: L) : @@ -78,7 +88,12 @@ abstract class MultiAdapter(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 + ) override fun getItemCount(): Int = data.getItemCount() @@ -94,8 +109,15 @@ abstract class MultiAdapter(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 + ) { + onBind(viewHolder, data.getItem(position), listener, payload) } } diff --git a/app/src/main/java/org/oxycblt/auxio/ui/accent/AccentAdapter.kt b/app/src/main/java/org/oxycblt/auxio/ui/accent/AccentAdapter.kt index ee8066e04..4eb9cd822 100644 --- 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(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 + ) { + 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) : diff --git a/app/src/main/java/org/oxycblt/auxio/ui/accent/AccentCustomizeDialog.kt b/app/src/main/java/org/oxycblt/auxio/ui/accent/AccentCustomizeDialog.kt index c255603b1..89e31d73d 100644 --- 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 { diff --git a/app/src/main/java/org/oxycblt/auxio/util/FrameworkUtil.kt b/app/src/main/java/org/oxycblt/auxio/util/FrameworkUtil.kt index b3a3f87b3..ec11f8a5b 100644 --- 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 Flow.collectWith(other: Flow, block: suspend (T1, T2) -> Unit) { combine(this, other) { a, b -> a to b }.collect { block(it.first, it.second) } }