From fe5609b44790e04043b620133576eb260ff8113e Mon Sep 17 00:00:00 2001 From: Alexander Capehart Date: Tue, 6 Sep 2022 22:39:48 -0600 Subject: [PATCH] ui: fix click/menu issues Fix some issues with how slapshod the menu/click code is. This fixes a crash on the genre view when a song menu was opened. --- CHANGELOG.md | 1 + .../auxio/detail/AlbumDetailFragment.kt | 23 ++++++++----------- .../auxio/detail/ArtistDetailFragment.kt | 6 ++--- .../auxio/detail/GenreDetailFragment.kt | 11 +++------ .../auxio/home/list/AlbumListFragment.kt | 9 +++----- .../auxio/home/list/ArtistListFragment.kt | 11 +++------ .../auxio/home/list/GenreListFragment.kt | 9 +++----- .../auxio/home/list/SongListFragment.kt | 8 +++---- .../java/org/oxycblt/auxio/music/Music.kt | 14 ++++++++++- .../org/oxycblt/auxio/ui/recycler/Adapters.kt | 15 ++++-------- 10 files changed, 45 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5993a51fe..011d43e86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - Fixed issue where the scroll popup would not display correctly in landscape mode [#230] - Fixed issue where the playback progress would continue in the notification even if audio focus was lost +- Fixed issue where the app would crash if a song in the genre menu was opened #### Dev/Meta - Completed migration to reactive playback system diff --git a/app/src/main/java/org/oxycblt/auxio/detail/AlbumDetailFragment.kt b/app/src/main/java/org/oxycblt/auxio/detail/AlbumDetailFragment.kt index 68bc7b228..94fc9f868 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/AlbumDetailFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/AlbumDetailFragment.kt @@ -120,23 +120,18 @@ class AlbumDetailFragment : } override fun onItemClick(item: Item) { - if (item is Song) { - val playbackMode = settings.detailPlaybackMode - if (playbackMode != null) { - playbackModel.play(item, playbackMode) - } else { - playbackModel.playFromAlbum(item) - } + check(item is Song) { "Unexpected datatype: ${item::class.simpleName}" } + val playbackMode = settings.detailPlaybackMode + if (playbackMode != null) { + playbackModel.play(item, playbackMode) + } else { + playbackModel.playFromAlbum(item) } } override fun onOpenMenu(item: Item, anchor: View) { - if (item is Song) { - musicMenu(anchor, R.menu.menu_album_song_actions, item) - return - } - - error("Unexpected datatype when opening menu: ${item::class.java}") + check(item is Song) { "Unexpected datatype: ${item::class.simpleName}" } + musicMenu(anchor, R.menu.menu_album_song_actions, item) } override fun onPlayParent() { @@ -219,7 +214,7 @@ class AlbumDetailFragment : .navigate(AlbumDetailFragmentDirections.actionShowArtist(item.uid)) } null -> {} - else -> error("Unexpected navigation item ${item::class.java}") + else -> error("Unexpected datatype: ${item::class.java}") } } diff --git a/app/src/main/java/org/oxycblt/auxio/detail/ArtistDetailFragment.kt b/app/src/main/java/org/oxycblt/auxio/detail/ArtistDetailFragment.kt index 6083c28cf..a76c27d6d 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/ArtistDetailFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/ArtistDetailFragment.kt @@ -111,7 +111,6 @@ class ArtistDetailFragment : } override fun onItemClick(item: Item) { - when (item) { is Song -> { val playbackMode = settings.detailPlaybackMode @@ -122,6 +121,7 @@ class ArtistDetailFragment : } } is Album -> navModel.exploreNavigateTo(item) + else -> error("Unexpected datatype: ${item::class.simpleName}") } } @@ -129,7 +129,7 @@ class ArtistDetailFragment : when (item) { is Song -> musicMenu(anchor, R.menu.menu_artist_song_actions, item) is Album -> musicMenu(anchor, R.menu.menu_artist_album_actions, item) - else -> error("Unexpected datatype when opening menu: ${item::class.java}") + else -> error("Unexpected datatype: ${item::class.simpleName}") } } @@ -196,7 +196,7 @@ class ArtistDetailFragment : } } null -> {} - else -> error("Unexpected navigation item ${item::class.java}") + else -> error("Unexpected datatype: ${item::class.java}") } } diff --git a/app/src/main/java/org/oxycblt/auxio/detail/GenreDetailFragment.kt b/app/src/main/java/org/oxycblt/auxio/detail/GenreDetailFragment.kt index cf190f591..777aa2420 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/GenreDetailFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/GenreDetailFragment.kt @@ -112,8 +112,7 @@ class GenreDetailFragment : } override fun onItemClick(item: Item) { - check(item is Song) - + check(item is Song) { "Unexpected datatype: ${item::class.simpleName}" } val playbackMode = settings.detailPlaybackMode if (playbackMode != null) { playbackModel.play(item, playbackMode) @@ -123,12 +122,8 @@ class GenreDetailFragment : } override fun onOpenMenu(item: Item, anchor: View) { - if (item is Song) { - musicMenu(anchor, R.menu.menu_song_actions, item) - return - } - - error("Unexpected datatype when opening menu: ${item::class.java}") + check(item is Song) { "Unexpected datatype: ${item::class.simpleName}" } + musicMenu(anchor, R.menu.menu_song_actions, item) } override fun onPlayParent() { diff --git a/app/src/main/java/org/oxycblt/auxio/home/list/AlbumListFragment.kt b/app/src/main/java/org/oxycblt/auxio/home/list/AlbumListFragment.kt index 6be4243b3..82e1c8375 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/list/AlbumListFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/list/AlbumListFragment.kt @@ -25,7 +25,6 @@ import java.util.* import org.oxycblt.auxio.R import org.oxycblt.auxio.databinding.FragmentHomeListBinding import org.oxycblt.auxio.music.Album -import org.oxycblt.auxio.music.Music import org.oxycblt.auxio.music.MusicParent import org.oxycblt.auxio.ui.DisplayMode import org.oxycblt.auxio.ui.Sort @@ -98,15 +97,13 @@ class AlbumListFragment : HomeListFragment() { } override fun onItemClick(item: Item) { - check(item is Music) + check(item is Album) { "Unexpected datatype: ${item::class.java}" } navModel.exploreNavigateTo(item) } override fun onOpenMenu(item: Item, anchor: View) { - when (item) { - is Album -> musicMenu(anchor, R.menu.menu_album_actions, item) - else -> error("Unexpected datatype when opening menu: ${item::class.java}") - } + check(item is Album) { "Unexpected datatype: ${item::class.java}" } + musicMenu(anchor, R.menu.menu_album_actions, item) } private fun handleParent(parent: MusicParent?, isPlaying: Boolean) { diff --git a/app/src/main/java/org/oxycblt/auxio/home/list/ArtistListFragment.kt b/app/src/main/java/org/oxycblt/auxio/home/list/ArtistListFragment.kt index 1be6155cb..224baccd7 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/list/ArtistListFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/list/ArtistListFragment.kt @@ -23,7 +23,6 @@ import android.view.ViewGroup import org.oxycblt.auxio.R import org.oxycblt.auxio.databinding.FragmentHomeListBinding import org.oxycblt.auxio.music.Artist -import org.oxycblt.auxio.music.Music import org.oxycblt.auxio.music.MusicParent import org.oxycblt.auxio.ui.DisplayMode import org.oxycblt.auxio.ui.Sort @@ -74,17 +73,13 @@ class ArtistListFragment : HomeListFragment() { } override fun onItemClick(item: Item) { - check(item is Music) + check(item is Artist) { "Unexpected datatype: ${item::class.java}" } navModel.exploreNavigateTo(item) } override fun onOpenMenu(item: Item, anchor: View) { - if (item is Artist) { - musicMenu(anchor, R.menu.menu_genre_artist_actions, item) - return - } - - error("Unexpected datatype when opening menu: ${item::class.java}") + check(item is Artist) { "Unexpected datatype: ${item::class.java}" } + musicMenu(anchor, R.menu.menu_genre_artist_actions, item) } private fun handleParent(parent: MusicParent?, isPlaying: Boolean) { diff --git a/app/src/main/java/org/oxycblt/auxio/home/list/GenreListFragment.kt b/app/src/main/java/org/oxycblt/auxio/home/list/GenreListFragment.kt index c51c1ab3a..640711ddd 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/list/GenreListFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/list/GenreListFragment.kt @@ -23,7 +23,6 @@ import android.view.ViewGroup import org.oxycblt.auxio.R import org.oxycblt.auxio.databinding.FragmentHomeListBinding import org.oxycblt.auxio.music.Genre -import org.oxycblt.auxio.music.Music import org.oxycblt.auxio.music.MusicParent import org.oxycblt.auxio.ui.DisplayMode import org.oxycblt.auxio.ui.Sort @@ -74,15 +73,13 @@ class GenreListFragment : HomeListFragment() { } override fun onItemClick(item: Item) { - check(item is Music) + check(item is Genre) { "Unexpected datatype: ${item::class.java}" } navModel.exploreNavigateTo(item) } override fun onOpenMenu(item: Item, anchor: View) { - when (item) { - is Genre -> musicMenu(anchor, R.menu.menu_genre_artist_actions, item) - else -> error("Unexpected datatype when opening menu: ${item::class.java}") - } + check(item is Genre) { "Unexpected datatype: ${item::class.java}" } + musicMenu(anchor, R.menu.menu_genre_artist_actions, item) } private fun handlePlayback(parent: MusicParent?, isPlaying: Boolean) { diff --git a/app/src/main/java/org/oxycblt/auxio/home/list/SongListFragment.kt b/app/src/main/java/org/oxycblt/auxio/home/list/SongListFragment.kt index 37f0f3a72..78d578e10 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/list/SongListFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/list/SongListFragment.kt @@ -103,15 +103,13 @@ class SongListFragment : HomeListFragment() { } override fun onItemClick(item: Item) { - check(item is Song) + check(item is Song) { "Unexpected datatype: ${item::class.java}" } playbackModel.play(item, settings.libPlaybackMode) } override fun onOpenMenu(item: Item, anchor: View) { - when (item) { - is Song -> musicMenu(anchor, R.menu.menu_song_actions, item) - else -> error("Unexpected datatype when opening menu: ${item::class.java}") - } + check(item is Song) { "Unexpected datatype: ${item::class.java}" } + musicMenu(anchor, R.menu.menu_song_actions, item) } private fun handlePlayback(song: Song?, parent: MusicParent?, isPlaying: Boolean) { diff --git a/app/src/main/java/org/oxycblt/auxio/music/Music.kt b/app/src/main/java/org/oxycblt/auxio/music/Music.kt index 890ad4f28..24c976d0f 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/Music.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/Music.kt @@ -71,7 +71,18 @@ sealed class Music : Item { override fun equals(other: Any?) = other is Music && javaClass == other.javaClass && uid == other.uid - /** A unique identifier for a piece of music. */ + /** + * A unique identifier for a piece of music. + * + * UID enables a much cheaper and more reliable form of differentiating music, derived from + * either a hash of meaningful metadata or the MusicBrainz UUID spec. It is the default datatype + * used when comparing music, and it is also the datatype used when serializing music to + * external sources. + * + * TODO: Verify hash mechanism works + * + * @author OxygenCobalt + */ @Parcelize class UID private constructor(val datatype: String, val isMusicBrainz: Boolean, val uuid: UUID) : @@ -134,6 +145,7 @@ sealed class Music : Item { val digest = MessageDigest.getInstance("MD5") updates(digest) + // Make the MD5 hash and then bitshift it into a UUID. val hash = digest.digest() val uuid = UUID( diff --git a/app/src/main/java/org/oxycblt/auxio/ui/recycler/Adapters.kt b/app/src/main/java/org/oxycblt/auxio/ui/recycler/Adapters.kt index 0e1b0b18f..abd66548d 100644 --- a/app/src/main/java/org/oxycblt/auxio/ui/recycler/Adapters.kt +++ b/app/src/main/java/org/oxycblt/auxio/ui/recycler/Adapters.kt @@ -37,11 +37,7 @@ abstract class IndicatorAdapter : RecyclerView.Ada if (holder is ViewHolder) { val item = currentList[position] val currentItem = currentItem - holder.updateIndicator( - currentItem != null && - item.javaClass == currentItem.javaClass && - item == currentItem, - isPlaying) + holder.updateIndicator(item == currentItem, isPlaying) } } @@ -55,10 +51,7 @@ abstract class IndicatorAdapter : RecyclerView.Ada currentItem = item if (oldItem != null) { - val pos = - currentList.indexOfFirst { - it.javaClass == oldItem.javaClass && item == currentItem - } + val pos = currentList.indexOfFirst { it == oldItem } if (pos > -1) { notifyItemChanged(pos, PAYLOAD_INDICATOR_CHANGED) @@ -68,7 +61,7 @@ abstract class IndicatorAdapter : RecyclerView.Ada } if (item != null) { - val pos = currentList.indexOfFirst { it.javaClass == item.javaClass && it == item } + val pos = currentList.indexOfFirst { it == item } if (pos > -1) { notifyItemChanged(pos, PAYLOAD_INDICATOR_CHANGED) @@ -84,7 +77,7 @@ abstract class IndicatorAdapter : RecyclerView.Ada this.isPlaying = isPlaying if (!updatedItem && item != null) { - val pos = currentList.indexOfFirst { it.javaClass == item.javaClass && it == item } + val pos = currentList.indexOfFirst { it == item } if (pos > -1) { notifyItemChanged(pos, PAYLOAD_INDICATOR_CHANGED)