From c2def19aee79857ffe6371c904ce6dfd37468229 Mon Sep 17 00:00:00 2001 From: Alexander Capehart Date: Thu, 25 May 2023 13:10:49 -0600 Subject: [PATCH] ui: handle playing indicator edge cases Handle two edge cases identified with the playing indicator behavior: 1. When enqueing songs from another parent, the prior parent is still indicates as "playing" when it kind-of isn't. 2. When playback is stopped, the parent is not reset, and thus will still be indicated as "playing" after the song has disappeared. This is rarer and should be resolved in other ways, but the solution to 1 also fixes this. Resolves #380. --- .../auxio/detail/ArtistDetailFragment.kt | 6 +++--- .../auxio/detail/GenreDetailFragment.kt | 21 +++++++++++-------- .../auxio/home/list/AlbumListFragment.kt | 12 +++++++---- .../auxio/home/list/ArtistListFragment.kt | 12 +++++++---- .../auxio/home/list/GenreListFragment.kt | 12 +++++++---- .../auxio/home/list/PlaylistListFragment.kt | 14 +++++++------ .../auxio/home/list/SongListFragment.kt | 8 ++----- 7 files changed, 49 insertions(+), 36 deletions(-) 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 310d9bc35..5384ead4c 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/ArtistDetailFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/ArtistDetailFragment.kt @@ -254,14 +254,14 @@ class ArtistDetailFragment : val currentArtist = unlikelyToBeNull(detailModel.currentArtist.value) val playingItem = when (parent) { - // Always highlight a playing album if it's from this artist. - is Album -> parent + // Always highlight a playing album if it's from this artist, and if the currently + // playing song is contained within. + is Album -> parent.takeIf { song?.album == it } // If the parent is the artist itself, use the currently playing song. currentArtist -> song // Nothing is playing from this artist. else -> null } - artistListAdapter.setPlaying(playingItem, isPlaying) } 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 b3d417761..c178f4a1a 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/GenreDetailFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/GenreDetailFragment.kt @@ -239,15 +239,18 @@ class GenreDetailFragment : } private fun updatePlayback(song: Song?, parent: MusicParent?, isPlaying: Boolean) { - var playingMusic: Music? = null - if (parent is Artist) { - playingMusic = parent - } - // Prefer songs that might be playing from this genre. - if (parent is Genre && parent.uid == unlikelyToBeNull(detailModel.currentGenre.value).uid) { - playingMusic = song - } - genreListAdapter.setPlaying(playingMusic, isPlaying) + val currentGenre = unlikelyToBeNull(detailModel.currentGenre.value) + val playingItem = + when (parent) { + // Always highlight a playing artist if it's from this genre, and if the currently + // playing song is contained within. + is Artist -> parent.takeIf { song?.run { artists.contains(it) } ?: false } + // If the parent is the artist itself, use the currently playing song. + currentGenre -> song + // Nothing is playing from this artist. + else -> null + } + genreListAdapter.setPlaying(playingItem, isPlaying) } private fun handleNavigation(item: Music?) { 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 c81b15ee1..31b6b67bf 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 @@ -41,6 +41,7 @@ import org.oxycblt.auxio.music.Music import org.oxycblt.auxio.music.MusicMode import org.oxycblt.auxio.music.MusicParent import org.oxycblt.auxio.music.MusicViewModel +import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.navigation.NavigationViewModel import org.oxycblt.auxio.playback.PlaybackViewModel import org.oxycblt.auxio.playback.formatDurationMs @@ -82,7 +83,8 @@ class AlbumListFragment : collectImmediately(homeModel.albumsList, ::updateAlbums) collectImmediately(selectionModel.selected, ::updateSelection) - collectImmediately(playbackModel.parent, playbackModel.isPlaying, ::updatePlayback) + collectImmediately( + playbackModel.song, playbackModel.parent, playbackModel.isPlaying, ::updatePlayback) } override fun onDestroyBinding(binding: FragmentHomeListBinding) { @@ -151,9 +153,11 @@ class AlbumListFragment : albumAdapter.setSelected(selection.filterIsInstanceTo(mutableSetOf())) } - private fun updatePlayback(parent: MusicParent?, isPlaying: Boolean) { - // If an album is playing, highlight it within this adapter. - albumAdapter.setPlaying(parent as? Album, isPlaying) + private fun updatePlayback(song: Song?, parent: MusicParent?, isPlaying: Boolean) { + // Only highlight the album if it is currently playing, and if the currently + // playing song is also contained within. + val playlist = (parent as? Album)?.takeIf { song?.album == it } + albumAdapter.setPlaying(playlist, isPlaying) } /** 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 7fd4adf23..46d42f16c 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 @@ -39,6 +39,7 @@ import org.oxycblt.auxio.music.Music import org.oxycblt.auxio.music.MusicMode import org.oxycblt.auxio.music.MusicParent import org.oxycblt.auxio.music.MusicViewModel +import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.navigation.NavigationViewModel import org.oxycblt.auxio.playback.PlaybackViewModel import org.oxycblt.auxio.playback.formatDurationMs @@ -78,7 +79,8 @@ class ArtistListFragment : collectImmediately(homeModel.artistsList, ::updateArtists) collectImmediately(selectionModel.selected, ::updateSelection) - collectImmediately(playbackModel.parent, playbackModel.isPlaying, ::updatePlayback) + collectImmediately( + playbackModel.song, playbackModel.parent, playbackModel.isPlaying, ::updatePlayback) } override fun onDestroyBinding(binding: FragmentHomeListBinding) { @@ -128,9 +130,11 @@ class ArtistListFragment : artistAdapter.setSelected(selection.filterIsInstanceTo(mutableSetOf())) } - private fun updatePlayback(parent: MusicParent?, isPlaying: Boolean) { - // If an artist is playing, highlight it within this adapter. - artistAdapter.setPlaying(parent as? Artist, isPlaying) + private fun updatePlayback(song: Song?, parent: MusicParent?, isPlaying: Boolean) { + // Only highlight the artist if it is currently playing, and if the currently + // playing song is also contained within. + val playlist = (parent as? Artist)?.takeIf { song?.run { artists.contains(it) } ?: false } + artistAdapter.setPlaying(playlist, isPlaying) } /** 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 d1f44eb0d..d592ba377 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 @@ -39,6 +39,7 @@ import org.oxycblt.auxio.music.Music import org.oxycblt.auxio.music.MusicMode import org.oxycblt.auxio.music.MusicParent import org.oxycblt.auxio.music.MusicViewModel +import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.navigation.NavigationViewModel import org.oxycblt.auxio.playback.PlaybackViewModel import org.oxycblt.auxio.playback.formatDurationMs @@ -77,7 +78,8 @@ class GenreListFragment : collectImmediately(homeModel.genresList, ::updateGenres) collectImmediately(selectionModel.selected, ::updateSelection) - collectImmediately(playbackModel.parent, playbackModel.isPlaying, ::updatePlayback) + collectImmediately( + playbackModel.song, playbackModel.parent, playbackModel.isPlaying, ::updatePlayback) } override fun onDestroyBinding(binding: FragmentHomeListBinding) { @@ -127,9 +129,11 @@ class GenreListFragment : genreAdapter.setSelected(selection.filterIsInstanceTo(mutableSetOf())) } - private fun updatePlayback(parent: MusicParent?, isPlaying: Boolean) { - // If a genre is playing, highlight it within this adapter. - genreAdapter.setPlaying(parent as? Genre, isPlaying) + private fun updatePlayback(song: Song?, parent: MusicParent?, isPlaying: Boolean) { + // Only highlight the genre if it is currently playing, and if the currently + // playing song is also contained within. + val playlist = (parent as? Genre)?.takeIf { song?.run { genres.contains(it) } ?: false } + genreAdapter.setPlaying(playlist, isPlaying) } /** diff --git a/app/src/main/java/org/oxycblt/auxio/home/list/PlaylistListFragment.kt b/app/src/main/java/org/oxycblt/auxio/home/list/PlaylistListFragment.kt index 1499fcb8f..167619c15 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/list/PlaylistListFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/list/PlaylistListFragment.kt @@ -38,6 +38,7 @@ import org.oxycblt.auxio.music.MusicMode import org.oxycblt.auxio.music.MusicParent import org.oxycblt.auxio.music.MusicViewModel import org.oxycblt.auxio.music.Playlist +import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.navigation.NavigationViewModel import org.oxycblt.auxio.playback.PlaybackViewModel import org.oxycblt.auxio.playback.formatDurationMs @@ -48,8 +49,6 @@ import org.oxycblt.auxio.util.logD * A [ListFragment] that shows a list of [Playlist]s. * * @author Alexander Capehart (OxygenCobalt) - * - * TODO: Show a placeholder when there are no playlists. */ class PlaylistListFragment : ListFragment(), @@ -77,7 +76,8 @@ class PlaylistListFragment : collectImmediately(homeModel.playlistsList, ::updatePlaylists) collectImmediately(selectionModel.selected, ::updateSelection) - collectImmediately(playbackModel.parent, playbackModel.isPlaying, ::updatePlayback) + collectImmediately( + playbackModel.song, playbackModel.parent, playbackModel.isPlaying, ::updatePlayback) } override fun onDestroyBinding(binding: FragmentHomeListBinding) { @@ -128,9 +128,11 @@ class PlaylistListFragment : playlistAdapter.setSelected(selection.filterIsInstanceTo(mutableSetOf())) } - private fun updatePlayback(parent: MusicParent?, isPlaying: Boolean) { - // If a playlist is playing, highlight it within this adapter. - playlistAdapter.setPlaying(parent as? Playlist, isPlaying) + private fun updatePlayback(song: Song?, parent: MusicParent?, isPlaying: Boolean) { + // Only highlight the playlist if it is currently playing, and if the currently + // playing song is also contained within. + val playlist = (parent as? Playlist)?.takeIf { it.songs.contains(song) } ?: return + playlistAdapter.setPlaying(playlist, isPlaying) } /** 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 12423dabf..62643f4cf 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 @@ -155,12 +155,8 @@ class SongListFragment : } private fun updatePlayback(song: Song?, parent: MusicParent?, isPlaying: Boolean) { - if (parent == null) { - songAdapter.setPlaying(song, isPlaying) - } else { - // Ignore playback that is not from all songs - songAdapter.setPlaying(null, isPlaying) - } + // Only indicate playback that is from all songs + songAdapter.setPlaying(song.takeIf { parent == null }, isPlaying) } /**