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) } }