From 33abb1419ad85e83e442aacff902ed2f4a822fc0 Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Thu, 31 Dec 2020 08:05:34 -0700 Subject: [PATCH] Refactor playing indicator holder usage Collapse the two lastHolder values into one value inside the detail adapters in order to cut down on needless redundancy. --- .../auxio/detail/AlbumDetailFragment.kt | 33 ++----------------- .../auxio/detail/ArtistDetailFragment.kt | 28 ++-------------- .../oxycblt/auxio/detail/DetailFragment.kt | 7 ---- .../detail/adapters/AlbumDetailAdapter.kt | 20 ++++++++++- .../detail/adapters/ArtistDetailAdapter.kt | 18 +++++++++- .../java/org/oxycblt/auxio/music/Models.kt | 2 +- .../auxio/playback/NotificationUtils.kt | 2 +- .../oxycblt/auxio/playback/PlaybackService.kt | 2 +- .../auxio/playback/queue/QueueFragment.kt | 2 +- .../playback/state/PlaybackStateManager.kt | 2 -- app/src/main/res/layout/fragment_detail.xml | 1 - app/src/main/res/values/styles.xml | 1 + 12 files changed, 46 insertions(+), 72 deletions(-) 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 56ebde771..bdb8f68e8 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/AlbumDetailFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/AlbumDetailFragment.kt @@ -16,7 +16,6 @@ import org.oxycblt.auxio.music.MusicStore import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.playback.state.PlaybackMode import org.oxycblt.auxio.recycler.CenterSmoothScroller -import org.oxycblt.auxio.recycler.Highlightable import org.oxycblt.auxio.ui.createToast import org.oxycblt.auxio.ui.setupAlbumSongActions @@ -136,42 +135,16 @@ class AlbumDetailFragment : DetailFragment() { if (playbackModel.mode.value == PlaybackMode.IN_ALBUM && playbackModel.parent.value?.id == detailModel.currentAlbum.value!!.id ) { - detailAdapter.setCurrentSong(song) - - lastHolder?.setHighlighted(false) - lastHolder = null - - if (song != null) { - // Use existing data instead of having to re-sort it. - val pos = detailAdapter.currentList.indexOfFirst { - it.name == song.name - } - - // Check if the ViewHolder for this song is visible, if it is then highlight it. - // If the ViewHolder is not visible, then the adapter should take care of it if it does become visible. - binding.detailRecycler.layoutManager?.findViewByPosition(pos)?.let { child -> - binding.detailRecycler.getChildViewHolder(child)?.let { - lastHolder = it as Highlightable - - lastHolder?.setHighlighted(true) - } - } - } + detailAdapter.highlightSong(song, binding.detailRecycler) } else { // Clear the viewholders if the mode isn't ALL_SONGS - detailAdapter.setCurrentSong(null) - - lastHolder?.setHighlighted(false) - lastHolder = null + detailAdapter.highlightSong(null, binding.detailRecycler) } } playbackModel.isInUserQueue.observe(viewLifecycleOwner) { if (it) { - // Remove any highlighted ViewHolders if the playback is in the user queue. - detailAdapter.setCurrentSong(null) - lastHolder?.setHighlighted(false) - lastHolder = null + detailAdapter.highlightSong(null, binding.detailRecycler) } } 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 b7bd521c8..387b44340 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/ArtistDetailFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/ArtistDetailFragment.kt @@ -15,7 +15,6 @@ import org.oxycblt.auxio.music.Artist import org.oxycblt.auxio.music.BaseModel import org.oxycblt.auxio.music.MusicStore import org.oxycblt.auxio.playback.state.PlaybackMode -import org.oxycblt.auxio.recycler.Highlightable import org.oxycblt.auxio.ui.setupAlbumActions /** @@ -111,32 +110,9 @@ class ArtistDetailFragment : DetailFragment() { playbackModel.parent.observe(viewLifecycleOwner) { parent -> if (playbackModel.mode.value == PlaybackMode.IN_ALBUM && parent is Album?) { - detailAdapter.setCurrentAlbum(parent) - - lastHolder?.setHighlighted(false) - lastHolder = null - - if (parent != null) { - // Use existing data instead of having to re-sort it. - val pos = detailAdapter.currentList.indexOfFirst { - it.name == parent.name - } - - // Check if the ViewHolder if this album is visible, and highlight it if so. - binding.detailRecycler.layoutManager?.findViewByPosition(pos)?.let { child -> - binding.detailRecycler.getChildViewHolder(child)?.let { - lastHolder = it as Highlightable - - lastHolder?.setHighlighted(true) - } - } - } + detailAdapter.setCurrentAlbum(parent, binding.detailRecycler) } else { - // Clear the viewholders if the mode isn't IN_ALBUM - detailAdapter.setCurrentAlbum(null) - - lastHolder?.setHighlighted(false) - lastHolder = null + detailAdapter.setCurrentAlbum(null, binding.detailRecycler) } } diff --git a/app/src/main/java/org/oxycblt/auxio/detail/DetailFragment.kt b/app/src/main/java/org/oxycblt/auxio/detail/DetailFragment.kt index ede00d17c..4bcca3a42 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/DetailFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/DetailFragment.kt @@ -29,7 +29,6 @@ abstract class DetailFragment : Fragment() { protected val binding: FragmentDetailBinding by memberBinding( FragmentDetailBinding::inflate ) - protected var lastHolder: Highlightable? = null override fun onViewCreated(view: View, savedInstanceState: Bundle?) { requireActivity().onBackPressedDispatcher.addCallback(viewLifecycleOwner, callback) @@ -47,12 +46,6 @@ abstract class DetailFragment : Fragment() { callback.isEnabled = false } - override fun onDestroyView() { - super.onDestroyView() - - lastHolder = null - } - /** * Shortcut method for doing setup of the detail toolbar. */ diff --git a/app/src/main/java/org/oxycblt/auxio/detail/adapters/AlbumDetailAdapter.kt b/app/src/main/java/org/oxycblt/auxio/detail/adapters/AlbumDetailAdapter.kt index 090a01e7d..ad47aa673 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/adapters/AlbumDetailAdapter.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/adapters/AlbumDetailAdapter.kt @@ -75,13 +75,31 @@ class AlbumDetailAdapter( * Update the current song that this adapter should be watching for to highlight. * @param song The [Song] to highlight if found, null to clear any highlighted ViewHolders */ - fun setCurrentSong(song: Song?) { + fun highlightSong(song: Song?, recycler: RecyclerView) { // Clear out the last ViewHolder as a song update usually signifies that this current // ViewHolder is likely invalid. lastHolder?.setHighlighted(false) lastHolder = null currentSong = song + + if (song != null) { + // Use existing data instead of having to re-sort it. + val pos = currentList.indexOfFirst { + it.name == song.name && it is Song + } + + // Check if the ViewHolder for this song is visible, if it is then highlight it. + // If the ViewHolder is not visible, then the adapter should take care of it if + // it does become visible. + recycler.layoutManager?.findViewByPosition(pos)?.let { child -> + recycler.getChildViewHolder(child)?.let { + lastHolder = it as Highlightable + + lastHolder?.setHighlighted(true) + } + } + } } inner class AlbumHeaderViewHolder( diff --git a/app/src/main/java/org/oxycblt/auxio/detail/adapters/ArtistDetailAdapter.kt b/app/src/main/java/org/oxycblt/auxio/detail/adapters/ArtistDetailAdapter.kt index c398c9009..941bdf597 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/adapters/ArtistDetailAdapter.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/adapters/ArtistDetailAdapter.kt @@ -76,13 +76,29 @@ class ArtistDetailAdapter( * Update the current album that this adapter should be watching for to highlight. * @param album The [Album] to highlight if found, null to clear any highlighted ViewHolders */ - fun setCurrentAlbum(album: Album?) { + fun setCurrentAlbum(album: Album?, recycler: RecyclerView) { // Clear out the last ViewHolder as a song update usually signifies that this current // ViewHolder is likely invalid. lastHolder?.setHighlighted(false) lastHolder = null currentAlbum = album + + if (album != null) { + // Use existing data instead of having to re-sort it. + val pos = currentList.indexOfFirst { + it.name == album.name && it is Album + } + + // Check if the ViewHolder if this album is visible, and highlight it if so. + recycler.layoutManager?.findViewByPosition(pos)?.let { child -> + recycler.getChildViewHolder(child)?.let { + lastHolder = it as Highlightable + + lastHolder?.setHighlighted(true) + } + } + } } inner class ArtistHeaderViewHolder( diff --git a/app/src/main/java/org/oxycblt/auxio/music/Models.kt b/app/src/main/java/org/oxycblt/auxio/music/Models.kt index 803e6adbf..f4ee767c4 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/Models.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/Models.kt @@ -59,7 +59,7 @@ data class Song( /** * Apply an album to a song. - * @throws IllegalArgumentException When a album is already applied. + * @throws IllegalArgumentException When an album is already applied. */ fun applyAlbum(album: Album) { check(mAlbum == null) { "Album is already applied" } diff --git a/app/src/main/java/org/oxycblt/auxio/playback/NotificationUtils.kt b/app/src/main/java/org/oxycblt/auxio/playback/NotificationUtils.kt index 992591ed0..94c9a26f5 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/NotificationUtils.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/NotificationUtils.kt @@ -43,7 +43,7 @@ fun NotificationManager.createMediaNotification( context: Context, mediaSession: MediaSessionCompat ): NotificationCompat.Builder { - // Create a notification channel if requireds + // Create a notification channel if required if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { val channel = NotificationChannel( NotificationUtils.CHANNEL_ID, diff --git a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackService.kt b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackService.kt index 8bf9bb837..306e86a89 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackService.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackService.kt @@ -438,7 +438,7 @@ class PlaybackService : Service(), Player.EventListener, PlaybackStateManager.Ca * @param reason (Debug) The reason for this call. */ private fun startForegroundOrNotify(reason: String) { - // Don't start the foreground if: + // Don't start foreground if: // - The playback hasnt even started // - The playback hasnt been restored // - There is nothing to play diff --git a/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueFragment.kt b/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueFragment.kt index d67eba8e5..4b115130f 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueFragment.kt @@ -130,7 +130,7 @@ class QueueFragment : Fragment() { getString(R.string.label_all_songs) } else { if (playbackModel.parent.value is Genre) { - // Since t + // Use display name for Genres so that numbers dont show up (playbackModel.parent.value as Genre).displayName } else { playbackModel.parent.value!!.name 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 aa8796a1e..deb5ee6b3 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 @@ -118,8 +118,6 @@ class PlaybackStateManager private constructor() { val isRestored: Boolean get() = mIsRestored /** Whether this instance has started playing or not */ val hasPlayed: Boolean get() = mHasPlayed - /** Whether playback is in the user queue or not */ - val isInUserQueue: Boolean get() = mIsInUserQueue private val settingsManager = SettingsManager.getInstance() diff --git a/app/src/main/res/layout/fragment_detail.xml b/app/src/main/res/layout/fragment_detail.xml index 72a020057..4fa98ebbe 100644 --- a/app/src/main/res/layout/fragment_detail.xml +++ b/app/src/main/res/layout/fragment_detail.xml @@ -14,7 +14,6 @@ style="@style/Toolbar.Style.Icon" android:background="?android:attr/windowBackground" android:elevation="@dimen/elevation_normal" - app:contentInsetStartWithNavigation="0dp" app:title="@string/label_library" tools:menu="@menu/menu_artist_actions" /> diff --git a/app/src/main/res/values/styles.xml b/app/src/main/res/values/styles.xml index ac498bb02..f108afbe2 100644 --- a/app/src/main/res/values/styles.xml +++ b/app/src/main/res/values/styles.xml @@ -34,6 +34,7 @@