From 4ed8a7e9679a97cdfaf888a9e2d4095cadd0c761 Mon Sep 17 00:00:00 2001 From: Alexander Capehart Date: Fri, 30 Dec 2022 17:19:41 -0700 Subject: [PATCH] queue: scroll to top of list on song changes Always scroll to the top of the queue list when the current song changes. This way, the user can see future items rather than past items. In an ideal world, I would try to go to the center of the queue, but it seems like the "average" scroll tends to settle at the top no matter what I do, so whatever. There's also a slight in-accuracy in what the app considers the "Top" of the queue, but that's considered minimally detrimental given how much a QoL improvement this is. Resolves #210. --- CHANGELOG.md | 1 + .../main/java/org/oxycblt/auxio/music/Music.kt | 1 - .../auxio/playback/queue/QueueAdapter.kt | 3 ++- .../auxio/playback/queue/QueueFragment.kt | 18 +++++++++++++----- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3d67894a..61e4cbaea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ #### What's Improved - Formalized whitespace handling - Value lists are now properly localized +- Queue no longer primarily shows previous songs when opened ## 3.0.0 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 04ba735f7..8d86c5c78 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/Music.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/Music.kt @@ -520,7 +520,6 @@ class Song constructor(raw: Raw, settings: Settings) : Music() { for (i in _artists.indices) { // Non-destructively reorder the linked artists so that they align with // the artist ordering within the song metadata. - // TODO: Make sure this works for artists only derived from album artists. val newIdx = _artists[i]._getOriginalPositionIn(_rawArtists) val other = _artists[newIdx] _artists[newIdx] = _artists[i] diff --git a/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueAdapter.kt b/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueAdapter.kt index 102fa38a2..d8d761dc5 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueAdapter.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueAdapter.kt @@ -199,7 +199,8 @@ class QueueSongViewHolder private constructor(private val binding: ItemQueueSong binding.songAlbumCover.bind(song) binding.songName.text = song.resolveName(binding.context) binding.songInfo.text = song.resolveArtistContents(binding.context) - // TODO: Why is this here? + // Not swiping this ViewHolder if it's being re-bound, ensure that the background is + // not visible. See QueueDragCallback for why this is done. binding.background.isInvisible = true // Set up the drag handle to start a drag whenever it is touched. 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 b86db473e..29421415c 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 @@ -24,6 +24,7 @@ import androidx.fragment.app.activityViewModels import androidx.recyclerview.widget.ItemTouchHelper import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.RecyclerView +import kotlin.math.min import org.oxycblt.auxio.databinding.FragmentQueueBinding import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.playback.PlaybackViewModel @@ -108,17 +109,24 @@ class QueueFragment : ViewBindingFragment(), QueueAdapter. queueModel.finishReplace() // If requested, scroll to a new item (occurs when the index moves) - // TODO: Scroll to center/top instead of bottom val scrollTo = queueModel.scrollTo if (scrollTo != null) { - // Do not scroll to indices that are in the currently visible range. As that would - // lead to the queue jumping around every time goto is called. val lmm = binding.queueRecycler.layoutManager as LinearLayoutManager val start = lmm.findFirstCompletelyVisibleItemPosition() val end = lmm.findLastCompletelyVisibleItemPosition() - if (scrollTo !in start..end) { - logD("Scrolling to new position") + val notInitialized = start == RecyclerView.NO_POSITION || end == RecyclerView.NO_POSITION + // When we scroll, we want to scroll to the almost-top so the user can see + // future songs instead of past songs. The way we have to do this however is + // dependent on where we have to scroll to get to the currently playing song. + if (notInitialized || scrollTo < start) { + // We need to scroll upwards, or initialize the scroll, no need to offset binding.queueRecycler.scrollToPosition(scrollTo) + } else if (scrollTo > end) { + // We need to scroll downwards, we need to offset by a screen of songs. + // This does have some error due to what the layout manager returns being + // somewhat mutable. This is considered okay. + binding.queueRecycler.scrollToPosition( + min(queue.lastIndex, scrollTo + (end - start))) } } queueModel.finishScrollTo()