From 572b0e52f81ad0b7f1154c5e6214dcb3a95c25d4 Mon Sep 17 00:00:00 2001 From: Alexander Capehart Date: Sat, 20 May 2023 11:23:16 -0600 Subject: [PATCH] music: clean up playlist experience Add a variety of mild fixes and qol improvements regarding playlists. --- .../oxycblt/auxio/detail/DetailViewModel.kt | 28 ++++++++++--------- .../auxio/detail/PlaylistDetailFragment.kt | 11 ++++---- .../header/PlaylistDetailHeaderAdapter.kt | 1 + .../detail/list/PlaylistDetailListAdapter.kt | 3 +- .../auxio/home/list/PlaylistListFragment.kt | 7 +++++ .../auxio/list/adapter/FlexibleListAdapter.kt | 5 ++-- .../auxio/music/system/IndexerService.kt | 1 - .../auxio/playback/PlaybackPanelFragment.kt | 8 ++++++ .../org/oxycblt/auxio/playback/queue/Queue.kt | 2 +- .../playback/state/PlaybackStateManager.kt | 19 +++++++++---- .../main/res/drawable-v23/ui_item_ripple.xml | 3 +- app/src/main/res/drawable/ic_save_24.xml | 3 +- ...tem_ripple_bg.xml => sel_selection_bg.xml} | 0 app/src/main/res/drawable/ui_item_bg.xml | 5 ++++ app/src/main/res/drawable/ui_item_ripple.xml | 1 - app/src/main/res/layout/item_album_song.xml | 2 +- .../main/res/layout/item_editable_song.xml | 2 +- app/src/main/res/layout/item_parent.xml | 2 +- app/src/main/res/layout/item_song.xml | 2 +- app/src/main/res/menu/menu_playback.xml | 3 ++ .../res/menu/menu_playlist_song_actions.xml | 18 ++++++++++++ 21 files changed, 89 insertions(+), 37 deletions(-) rename app/src/main/res/drawable/{sel_item_ripple_bg.xml => sel_selection_bg.xml} (100%) create mode 100644 app/src/main/res/drawable/ui_item_bg.xml create mode 100644 app/src/main/res/menu/menu_playlist_song_actions.xml diff --git a/app/src/main/java/org/oxycblt/auxio/detail/DetailViewModel.kt b/app/src/main/java/org/oxycblt/auxio/detail/DetailViewModel.kt index 367ef3545..e62859543 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/DetailViewModel.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/DetailViewModel.kt @@ -348,7 +348,6 @@ constructor( * @param at The position of the item to remove, in the list adapter data. */ fun removePlaylistSong(at: Int) { - // TODO: Remove header when empty val playlist = _currentPlaylist.value ?: return val editedPlaylist = (_editedPlaylist.value ?: return).toMutableList() val realAt = at - 2 @@ -357,7 +356,13 @@ constructor( } editedPlaylist.removeAt(realAt) _editedPlaylist.value = editedPlaylist - refreshPlaylistList(playlist, UpdateInstructions.Remove(at)) + refreshPlaylistList( + playlist, + if (editedPlaylist.isNotEmpty()) { + UpdateInstructions.Remove(at, 1) + } else { + UpdateInstructions.Remove(at - 2, 3) + }) } private fun refreshAudioInfo(song: Song) { @@ -490,18 +495,15 @@ constructor( logD("Refreshing playlist list") val list = mutableListOf() - val newInstructions = - if (playlist.songs.isNotEmpty()) { - val header = EditHeader(R.string.lbl_songs) - list.add(Divider(header)) - list.add(header) - list.addAll(_editedPlaylist.value ?: playlist.songs) - instructions - } else { - UpdateInstructions.Diff - } + val songs = editedPlaylist.value ?: playlist.songs + if (songs.isNotEmpty()) { + val header = EditHeader(R.string.lbl_songs) + list.add(Divider(header)) + list.add(header) + list.addAll(songs) + } - _playlistInstructions.put(newInstructions) + _playlistInstructions.put(instructions) _playlistList.value = list } diff --git a/app/src/main/java/org/oxycblt/auxio/detail/PlaylistDetailFragment.kt b/app/src/main/java/org/oxycblt/auxio/detail/PlaylistDetailFragment.kt index 4b8c2650c..4bd406ed0 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/PlaylistDetailFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/PlaylistDetailFragment.kt @@ -211,8 +211,7 @@ class PlaylistDetailFragment : } override fun onOpenMenu(item: Song, anchor: View) { - // TODO: Remove "Add to playlist" option, makes no sense - openMusicMenu(anchor, R.menu.menu_song_actions, item) + openMusicMenu(anchor, R.menu.menu_playlist_song_actions, item) } override fun onPlay() { @@ -284,9 +283,11 @@ class PlaylistDetailFragment : playlistHeaderAdapter.setEditedPlaylist(editedPlaylist) selectionModel.drop() - logD(editedPlaylist == detailModel.currentPlaylist.value?.songs) - requireBinding().detailEditToolbar.menu.findItem(R.id.action_save).isEnabled = - editedPlaylist != detailModel.currentPlaylist.value?.songs + if (editedPlaylist != null) { + requireBinding().detailEditToolbar.menu.findItem(R.id.action_save).apply { + isEnabled = editedPlaylist != detailModel.currentPlaylist.value?.songs + } + } updateMultiToolbar() } diff --git a/app/src/main/java/org/oxycblt/auxio/detail/header/PlaylistDetailHeaderAdapter.kt b/app/src/main/java/org/oxycblt/auxio/detail/header/PlaylistDetailHeaderAdapter.kt index 375278a39..08ce66ba4 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/header/PlaylistDetailHeaderAdapter.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/header/PlaylistDetailHeaderAdapter.kt @@ -83,6 +83,7 @@ private constructor(private val binding: ItemDetailHeaderBinding) : editedPlaylist: List?, listener: DetailHeaderAdapter.Listener ) { + // TODO: Debug perpetually re-binding images binding.detailCover.bind(playlist, editedPlaylist) binding.detailType.text = binding.context.getString(R.string.lbl_playlist) binding.detailName.text = playlist.name.resolve(binding.context) diff --git a/app/src/main/java/org/oxycblt/auxio/detail/list/PlaylistDetailListAdapter.kt b/app/src/main/java/org/oxycblt/auxio/detail/list/PlaylistDetailListAdapter.kt index 81fde8777..69e0509d5 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/list/PlaylistDetailListAdapter.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/list/PlaylistDetailListAdapter.kt @@ -99,7 +99,7 @@ class PlaylistDetailListAdapter(private val listener: Listener) : return } this.isEditing = editing - notifyItemRangeChanged(1, currentList.size - 2, PAYLOAD_EDITING_CHANGED) + notifyItemRangeChanged(1, currentList.size - 1, PAYLOAD_EDITING_CHANGED) } /** An extended [DetailListAdapter.Listener] for [PlaylistDetailListAdapter]. */ @@ -256,6 +256,7 @@ private constructor(private val binding: ItemEditableSongBinding) : } override fun updateSelectionIndicator(isSelected: Boolean) { + binding.interactBody.isActivated = isSelected binding.songAlbumCover.isActivated = isSelected } 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 a41abdd1d..4c5d8d19a 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 @@ -44,6 +44,13 @@ import org.oxycblt.auxio.playback.formatDurationMs import org.oxycblt.auxio.util.collectImmediately 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(), FastScrollRecyclerView.PopupProvider, diff --git a/app/src/main/java/org/oxycblt/auxio/list/adapter/FlexibleListAdapter.kt b/app/src/main/java/org/oxycblt/auxio/list/adapter/FlexibleListAdapter.kt index c76ffaae6..b9d77b0f8 100644 --- a/app/src/main/java/org/oxycblt/auxio/list/adapter/FlexibleListAdapter.kt +++ b/app/src/main/java/org/oxycblt/auxio/list/adapter/FlexibleListAdapter.kt @@ -93,8 +93,9 @@ sealed interface UpdateInstructions { * Remove an item. * * @param at The location that the item should be removed from. + * @param size The amount of items to add. */ - data class Remove(val at: Int) : UpdateInstructions + data class Remove(val at: Int, val size: Int) : UpdateInstructions } /** @@ -147,7 +148,7 @@ private class FlexibleListDiffer( } is UpdateInstructions.Remove -> { currentList = newList - updateCallback.onRemoved(instructions.at, 1) + updateCallback.onRemoved(instructions.at, instructions.size) callback?.invoke() } is UpdateInstructions.Diff, diff --git a/app/src/main/java/org/oxycblt/auxio/music/system/IndexerService.kt b/app/src/main/java/org/oxycblt/auxio/music/system/IndexerService.kt index 557dda7c4..eee390b11 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/system/IndexerService.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/system/IndexerService.kt @@ -131,7 +131,6 @@ class IndexerService : override val scope = indexScope override fun onMusicChanges(changes: MusicRepository.Changes) { - // TODO: Do not pause when playlist changes val deviceLibrary = musicRepository.deviceLibrary ?: return // Wipe possibly-invalidated outdated covers imageLoader.memoryCache?.clear() diff --git a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt index 636e2e2ca..f7bad82e9 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt @@ -34,6 +34,7 @@ import org.oxycblt.auxio.MainFragmentDirections import org.oxycblt.auxio.R import org.oxycblt.auxio.databinding.FragmentPlaybackPanelBinding import org.oxycblt.auxio.music.MusicParent +import org.oxycblt.auxio.music.MusicViewModel import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.music.resolveNames import org.oxycblt.auxio.navigation.MainNavigationAction @@ -50,6 +51,8 @@ import org.oxycblt.auxio.util.systemBarInsetsCompat * available controls. * * @author Alexander Capehart (OxygenCobalt) + * + * TODO: Improve flickering situation on play button */ @AndroidEntryPoint class PlaybackPanelFragment : @@ -57,6 +60,7 @@ class PlaybackPanelFragment : Toolbar.OnMenuItemClickListener, StyledSeekBar.Listener { private val playbackModel: PlaybackViewModel by activityViewModels() + private val musicModel: MusicViewModel by activityViewModels() private val navModel: NavigationViewModel by activityViewModels() private var equalizerLauncher: ActivityResultLauncher? = null @@ -164,6 +168,10 @@ class PlaybackPanelFragment : navigateToCurrentAlbum() true } + R.id.action_playlist_add -> { + playbackModel.song.value?.let(musicModel::addToPlaylist) + true + } R.id.action_song_detail -> { playbackModel.song.value?.let { song -> navModel.mainNavigateTo( diff --git a/app/src/main/java/org/oxycblt/auxio/playback/queue/Queue.kt b/app/src/main/java/org/oxycblt/auxio/playback/queue/Queue.kt index 1ccf3b4ab..434e8f479 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/queue/Queue.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/queue/Queue.kt @@ -306,7 +306,7 @@ class EditableQueue : Queue { else -> Queue.Change.Type.MAPPING } check() - return Queue.Change(type, UpdateInstructions.Remove(at)) + return Queue.Change(type, UpdateInstructions.Remove(at, 1)) } /** diff --git a/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt b/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt index c15982e03..63fb85ed2 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt @@ -534,17 +534,24 @@ class PlaybackStateManagerImpl @Inject constructor() : PlaybackStateManager { val internalPlayer = internalPlayer ?: return logD("Restoring state $savedState") + val lastSong = queue.currentSong parent = savedState.parent queue.applySavedState(savedState.queueState) repeatMode = savedState.repeatMode notifyNewPlayback() - // Continuing playback while also possibly doing drastic state updates is - // a bad idea, so pause. - internalPlayer.loadSong(queue.currentSong, false) - if (queue.currentSong != null) { - // Internal player may have reloaded the media item, re-seek to the previous position - seekTo(savedState.positionMs) + // Check if we need to reload the player with a new music file, or if we can just leave + // it be. Specifically done so we don't pause on music updates that don't really change + // what's playing (ex. playlist editing) + if (lastSong != queue.currentSong) { + // Continuing playback while also possibly doing drastic state updates is + // a bad idea, so pause. + internalPlayer.loadSong(queue.currentSong, false) + if (queue.currentSong != null) { + // Internal player may have reloaded the media item, re-seek to the previous + // position + seekTo(savedState.positionMs) + } } isInitialized = true } diff --git a/app/src/main/res/drawable-v23/ui_item_ripple.xml b/app/src/main/res/drawable-v23/ui_item_ripple.xml index f8f2d8917..8f0d43cfb 100644 --- a/app/src/main/res/drawable-v23/ui_item_ripple.xml +++ b/app/src/main/res/drawable-v23/ui_item_ripple.xml @@ -1,6 +1,5 @@ - - + \ No newline at end of file diff --git a/app/src/main/res/drawable/ic_save_24.xml b/app/src/main/res/drawable/ic_save_24.xml index 3761438c0..4fc73a9f3 100644 --- a/app/src/main/res/drawable/ic_save_24.xml +++ b/app/src/main/res/drawable/ic_save_24.xml @@ -4,8 +4,9 @@ android:height="24dp" android:viewportWidth="960" android:viewportHeight="960" - android:tint="?attr/colorControlNormal"> + android:tint="@color/sel_activatable_icon"> + diff --git a/app/src/main/res/drawable/sel_item_ripple_bg.xml b/app/src/main/res/drawable/sel_selection_bg.xml similarity index 100% rename from app/src/main/res/drawable/sel_item_ripple_bg.xml rename to app/src/main/res/drawable/sel_selection_bg.xml diff --git a/app/src/main/res/drawable/ui_item_bg.xml b/app/src/main/res/drawable/ui_item_bg.xml new file mode 100644 index 000000000..fb0a9dec3 --- /dev/null +++ b/app/src/main/res/drawable/ui_item_bg.xml @@ -0,0 +1,5 @@ + + + + + \ No newline at end of file diff --git a/app/src/main/res/drawable/ui_item_ripple.xml b/app/src/main/res/drawable/ui_item_ripple.xml index 03fd102f4..10aa281e7 100644 --- a/app/src/main/res/drawable/ui_item_ripple.xml +++ b/app/src/main/res/drawable/ui_item_ripple.xml @@ -1,5 +1,4 @@ - \ No newline at end of file diff --git a/app/src/main/res/layout/item_album_song.xml b/app/src/main/res/layout/item_album_song.xml index 7a0e7ece7..2505dcf32 100644 --- a/app/src/main/res/layout/item_album_song.xml +++ b/app/src/main/res/layout/item_album_song.xml @@ -4,7 +4,7 @@ xmlns:tools="http://schemas.android.com/tools" android:layout_width="match_parent" android:layout_height="wrap_content" - android:background="@drawable/ui_item_ripple" + android:background="@drawable/ui_item_bg" android:paddingStart="@dimen/spacing_medium" android:paddingTop="@dimen/spacing_mid_medium" android:paddingEnd="@dimen/spacing_mid_medium" diff --git a/app/src/main/res/layout/item_editable_song.xml b/app/src/main/res/layout/item_editable_song.xml index 9cfa194eb..93fe6f0de 100644 --- a/app/src/main/res/layout/item_editable_song.xml +++ b/app/src/main/res/layout/item_editable_song.xml @@ -32,7 +32,7 @@ android:id="@+id/interact_body" android:layout_width="match_parent" android:layout_height="wrap_content" - android:background="?attr/selectableItemBackground"> + android:background="@drawable/ui_item_ripple"> + + + + + + + + \ No newline at end of file