From 55ed55c5dcb1571a4b023a2a1c91b4f7cb812d68 Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Fri, 23 Oct 2020 19:15:10 -0600 Subject: [PATCH] Improve queue editing Fix issue where moving queue items would swap them, not move them, also prevent QueueAdapter from scrolling uncontrollably if the first item is moved. --- .../auxio/playback/PlaybackViewModel.kt | 14 +++++++------- .../auxio/playback/queue/QueueAdapter.kt | 11 +++++------ .../auxio/playback/queue/QueueDragCallback.kt | 18 ++++++++++++------ .../auxio/playback/queue/QueueFragment.kt | 17 +++++++++++++---- .../recycler/viewholders/BaseViewHolder.kt | 1 + app/src/main/res/layout/item_queue_song.xml | 7 ++++--- 6 files changed, 42 insertions(+), 26 deletions(-) diff --git a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt index e3fd8d617..9fba5968d 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt @@ -255,7 +255,8 @@ class PlaybackViewModel : ViewModel() { mCanAnimate = false } - // Move two queue items. Called by QueueDragCallback. + // Move two queue items. Note that this function does not force-update the queue, + // as calling updateData with a drag would cause bugs. fun moveQueueItems(adapterFrom: Int, adapterTo: Int) { // Translate the adapter indices into the correct queue indices val delta = mQueue.value!!.size - formattedQueue.value!!.size @@ -265,13 +266,11 @@ class PlaybackViewModel : ViewModel() { try { val currentItem = mQueue.value!![from] - val targetItem = mQueue.value!![to] - // Then swap the items manually since kotlin does have a swap function. - mQueue.value!![to] = currentItem - mQueue.value!![from] = targetItem + mQueue.value!!.removeAt(from) + mQueue.value!!.add(to, currentItem) } catch (exception: IndexOutOfBoundsException) { - Log.e(this::class.simpleName, "Indices were out of bounds, did not swap queue items") + Log.e(this::class.simpleName, "Indices were out of bounds, did not move queue item") return } @@ -279,7 +278,8 @@ class PlaybackViewModel : ViewModel() { forceQueueUpdate() } - // Remove a queue item. Called by QueueDragCallback. + // Remove a queue item. Note that this function does not force-update the queue, + // as calling updateData with a drag would cause bugs. fun removeQueueItem(adapterIndex: Int) { // Translate the adapter index into the correct queue index val delta = mQueue.value!!.size - formattedQueue.value!!.size 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 f3f395dd9..e0eb9121c 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 @@ -11,10 +11,9 @@ import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.recycler.DiffCallback import org.oxycblt.auxio.recycler.viewholders.BaseViewHolder -// FIXME: Build a Diff function so that QueueAdapter doesn't scroll wildly when things are moved -class QueueAdapter(private val dragCallback: ItemTouchHelper) : - ListAdapter(DiffCallback()) { - +class QueueAdapter( + val touchHelper: ItemTouchHelper +) : ListAdapter(DiffCallback()) { override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): ViewHolder { return ViewHolder(ItemQueueSongBinding.inflate(LayoutInflater.from(parent.context))) } @@ -23,7 +22,7 @@ class QueueAdapter(private val dragCallback: ItemTouchHelper) : holder.bind(getItem(position)) } - // Generic ViewHolder for a detail album + // Generic ViewHolder for a queue item inner class ViewHolder( private val binding: ItemQueueSongBinding, ) : BaseViewHolder(binding, null) { @@ -35,7 +34,7 @@ class QueueAdapter(private val dragCallback: ItemTouchHelper) : binding.songDragHandle.performClick() if (motionEvent.actionMasked == MotionEvent.ACTION_DOWN) { - dragCallback.startDrag(this) + touchHelper.startDrag(this) return@setOnTouchListener true } diff --git a/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueDragCallback.kt b/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueDragCallback.kt index d57f54516..bae506f83 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueDragCallback.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueDragCallback.kt @@ -4,14 +4,17 @@ import androidx.recyclerview.widget.ItemTouchHelper import androidx.recyclerview.widget.RecyclerView import org.oxycblt.auxio.playback.PlaybackViewModel import kotlin.math.abs +import kotlin.math.max import kotlin.math.min import kotlin.math.sign -class QueueDragCallback(private val playbackModel: PlaybackViewModel) : - ItemTouchHelper.SimpleCallback( - ItemTouchHelper.UP or ItemTouchHelper.DOWN, - ItemTouchHelper.START - ) { +// The drag callback used for the Queue RecyclerView. +class QueueDragCallback( + private val playbackModel: PlaybackViewModel +) : ItemTouchHelper.SimpleCallback( + ItemTouchHelper.UP or ItemTouchHelper.DOWN, + ItemTouchHelper.START +) { override fun interpolateOutOfBoundsScroll( recyclerView: RecyclerView, viewSize: Int, @@ -19,11 +22,14 @@ class QueueDragCallback(private val playbackModel: PlaybackViewModel) : totalSize: Int, msSinceStartScroll: Long ): Int { + // Fix to make QueueFragment scroll when an item is scrolled out of bounds. + // Adapted from NewPipe: https://github.com/TeamNewPipe/NewPipe + val standardSpeed = super.interpolateOutOfBoundsScroll( recyclerView, viewSize, viewSizeOutOfBounds, totalSize, msSinceStartScroll ) - val clampedAbsVelocity = Math.max( + val clampedAbsVelocity = max( MINIMUM_INITIAL_DRAG_VELOCITY, min( abs(standardSpeed), 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 0e9dddfba..ccafbca0f 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 @@ -27,8 +27,9 @@ class QueueFragment : BottomSheetDialogFragment() { ): View? { val binding = FragmentQueueBinding.inflate(inflater) - val helper = ItemTouchHelper(QueueDragCallback(playbackModel)) - + val helper = ItemTouchHelper( + QueueDragCallback(playbackModel) + ) val queueAdapter = QueueAdapter(helper) // --- UI SETUP --- @@ -36,9 +37,9 @@ class QueueFragment : BottomSheetDialogFragment() { binding.queueHeader.setTextColor(accent.first.toColor(requireContext())) binding.queueRecycler.apply { adapter = queueAdapter + itemAnimator = DefaultItemAnimator() applyDivider() setHasFixedSize(true) - itemAnimator = DefaultItemAnimator() helper.attachToRecyclerView(this) } @@ -46,7 +47,15 @@ class QueueFragment : BottomSheetDialogFragment() { // --- VIEWMODEL SETUP --- playbackModel.formattedQueue.observe(viewLifecycleOwner) { - queueAdapter.submitList(it.toMutableList()) + // If the first item is being moved, then scroll to the top position on completion + // to prevent ListAdapter from scrolling uncontrollably. + if (queueAdapter.currentList.isNotEmpty() && it[0].id != queueAdapter.currentList[0].id) { + queueAdapter.submitList(it.toMutableList()) { + binding.queueRecycler.scrollToPosition(0) + } + } else { + queueAdapter.submitList(it.toMutableList()) + } } return binding.root diff --git a/app/src/main/java/org/oxycblt/auxio/recycler/viewholders/BaseViewHolder.kt b/app/src/main/java/org/oxycblt/auxio/recycler/viewholders/BaseViewHolder.kt index d405448ab..0c7017ce2 100644 --- a/app/src/main/java/org/oxycblt/auxio/recycler/viewholders/BaseViewHolder.kt +++ b/app/src/main/java/org/oxycblt/auxio/recycler/viewholders/BaseViewHolder.kt @@ -10,6 +10,7 @@ abstract class BaseViewHolder( private val doOnClick: ((T) -> Unit)? ) : RecyclerView.ViewHolder(baseBinding.root) { init { + // Force the layout to *actually* be the screen width baseBinding.root.layoutParams = RecyclerView.LayoutParams( RecyclerView.LayoutParams.MATCH_PARENT, RecyclerView.LayoutParams.WRAP_CONTENT ) diff --git a/app/src/main/res/layout/item_queue_song.xml b/app/src/main/res/layout/item_queue_song.xml index 90d788095..4f46ca0e8 100644 --- a/app/src/main/res/layout/item_queue_song.xml +++ b/app/src/main/res/layout/item_queue_song.xml @@ -66,11 +66,12 @@